nix
nix copied to clipboard
Clean up daemon handling
Split common.sh into the vars and functions definitions vs starting
the daemon (and possibly other initialization logic). This way,
init.sh can just source the former. Trying to start the daemon
before nix.conf is written will fail because nix daemon requires
--experimental-features 'nix-command'.
killDaemon is idempotent, so it's safe to call when no daemon is
running.
startDaemon and killDaemon use the PID (which is now exported to
subshells) to decide whether there is work to be done, rather than
NIX_REMOTE, which might conceivably be set differently even if a
daemon is running.
startDaemon and killDaemon can save/restore the old NIX_REMOTE as
NIX_REMOTE_OLD.
init.sh kills daemon before deleting everything (including the daemon
socket).
init.sh is tested on its own. We used to do that. I deleted it in
4720853129b6866775edd9f90ad6f10701f98a3c but I am not sure why. Better
to just restore it; at one point working on this every other test
passed, so seems good to check whether init.sh can be run twice.
~~depends on #6179~~
What was the issue here? I re-run init.sh all the time without any problems.
@edolstra At one point everything was passing but init.sh. This has now grown a lot into fixing various issues. (I am about to push again.)
OK updated with lots more stuff. Curious if CI will agree with me, but seems good locally.
macOS build timed it out?
Probably the flaky build issue in #3605
Thanks
Rebased, hoping won't get that spurious error this time.
Yay it did!
Ping @edolstra and @regnat
@edolstra Does this look good to you? I would like to start pushing on this and other testing improvements again prior to getting back to fun feature work like RFC 92, so we have a firmer foundation before any more (potentially destabilizing) new feature.
@thufschmitt I had thought about that, but I decided you (probably) had an intent of init.sh just preparing files and not running processes, and I decided that was a good thing :). I forget the exact reasons why.
@thufschmitt rebased!
:tada: All dependencies have been resolved !
@edolstra @thufschmitt approved. Is this OK with you?
@edolstra pinging again. It really would be good to merge this. It doesn't commit us to doing any more testing, it just makes the existing testing more robust.
I was inspired to return to this because #7600 has a devilishly hard to debug issue because it only cropped up in the daemon tests, and I was reminded how flaky that stuff is today.
In the process I found that the history had become quite messy, and so I am cleaning it up. ~~This is not just the first commit in what was a few cleanups. So the previous reply history may talk about changes that are no longer in here.~~ Ah I see that the original change probably didn't work on its own.
@thufschmitt Small reminder on reviewing this :). @roberth and I last night discussed some good ideas to clean stuff up further, and avoid the profusion of support files (to which this PR sadly adds one more). But I think it is good to land this in the meantime; having the daemon start and stop logic be more robust will make further cleanups easier.
Well, now there are some conflicts :upside_down_face:
Thanks @thufschmitt. For some reason when I rebased --- no conflicts!