chroot: require explicit initialisation (MR 2252)

We currently lazily initialize the chroot's on first use, plus a few
bonus calls to init. However, there are some instances where we actually
don't want the chroot to be initialised (mostly to break recursion
loops).

Simplify the codebase by removing all of this, and just calling
pmb.chroot.init() where it's needed.

In addition, print a warning if init() is called multiple times for one
chroot. This should help us catch these instances if they crop up again.

Signed-off-by: Caleb Connolly <caleb@postmarketos.org>
This commit is contained in:
Caleb Connolly 2024-05-24 04:03:57 +02:00 committed by Oliver Smith
parent 22f805a325
commit 1d9bbd613e
No known key found for this signature in database
GPG key ID: 5AE7F5513E0885CB
11 changed files with 21 additions and 21 deletions

View file

@ -43,9 +43,11 @@ def init(args: PmbArgs, chroot: Chroot=Chroot.native()):
if marker.exists(): if marker.exists():
return return
# Initialize chroot, install packages
pmb.chroot.init(args, Chroot.native())
pmb.chroot.init(args, chroot)
init_abuild_minimal(args, chroot) init_abuild_minimal(args, chroot)
# Initialize chroot, install packages
pmb.chroot.apk.install(args, pmb.config.build_packages, chroot, pmb.chroot.apk.install(args, pmb.config.build_packages, chroot,
build=False) build=False)

View file

@ -7,6 +7,7 @@ import pmb.chroot.user
from pmb.core.types import PmbArgs from pmb.core.types import PmbArgs
import pmb.helpers.cli import pmb.helpers.cli
import pmb.parse import pmb.parse
import pmb.build
from pmb.core import Chroot from pmb.core import Chroot

View file

@ -241,7 +241,6 @@ def install(args: PmbArgs, packages, chroot: Chroot, build=True):
# Initialize chroot # Initialize chroot
check_min_version(args, chroot) check_min_version(args, chroot)
pmb.chroot.init(args, chroot)
installed_pkgs = pmb.chroot.user(["apk", "info", "-e"] + packages, chroot, output_return=True, check=False) installed_pkgs = pmb.chroot.user(["apk", "info", "-e"] + packages, chroot, output_return=True, check=False)
if installed_pkgs is not None and installed_pkgs.strip().split("\n") == packages: if installed_pkgs is not None and installed_pkgs.strip().split("\n") == packages:

View file

@ -1,6 +1,7 @@
# Copyright 2023 Oliver Smith # Copyright 2023 Oliver Smith
# SPDX-License-Identifier: GPL-3.0-or-later # SPDX-License-Identifier: GPL-3.0-or-later
import os import os
from pmb.core.chroot import Chroot
from pmb.helpers import logging from pmb.helpers import logging
from pmb.core.types import PmbArgs from pmb.core.types import PmbArgs

View file

