Review: Needs Information
Diff comments: > diff --git a/curtin/distro.py b/curtin/distro.py > index 3284b69..18912e2 100644 > --- a/curtin/distro.py > +++ b/curtin/distro.py > @@ -280,6 +281,25 @@ def apt_install(mode, packages=None, opts=None, > env=None, target=None, > if opts is None: > opts = [] > > + if systemd_force_offline: > + env = env.copy() if env is not None else os.environ.copy() > + # To determine if we are running in a chroot, systemd checks if > + # /proc/1/root (corresponding to the init process) and / are the same > + # inode. If they are different, systemd assumes we are in a chroot. > + # However, we are running apt-get in a new PID namespace (with /proc > + # properly mounted). This means that in the new namespace, apt-get > gets > + # assigned PID 1 and is therefore the "init" process. > + # When systemd compares /proc/1/root and /, it sees they are > identical > + # because the init process is actually running in the chroot. > + # > + # Before we started passing the --mount-proc option to unshare, it > was > + # working because /proc/1 in the chroot would still refer to the > + # systemd init process (running outside the chroot). > + # > + # With the SYSTEMD_OFFLINE variable, one can "force" systemd to > assume > + # it is running in a chroot. Let's use it. > + env['SYSTEMD_OFFLINE'] = 'true' >From an abstraction standpoint, if I have SYSTEMD_OFFLINE in the env dict, I >can alter this behavior without using the argument. Also, >systemd_force_offline is defaulting to True, so if someone is explicitly >setting it they may really want the False behavior, so maybe we should pass >that along on the resulting SYSTEMD_OFFLINE value. Proposal: * default the systemd_force_offline argument to None * if systemd_force_offline is None and SYSTEMD_OFFLINE set in the environment respect that * if systemd_force_offline is None and SYSTEMD_OFFLINE not set in the environment we default to setting it to True, or if systemd_force_offline==False we set it SYSTEMD_OFFLINE='false' Also, does 'true'/'false' matter? I do note that https://systemd.io/ENVIRONMENT/ lists '0'/'1', that might be safer. > + > if mode not in ['install', 'upgrade', 'dist-upgrade']: > raise ValueError( > 'Unsupported mode "%s" for apt package install/upgrade' % mode) -- https://code.launchpad.net/~ogayot/curtin/+git/curtin/+merge/462140 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~curtin-dev More help : https://help.launchpad.net/ListHelp
