Skip to content

Conversation

@linw1995
Copy link

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

  • Add _nix_refresh_gcroots function to update symlink mtimes
  • Prevent nh from collecting frequently used direnv environments
  • Refresh GC roots when using cached flakes and Nix dev shells

@bbenne10
Copy link
Contributor

bbenne10 commented Nov 2, 2025

I'm hesitant to merge this as it stands.
The logic it introduces is a wet (i.e not DRY) reimplementation of the gcroot creation logic, which I am afraid will become cumbersome and brittle.

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?

@linw1995
Copy link
Author

linw1995 commented Nov 3, 2025

@bbenne10 Good point — I’ve updated the code to use touch as suggested.

The gcroot existence and validity check was already in place, so the script now just calls touch after cache validation.

@bbenne10
Copy link
Contributor

bbenne10 commented Nov 3, 2025

Looking at this implementation makes me wonder: Is it acceptable to simply touch all files under ${layout_dir}, or is there some problem with that? If there is no problem with that - it could be unconditional (even if we're about to delete them).

@Mic92
Copy link
Member

Mic92 commented Nov 3, 2025

Looking at this implementation makes me wonder: Is it acceptable to simply touch all files under ${layout_dir}, or is there some problem with that? If there is no problem with that - it could be unconditional (even if we're about to delete them).

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-*
Copy link
Member

Choose a reason for hiding this comment

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

Simplified it further.

Copy link
Contributor

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.

@bbenne10
Copy link
Contributor

bbenne10 commented Nov 3, 2025

I am happy with this PR as it stands.

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.

3 participants