@ -127,8 +127,7 @@ def init(args: PmbArgs, chroot: Chroot=Chroot.native(), usr_merge=UsrMerge.AUTO,
already_setup = str(chroot) in pmb.helpers.other.cache["pmb.chroot.init"] already_setup = str(chroot) in pmb.helpers.other.cache["pmb.chroot.init"]
if already_setup: if already_setup:
# FIXME: drop to debug/verbose later logging.warning(f"({chroot}) FIXME! init() called multiple times!")
logging.debug(f"({chroot}) already initialised")
return return
pmb.chroot.mount(args, chroot) pmb.chroot.mount(args, chroot)
@ -176,7 +175,7 @@ def init(args: PmbArgs, chroot: Chroot=Chroot.native(), usr_merge=UsrMerge.AUTO,
if not chroot.type == ChrootType.ROOTFS: if not chroot.type == ChrootType.ROOTFS:
pmb.chroot.root(args, ["adduser", "-D", "pmos", "-u", pmb.chroot.root(args, ["adduser", "-D", "pmos", "-u",
pmb.config.chroot_uid_user], pmb.config.chroot_uid_user],
chroot, auto_init=False) chroot)
# Create the links (with subfolders if necessary) # Create the links (with subfolders if necessary)
for target, link_name in pmb.config.chroot_home_symlinks.items(): for target, link_name in pmb.config.chroot_home_symlinks.items():

View file

@ -30,14 +30,13 @@ def executables_absolute_path():
def root(args: PmbArgs, cmd: Sequence[PathString], chroot: Chroot=Chroot.native(), working_dir: PurePath=PurePath("/"), output="log", def root(args: PmbArgs, cmd: Sequence[PathString], chroot: Chroot=Chroot.native(), working_dir: PurePath=PurePath("/"), output="log",
output_return=False, check=None, env={}, auto_init=True, output_return=False, check=None, env={},
disable_timeout=False, add_proxy_env_vars=True): disable_timeout=False, add_proxy_env_vars=True):
""" """
Run a command inside a chroot as root. Run a command inside a chroot as root.
:param env: dict of environment variables to be passed to the command, e.g. :param env: dict of environment variables to be passed to the command, e.g.
{"JOBS": "5"} {"JOBS": "5"}
:param auto_init: automatically initialize the chroot
:param working_dir: chroot-relative working directory :param working_dir: chroot-relative working directory
:param add_proxy_env_vars: if True, preserve HTTP_PROXY etc. vars from host :param add_proxy_env_vars: if True, preserve HTTP_PROXY etc. vars from host
environment. pmb.chroot.user sets this to False environment. pmb.chroot.user sets this to False
@ -47,11 +46,6 @@ def root(args: PmbArgs, cmd: Sequence[PathString], chroot: Chroot=Chroot.native(
See pmb.helpers.run_core.core() for a detailed description of all other See pmb.helpers.run_core.core() for a detailed description of all other
arguments and the return value. arguments and the return value.
""" """
# Initialize chroot
if not auto_init and not (chroot / "bin/sh").is_symlink():
raise RuntimeError(f"Chroot does not exist: {chroot}")
if auto_init:
pmb.chroot.init(args, chroot)
# Convert any Path objects to their string representation # Convert any Path objects to their string representation
cmd_str = [os.fspath(x) for x in cmd] cmd_str = [os.fspath(x) for x in cmd]

View file

@ -73,7 +73,7 @@ def shutdown(args: PmbArgs, only_install_related=False):
if pmb.helpers.mount.ismount(chroot / "dev/loop-control"): if pmb.helpers.mount.ismount(chroot / "dev/loop-control"):
for path_outside in (chroot / "/home/pmos/rootfs").glob("*.img"): for path_outside in (chroot / "/home/pmos/rootfs").glob("*.img"):
path = path_outside.relative_to(chroot.path) path = path_outside.relative_to(chroot.path)
pmb.install.losetup.umount(args, path, auto_init=False) pmb.install.losetup.umount(args, path)
# Umount device rootfs and installer chroots # Umount device rootfs and installer chroots
for chroot_type in [ChrootType.ROOTFS, ChrootType.INSTALLER]: for chroot_type in [ChrootType.ROOTFS, ChrootType.INSTALLER]:

View file

@ -9,7 +9,7 @@ from pmb.core.types import PathString, PmbArgs
def user(args: PmbArgs, cmd, chroot: Chroot=Chroot.native(), working_dir: Path = Path("/"), output="log", def user(args: PmbArgs, cmd, chroot: Chroot=Chroot.native(), working_dir: Path = Path("/"), output="log",
output_return=False, check=None, env={}, auto_init=True): output_return=False, check=None, env={}):
""" """
Run a command inside a chroot as "user". We always use the BusyBox Run a command inside a chroot as "user". We always use the BusyBox
implementation of 'su', because other implementations may override the PATH implementation of 'su', because other implementations may override the PATH
@ -17,7 +17,6 @@ def user(args: PmbArgs, cmd, chroot: Chroot=Chroot.native(), working_dir: Path =
:param env: dict of environment variables to be passed to the command, e.g. :param env: dict of environment variables to be passed to the command, e.g.
{"JOBS": "5"} {"JOBS": "5"}
:param auto_init: automatically initialize the chroot
See pmb.helpers.run_core.core() for a detailed description of all other See pmb.helpers.run_core.core() for a detailed description of all other
arguments and the return value. arguments and the return value.
@ -31,7 +30,7 @@ def user(args: PmbArgs, cmd, chroot: Chroot=Chroot.native(), working_dir: Path =
flat_cmd = pmb.helpers.run_core.flat_cmd(cmd, env=env) flat_cmd = pmb.helpers.run_core.flat_cmd(cmd, env=env)
cmd = ["busybox", "su", "pmos", "-c", flat_cmd] cmd = ["busybox", "su", "pmos", "-c", flat_cmd]
return pmb.chroot.root(args, cmd, chroot, working_dir, output, return pmb.chroot.root(args, cmd, chroot, working_dir, output,
output_return, check, {}, auto_init, output_return, check, {},
add_proxy_env_vars=False) add_proxy_env_vars=False)

View file

@ -11,6 +11,7 @@ import pmb.aportgen
import pmb.build import pmb.build
import pmb.build.autodetect import pmb.build.autodetect
import pmb.chroot import pmb.chroot
import pmb.chroot.apk
import pmb.chroot.initfs import pmb.chroot.initfs
import pmb.chroot.other import pmb.chroot.other
import pmb.ci import pmb.ci
@ -172,6 +173,8 @@ def chroot(args: PmbArgs):
if args.add: if args.add:
pmb.chroot.apk.install(args, args.add.split(","), suffix) pmb.chroot.apk.install(args, args.add.split(","), suffix)
pmb.chroot.init(args, suffix)
# Xauthority # Xauthority
env = {} env = {}
if args.xauth: if args.xauth:

View file

@ -1182,6 +1182,7 @@ def create_device_rootfs(args: PmbArgs, step, steps):
' ***') ' ***')
suffix = Chroot(ChrootType.ROOTFS, args.device) suffix = Chroot(ChrootType.ROOTFS, args.device)
pmb.chroot.init(args, suffix)
# Create user before installing packages, so post-install scripts of # Create user before installing packages, so post-install scripts of
# pmaports can figure out the username (legacy reasons: pmaports#820) # pmaports can figure out the username (legacy reasons: pmaports#820)
set_user(args) set_user(args)
@ -1302,6 +1303,7 @@ def install(args: PmbArgs):
# Install required programs in native chroot # Install required programs in native chroot
step = 1 step = 1
logging.info(f"*** ({step}/{steps}) PREPARE NATIVE CHROOT ***") logging.info(f"*** ({step}/{steps}) PREPARE NATIVE CHROOT ***")
pmb.chroot.init(args, Chroot.native())
pmb.chroot.apk.install(args, pmb.config.install_native_packages, Chroot.native(), pmb.chroot.apk.install(args, pmb.config.install_native_packages, Chroot.native(),
build=False) build=False)
step += 1 step += 1

View file

@ -57,14 +57,14 @@ def mount(args: PmbArgs, img_path: Path):
raise RuntimeError(f"Failed to mount loop device: {img_path}") raise RuntimeError(f"Failed to mount loop device: {img_path}")
def device_by_back_file(args: PmbArgs, back_file: Path, auto_init=True) -> Path: def device_by_back_file(args: PmbArgs, back_file: Path) -> Path:
""" """
Get the /dev/loopX device that points to a specific image file. Get the /dev/loopX device that points to a specific image file.
""" """
# Get list from losetup # Get list from losetup
losetup_output = pmb.chroot.root(args, ["losetup", "--json", "--list"], losetup_output = pmb.chroot.root(args, ["losetup", "--json", "--list"],
output_return=True, auto_init=auto_init) output_return=True)
if not losetup_output: if not losetup_output:
raise RuntimeError("losetup failed") raise RuntimeError("losetup failed")
@ -76,14 +76,14 @@ def device_by_back_file(args: PmbArgs, back_file: Path, auto_init=True) -> Path:
raise RuntimeError(f"Failed to find loop device for {back_file}") raise RuntimeError(f"Failed to find loop device for {back_file}")
def umount(args: PmbArgs, img_path: Path, auto_init=True): def umount(args: PmbArgs, img_path: Path):
""" """
:param img_path: Path to the img file inside native chroot. :param img_path: Path to the img file inside native chroot.
""" """
device: Path device: Path
try: try:
device = device_by_back_file(args, img_path, auto_init) device = device_by_back_file(args, img_path)
except RuntimeError: except RuntimeError:
return return
logging.debug(f"(native) umount {device}") logging.debug(f"(native) umount {device}")
pmb.chroot.root(args, ["losetup", "-d", device], auto_init=auto_init) pmb.chroot.root(args, ["losetup", "-d", device])