Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh: add a list of known nvidia drivers as an attr in nixGL #131

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

cfhammill
Copy link

This can be used to generate wrappers purely and programmatically inside of flakes.

There's a script I used to generate the version/hash pairs in the list, see the included readme for more details.

This can be used to generate wrappers purely and programmatically
inside of flakes
@joefiorini
Copy link

I like this! Just had to spend 2 hours or so updating a fork of this code because my distro updated my nvidia driver to 530. Although it now needs zstd, so I had to modify the dependencies of the fetcher as well.

Comment on lines +55 to +56
subprocess.check_output(["nix-store", "--delete", store_pth])
print("Cleanup success.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just run nix-collect-garbage?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my own nix use I'm not very careful with my gcroots and would prefer the gc not be run unless I do it explicitly. Do you see advantages to using that instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just feels unnecessary if there are easier ways.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean running nix-collect-garbage after downloading each runfile? If so I don't see if as much of an improvement and may inadvertently gc something unintended.

If you mean wait until the end of the script and then call gc, if so that's probably a bit faster, the store deletion adds some overhead to the download loop, but still runs the risk of gc'ing something unintended. Further the runfiles are now quite large and on computers with limited space waiting to the end to gc might not be best, this was why I wrote this in in the first place.

If you mean leave it up to the user to GC or not after running this script, that sounds good, suffers the space issue noted above, but in most contexts, especially with incremental downloading is probably fine.

Copy link
Member

@SuperSandro2000 SuperSandro2000 Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean running nix-collect-garbage after downloading each runfile? If so I don't see if as much of an improvement and may inadvertently gc something unintended.

Just once in the end if desired and I would be willing to accept that.

I mean if you really want to keep it, I don't have a problem with it, I just wanted to check if you just were searching for that command or so.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this approach; it seems more fine-grained and more performant than calling nix-collect-garbage, and you could always disabled cleanup and call the gc yourself if preferred.

@cfhammill
Copy link
Author

Now that this project has moved to nix-community, is anyone with commit rights able to say if they'd be interested in ever merging this?

I'm maintaining the branch for my own projects, no need to leave the PR open if this addition isn't desirable for the nixGL userbase at large.

@ulucs
Copy link

ulucs commented Oct 24, 2024

Hello! Is there any way for me to help let this PR get merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants