nixos-install: re-enable --chroot option #28777
Conversation
| # Set up some bind mounts we'll want regardless of chroot or not | ||
| mount --rbind /dev $mountPoint/dev | ||
| mount --rbind /proc $mountPoint/proc | ||
| mount --rbind /sys $mountPoint/sys |
There was a problem hiding this comment.
Is the mountPoint chosen by users? I wonder if it make sense to quote it.
There was a problem hiding this comment.
Yeah, probably does. I'll fix that, thanks
| rm -rf $mountPoint/var/run | ||
| ln -s /run $mountPoint/var/run | ||
| for f in /etc/resolv.conf /etc/hosts; do rm -f $mountPoint/$f; [ -f "$f" ] && cp -Lf $f $mountPoint/etc/; done | ||
| for f in /etc/passwd /etc/group; do touch $mountPoint/$f; [ -f "$f" ] && mount --rbind -o ro $f $mountPoint/$f; done |
There was a problem hiding this comment.
This line means that you cannot chroot into the new installation, and use passwd to setup passwords, right?
There was a problem hiding this comment.
Yeah, this is basically just copied and pasted from the old installer's --chroot support.
copumpkin
force-pushed
the
installer-chroot
branch
from
8e94a9b
to
1241b03
Compare
|
there was also a weird quick in how |
|
@cleverca22 I didn't really change the arg parsing at all. Does it look like I messed that up here? |
|
@copumpkin i dont think you broke it, but that is one quirk you can test for, see if it defaults to a shell, and also allows still running a given command directly |
|
Yeah, my point is just that I can't actually test it easily right now, because I don't have a Linux box and can't run VM tests, so I'd appreciate help from anyone who has an actual box to run it on 😄 |
| # Set up some bind mounts we'll want regardless of chroot or not | ||
| mount --rbind /dev "$mountPoint/dev" | ||
| mount --rbind /proc "$mountPoint/proc" | ||
| mount --rbind /sys "$mountPoint/sys" |
There was a problem hiding this comment.
In case of doing a fresh install those mount points don't exist yet.
There was a problem hiding this comment.
We only support doing this from some sort of live cd installation, as far as I know, which will have those mount points. I'm just replicating the old installer's behavior here, not trying to add new functionality.
There was a problem hiding this comment.
AFAICT those don't exist until nixos-prepare-root is ran later on in the installer, and installation starts off with more or less empty $mountPoint:
$ nix-build "nixos/release-combined.nix" -A nixos.tests.installer.simple.x86_64-linux
# bunch of noise omitted
machine: must succeed: nixos-install < /dev/null >&2
machine# > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > mount: /mnt/dev: mount point does not exist.
machine: exit status 32
machine: output:
error: command `nixos-install < /dev/null >&2' did not succeed (exit code 32)
command `nixos-install < /dev/null >&2' did not succeed (exit code 32)
There was a problem hiding this comment.
@pbogdan that's why I consider this option to be pretty awkward, but my understanding is that you're only expected to run --chroot on an "installed" (i.e., you've already run install on it once at some point in the past) $mountPoint. It really doesn't feel to me like this functionality belongs in the installer itself, but rather in an adjoining tool that's called something like "poke-around-post-install.sh", but I didn't want to break the existing interface so that's why it's here.
So at that point, $mountPoint has the folders. I think?
There was a problem hiding this comment.
Yes, for --chroot the assumption is that you have an existing install somewhere and all of those are present. I'm only trying to point out that as is this change would break the use case of using the installer as, well, installer 😆.
And I agree that it seems like somewhat strange functionality to be included, and recall initially being a bit confused as to what purpose it was supposed to serve in context of an installer.
There was a problem hiding this comment.
Oh, good point, sorry for being dense 😄 I'll push up a fix later
| @@ -102,16 +102,37 @@ fi | |||
| extraBuildFlags+=(--option "build-users-group" "$buildUsersGroup") | |||
|
|
|||
| # Inherit binary caches from the host | |||
| # TODO: will this still work with Nix 1.12 now that it has no perl? Probably not... | |||
| # TODO: will this still work with Nix 1.12 now that it has no perl? Probably not... | |||
| binary_caches="$(@perl@/bin/perl -I @nix@/lib/perl5/site_perl/*/* -e 'use Nix::Config; Nix::Config::readConfig; print $Nix::Config::config{"binary-caches"};')" | |||
| extraBuildFlags+=(--option "binary-caches" "$binary_caches") | |||
|
|
|||
| nixpkgs="$(readlink -f "$(nix-instantiate --find-file nixpkgs)")" | |||
| export NIX_PATH="nixpkgs=$nixpkgs:nixos-config=$mountPoint/$NIXOS_CONFIG" | |||
There was a problem hiding this comment.
Not sure if it matters much but this would propagate "configured-for-installation" NIX_PATH into the chroot environment.
There was a problem hiding this comment.
I think that's what it used to do. In case you're missing context, I basically rewrote how nixos-install worked a couple of months ago, and forgot to support the --chroot option at the time. This is me just bringing it back to life, not trying to do anything fancy.
I forgot to implement it the first time around. Whoops!
copumpkin
force-pushed
the
installer-chroot
branch
from
1241b03
to
186c120
Compare
|
|
||
| # These get created in nixos-prepare-root as well, but we want to make sure they're here in case we're | ||
| # running with --chroot. TODO: --chroot should just be split into a separate tool. | ||
| mkdir -m 0755 -p "$mountPoint/dev" "$mountPoint/proc" "$mountPoint/sys" |
There was a problem hiding this comment.
@pbogdan I think this resolves the issue you pointed out, right?
There was a problem hiding this comment.
@copumpkin LGTM, checked with nixos.tests.installer.simple.x86_64-linux and --chroot on a fresh install and everything is working AFAICT.
|
@globin should I backport this to 17.09 now? Or is that automated somehow? Sorry, I'm pretty clueless about this whole release thing 😄 |
|
Thank you, and sorry this took so long! I can cherry-pick going forward 😄 |
I forgot to implement it the first time around. Whoops!
Motivation for this change
I broke it, I fix it. Should fix #28251. @cleverca22 do you have an easy harness to check this? It would be nice to add to the installer tests but I can't really engage with any of the VM tests easily since I don't have a machine that can run them right now.