Hi Holger and Nicolas,

I've got some minor itches with piuparts and would like to figure out
which of them you are interested addressing such that I can file
corresponding MRs on salsa. In case you don't find this useful, please
just close the bug.

# tar compression

When using piuparts with --basetgz it really only accepts something
compressed with gzip. Other tools (schroot, pbuilder, etc.) support
other compression schemes and zstd is a popular contender, because it
can beat gzip on both ratio and speed at the same time.

piuparts has one "tar" invocation for performing the decompression and
passes the "-z" flag there. If we were to change that flag to
"--auto-compress", piuparts would immediately support all sorts of
compression formats. Just the option name "--basetgz" could become a
little misleading. Is that something worth exploring in a MR?

@@ ... @@ unpack_from_tgz
-run(prefix + ["tar", "-C", self.name, "-zxf", tarball])
+run(prefix + ["tar", "-C", self.name, "--auto-compress", "-xf", tarball])

# support mmdebstrap as a debootstrap alternative

If using piuparts without providing some chroot, it'll create one using
debootstrap. Since debootstrap is very compatible, it is relatively slow
compared to mmdebstrap. At the same time, the target audience of
piuparts is Debian, so mmdebstrap will be readily available. Also
mmdebstrap has an extensive test suite that validates its output to be
the same as debootstrap. Since piuparts sets up the target, one has to
pass --skip=check/empty to mmdebstrap though. Is that worth adding
another option?

# mount_proc

When mount_proc notices the absence of /dev/ptmx, it'll create a device
node there and the bind mounts /dev/pts/ptmx there. If running in a
container, mknod may fail and a non-recursive bind mount may fail as
well. A simple variant achieving similar semantics is creating /dev/ptmx
as a symlink when it is entirely missing. If anything, having one less
mount provides a small speedup. Do you want a MR?

@@ ... @@ mount_proc
         dev_ptmx_rel_path = self.relative("dev/ptmx")
         if not os.path.islink(dev_ptmx_rel_path):
             if not os.path.exists(dev_ptmx_rel_path):
-                os.mknod(dev_ptmx_rel_path, 0o0666 | stat.S_IFCHR, os.makedev(5, 2))
-            self.mount(self.relative("dev/pts/ptmx"), "/dev/ptmx", opts="bind", no_mkdir=True)
+                os.symlink("pts/ptmx", dev_ptmx_rel_path)
+            else:
+                self.mount(self.relative("dev/pts/ptmx"), "/dev/ptmx", opts="bind", no_mkdir=True)
         p = subprocess.Popen(["tty"], stdout=subprocess.PIPE,
                              universal_newlines=True)

# umount_all

In some of my experiments, I've seen umount_all fail. Some umount failed
to work and when it panic()ed, it failed on an infinite recursion. I
happen to not have understood the details. I just noticed that adding
--lazy to umount would fix this very problem. It enables you to umount
busy mount points reliably by hiding them from everything else and
letting the kernel garbage collect them when the last user exits. Do you
want that?

@@ ... @@ umount_all
-run(["umount", mountpoint], ignore_errors=True)
+run(["umount", "--lazy", mountpoint], ignore_errors=True)

# Default to recursive bind mounts

When specifying --bind, piuparts performs a non-recursive bind mount.
That may be unintuitive and it also is prohibited in containers. Would
you mind changing bind mounts to recursive ones in general? Is there a
need to support non-recursive ones at all?

@@ ... @@ configure_chroot
 for bindmount in settings.bindmounts:
-    self.mount(bindmount, bindmount, opts="bind")
+    self.mount(bindmount, bindmount, opts="rbind")

# Allow bind mounting non-directories

When specifying --bind, piuparts implies that the thing you mount is a
directory. It could be a device or a file however. When trying to do
that, piuparts still creates a target directory and fails the mount
operation as you cannot bind mount a non-directory on a directory. It
could easily check this situation and create a non-directory node in
such situations. Do you want a MR?

@@ ... @@ mount
 path = canonicalize_path(self.name, path)
+fullpath = self.relative(path)
 if not no_mkdir:
-    self.mkdir_p(path)
-fullpath = self.relative(path)
+    is_non_bind = not set(("bind", "rbind")).intersection((opts or "").split(","))
+    if is_non_bind or os.path.isdir(source):
+        self.mkdir_p(path)
+    elif not os.path.exists(fullpath):
+        self.mkdir_p(os.path.dirname(path))
+        os.mknod(fullpath, stat.S_IFREG)


Thank you for considering

Helmut

Hi Helmut,

can you take a look at #1064350 and #1064842, which may be a regression
(not enough device nodes mounted to /dev) caused by your changes.


Andreas

Hi Andreas,

I looked, but I'm having difficulties making sense of it. Can you assist
with reproducing this somehow?

The first bug points at
https://salsa.debian.org/nvidia-team/bumblebee/-/jobs/5333059#L218 and
line 218 says "Created resolv.conf.". As far as I can see, this is only
emitted from create_resolv_conf() which is only called from
configure_chroot() and the next thing the function does is to stat
self.name + "/dev/null". Given that apt tries to create it later, the
most plausible hypothesis is that it raises FileNotFoundError. Then we
try to os.mknod it as a character device. If that were to succeed, apt
couldn't create it, so it likely raises an OSError with EPERM. Then we
try to create it as a regular file. If that were to succeed, apt
wouldn't report it as missing. If that were to fail, an exception would
unwind configure_chroot and then create and make piuparts crash. Since
this is not happening, the most plausible hypothesis seems to be that
something deletes /dev/null after this method has concluded. Not much is
running there but tmp/scripts/post_chroot_unpack_allow_unauthenticated.

Do you see any flaws in this reasoning?

Regarding the other bug, I set up incus, launched a debian/sid container
and successfully ran piuparts there. The original reports says VM, not
container though. I also tried a --vm, but incus didn't like my qemu
(probably due to nested virtualization) and refused.

I have no idea how to reproduce these failures.

The assumption behind your message is that
https://salsa.debian.org/debian/piuparts/-/commit/aa916c1eabdc1579fc31e7ff12254df478cc9a14
causes this regression. Before the change, /dev/null is created using
the mknod binary. After the change /dev/null is created (also as a
character device) using os.mknod (Python function). I have no idea why
mknod does not work in these scenarios, but this is not the important
part of my change and more of an drive-by optimization (less forks ->
more speed was the idea). The important part is handling a failure from
mknod and turn it into a bind mount of the original device. From my pov,
the change from /bin/mknod to os.mknod can be reverted, but I'd like to
understand why it breaks stuff and have no luck at understanding nor
reproducing this.

Helmut