particle-os/debian-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

util: drop absolute path from Chroot.run() calls

Browse source 
We currently use the absolute path of these binaries in the
helper. This has some advantages but given that we control the
inputs for PATH in general it seems unnecessary.

We are also slightly inconsistent about this in the codebase but
favor the non absolute path version. A quick count:
```
$ git grep '"chroot"'|wc -l
13
$ git grep '"/usr/sbin/chroot"'|grep -v test_|wc -l
8
```
for `mount` and `umount` it seems this is the only place that uses
the absolute path.

It's not an important change but it has the nice property that it
allows us to use e.g. `testutil.mock_command()` in our tests and
it would be nice to be consistent.
This commit is contained in:
Michael Vogt 2024-09-11 09:50:41 +02:00 • committed by Simon de Vlieger
parent 0cf3cf9020
commit a3e32f3823
3 changed files with 14 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

10
osbuild/util/chroot.py
View file

@ -21,15 +21,15 @@ class Chroot:
print(f"Making missing chroot directory: {d}")
os.makedirs(self.root + d)
subprocess.run(["/usr/bin/mount", "-t", "proc", "-o", "nosuid,noexec,nodev",
subprocess.run(["mount", "-t", "proc", "-o", "nosuid,noexec,nodev",
"proc", f"{self.root}/proc"],
check=True)
subprocess.run(["/usr/bin/mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime",
subprocess.run(["mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime",
"devtmpfs", f"{self.root}/dev"],
check=True)
subprocess.run(["/usr/bin/mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev",
subprocess.run(["mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev",
"sysfs", f"{self.root}/sys"],
check=True)
@ -38,12 +38,12 @@ class Chroot:
def __exit__(self, exc_type, exc_value, tracebk):
failed_umounts = []
for d in ["/proc", "/dev", "/sys"]:
if subprocess.run(["/usr/bin/umount", "--lazy", self.root + d], check=False).returncode != 0:
if subprocess.run(["umount", "--lazy", self.root + d], check=False).returncode != 0:
failed_umounts.append(d)
if failed_umounts:
print(f"Error unmounting paths from chroot: {failed_umounts}")
def run(self, cmd, **kwargs):
cmd = ["/usr/sbin/chroot", self.root] + cmd
cmd = ["chroot", self.root] + cmd
# pylint: disable=subprocess-run-check
return subprocess.run(cmd, **kwargs) # noqa: PLW1510

2
stages/test/test_dracut.py
View file

@ -27,5 +27,5 @@ def test_dracut_with_initoverlayfs(mocked_run, tmp_path, stage_module, with_init
args, kwargs = mocked_run.call_args_list[3] # chroot is the 4th call
assert kwargs.get("check") is True
run_argv = args[0]
assert run_argv[0] == "/usr/sbin/chroot"
assert run_argv[0] == "chroot"
assert run_argv[2] == expected_argv2

16
test/mod/test_util_chroot.py
View file

@ -27,17 +27,17 @@ def test_chroot_context(mocked_run, tmp_path):
chroot.run(["/bin/false"], check=False)
assert mocked_run.call_args_list == [
call(["/usr/bin/mount", "-t", "proc", "-o", "nosuid,noexec,nodev",
call(["mount", "-t", "proc", "-o", "nosuid,noexec,nodev",
"proc", os.fspath(tmp_path / "proc")], check=True),
call(["/usr/bin/mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime",
call(["mount", "-t", "devtmpfs", "-o", "mode=0755,noexec,nosuid,strictatime",
"devtmpfs", os.fspath(tmp_path / "dev")], check=True),
call(["/usr/bin/mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev",
call(["mount", "-t", "sysfs", "-o", "nosuid,noexec,nodev",
"sysfs", os.fspath(tmp_path / "sys")], check=True),
call(["/usr/sbin/chroot", os.fspath(tmp_path), "/bin/true"], check=True),
call(["/usr/sbin/chroot", os.fspath(tmp_path), "/bin/false"], check=False),
call(["chroot", os.fspath(tmp_path), "/bin/true"], check=True),
call(["chroot", os.fspath(tmp_path), "/bin/false"], check=False),
call(["/usr/bin/umount", "--lazy", os.fspath(tmp_path / "proc")], check=False),
call(["/usr/bin/umount", "--lazy", os.fspath(tmp_path / "dev")], check=False),
call(["/usr/bin/umount", "--lazy", os.fspath(tmp_path / "sys")], check=False),
call(["umount", "--lazy", os.fspath(tmp_path / "proc")], check=False),
call(["umount", "--lazy", os.fspath(tmp_path / "dev")], check=False),
call(["umount", "--lazy", os.fspath(tmp_path / "sys")], check=False),
]