Use nix-store --add-root instead of hard-coding gcroot path (fix for Nix 2.14+)

[amarshall]

Do so by no longer hard-coding gcroot paths, and defer to Nix to manage things.

NixOS/nix#5226

[bbenne10]

I'm quite confused as to why this is failing.
I see the changes in 2.14 and understand that we need to update our handling of gcroots and profiles to account for the move.
I too am unsatisfied with the tests as they stand, but I am unsure how to test these changes in a more fine grained manner. I believe you can comment out the nix-collect-garbage invocations and still have valid results.

However, the main question I have right now (and I am surprised that it's taken me this long to realize this) is: Why aren't we using nix-store --add-root? We're simply reinventing the wheel here, I think? That'd allow us to not worry about this at all, wouldn't it? We would only need to worry about getting $profile_rc and $profile into $layout_dir then, I think.

[amarshall]

I also don’t understand the failure. I did a few runs with set -x (see here) and it was…not very enlightening.

I did end up temp. removing the nix-collect-garbage calls, but I was only running Nix 2.11 locally and everything passed.

nix-store --add-root definitely seems more reasonable, I was mostly going with the grain of what was here for minimal change. How would we remove the root? nix-store -q --roots $path or something, I guess? Either way, I think it’s orthogonal to the failure, unfortunately.

[bbenne10]

Is it really orthogonal? it creates the roots without us having to worry about exactly where the gcroot got created.

Regarding removal: couldnt we just remove the created files inside $LAYOUT_DIR, which would cause the symlink to dangle and then qualify that root for collection?

I tried to figure this out but couldnt get nix 2.14 no matter how I tried. The latest i got was 2.13.3 with latest nixpkgs-unstable and the nixUnstable package...

[amarshall]

I mean that I don’t think it will fix the failure currently on this branch, it’s certainly relevant to the problem this branch is trying to solve.

Yes, I believe you’re correct, there’s no need to micromanage all the references during removal so we don’t have to care.

[amarshall]

I’ve force-pushed an alternative impl based on your suggestions—definitely much nicer! However the tests are still failing and it’s not clear to me yet why it doesn’t think it can use the cache anymore, but I need to big a bit more through the trace.

[Mic92]

I have pinned the ci to nix 2.13 for now. You may want to unpin in this PR for testing: #312

[bbenne10]

Had a play around with this yesterday (using the podman image released as part of the nix-unstable-installer releases) and the problem is either in gcroot creation or gcroot persistence after garbage collection. The first pass seems to work well enough (after first GC, but pre second GC in the tests) but post-GC, we consistently enter the "cache invalid" portion of the branch logic. I modified direnv to print out WHY we were entering that part of the branch and it is because $profile is missing. I'm not sure yet why that is so, but I suspect we're not doing enough to register a gcroot.

[amarshall]

Alright, this is working now. It ended up being quite simple, but, IMO, the API of nix-store --add-root is not so obvious. It still needs the store path, and will just silently exit otherwise. Since it prints the path when it is doing things, swallow it. Since it will fail if the symlink already exists and is not a link to the given store path, still ln ourselves.

I’ve rebased off master and amended in the change, which is just:

diff --git a/direnvrc b/direnvrc                                                                                                                              
index 539168e..a447268 100644                                                                                                                                 
--- a/direnvrc                                                                                                                                                
+++ b/direnvrc                                                                                                                                                
@@ -132,7 +132,7 @@ _nix_add_gcroot() {                                                                                                                       
   local symlink=$2

   ln -fsn "$storepath" "$symlink"
-  nix-store --realise --add-root "$symlink"                                                                                                                  
+  nix-store --realise "$storepath" --add-root "$symlink" >/dev/null                                                                                          
 }

 _nix_clean_old_gcroots() {

[amarshall]

I didn’t want to unpin Nix in CI in this PR (probably pinning to something is useful to prevent unexpected change, and perhaps CI should run against multiple versions). I did a CI run on Nix 2.15 that passed.

[bbenne10]

I'm happy to merge this JUST for the simplification in root creation alone, but the fact that this makes it work moving forward to nix 2.14+ is awesome!

I'm actually inclined to unpin the Nix runner here as I want it to fail on unstable Nix versions so that we notice breakage. @Mic92 do you have thoughts on this? I'll go ahead and revert the pin if you don't have issue.

Going to go ahead and merge this. Thank you for the contribution!

[Mic92]

@bbenne10 please do so.