-
-
Notifications
You must be signed in to change notification settings - Fork 128
Add GC root refresh functionality #631
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
base: master
Are you sure you want to change the base?
Conversation
|
I'm hesitant to merge this as it stands. Is it possible to refactor the gcroot creation logic to simply touch the files in question if they already exist and then always call the creation logic? That would collapse "check if they exist; touch them if they do; create them if they don't" into just "touch these files", I think? |
|
@bbenne10 Good point — I’ve updated the code to use The gcroot existence and validity check was already in place, so the script now just calls |
|
Looking at this implementation makes me wonder: Is it acceptable to simply touch all files under |
This might interfere with other direnv integrations, I would not go down that route. |
This helps with tools that delete gcroots based on access time.
| local layout_dir | ||
| layout_dir=$(direnv_layout_dir) | ||
|
|
||
| touch -h "${layout_dir}"/flake-profile-* "${layout_dir}"/flake-inputs/* "${layout_dir}"/nix-profile-* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified it further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets at the heart of what I was trying to point out earlier. I wish we had a directory or namespace to select only "our" files, but checking to see if these files exist here is redundant, as the globs proved they do (+/- a race condition, which I refuse to admit is a problem here until I am proven wrong).
The fact that our files aren't isolated is not this PR's problem however and I might find time to introduce that change at some point in the near future.
|
I am happy with this PR as it stands. |
Hi! I use the nh tool to garbage-collect unused Nix environments — by default, it removes environments that haven’t been used recently. However, I noticed it sometimes cleans up environments that are still actively used but whose symlink files haven’t been updated recently.
After some digging, I found that nh determines whether an environment is “in use” based on the symlink file’s modification time (mtime). This means that even if an environment is used daily via direnv, nh may incorrectly mark it for garbage collection if the timestamp is old.
This PR introduces GC root refresh functionality to address that issue.
Changes
_nix_refresh_gcrootsfunction to update symlink mtimes