Log what causes the cache to be invalidated#627
Merged
Conversation
Fixes nix-community#597 Co-authored-by: Bryan Bennett <Bryan.Bennett@proton.me>
Member
Author
|
I'd love to write a test case for this, but ran into this even without my changes 😦 longIt's not immediately obvious what exactly is failing after prodding at it for a few minutes... doesn't fail in GHA, though! Could be because I use Lix? Though that seems semi-unlikely. |
winterqt
commented
Oct 20, 2025
| for file in "${watches[@]}"; do | ||
| if [[ $file -nt $profile_rc ]]; then | ||
| need_update=1 | ||
| break |
Member
Author
There was a problem hiding this comment.
Deviated from your original patch by removing the break here, so that all newer files get added to the array, instead of just the first.
Contributor
There was a problem hiding this comment.
This is just a performance increase. I have yet to see direnv or nix-direnv have speed issues on reasonably sized inputs. Watching all nix files in nixpkgs has been performant for me, so I'm not too worried about this personally.
Contributor
|
I use Lix as well, so that's not likely the cause. I'll see if I can find a few minutes to poke around (posting from a non-nix enabled system at all right now, so not quite available yet). I'm not sure how I feel about these being unconditionally verbose, but I also don't really know if it is worth it to make them optional. |
Member
Author
|
Thanks @bbenne10 — did you end up figuring out being able to reproduce what was wrong with the tests? |
Contributor
|
I didn't, but getting it merged seemed more important than chasing my tail on that. I'll have to find some time to get it looked over more closely. |
Contributor
|
Could this maybe be disabled by default? Nixpkgs has just bumped to a version with this commit included here, and it spams scrollback every single time I change a nix-direnv tracked file. For now, I'm working around it with: But I still think a saner default would be to not log so much. |
Contributor
|
Any significant change we make here makes someone upset. We simply cannot win. I believe this is a perfectly fine default, but welcome a PR to disable the functionality, much like manual reloading does. I will not be changing this myself in the near future due to personal reasons affecting my ability to contribute in my (already limited) free time. Thanks for your understanding.
… |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #597