Bug#1068382: sbuild: Support tarballs not including ./ when using the unshare chroot mode
Hi, Quoting Santiago Vila (2024-04-04 20:03:08) > El 4/4/24 a las 19:29, Johannes Schauer Marin Rodrigues escribió: > > Also I'm curious: what is your motivation for using unshare mode if you are > > creating your chroots using superuser privileges? > > > > And are you really storing your chroots in /srv instead of letting them get > > picked up automatically in ~/.cache/sbuild/unstable-arm64.tar? > > I am testing the unshare backend at the request of Jochen. I now also talked with Jochen on IRC. :) > As we speak, I am building all packages in bookworm using unshare. Thank you for doing that! I'd assume that quite a few testsuites break in that environment. > I already had a working setup for building packages using the other backends. > In my setup, I run debootstrap in a master server, create the tarballs there, > and the build nodes retrieve those tarballs using wget before they start to > build packages. > > So in my setup the logical thing to do to test the unshare backend > was to make symlinks to the already existing tarballs at /srv, since the > nodes are not creating any tarballs. That makes sense, yes. I think it makes sense for sbuild to accept tarballs without the leading ./. I think the best way to achieve this, is to filter out the device nodes from the input tarball. The utility that I know that can do this is mmtarfilter like this: mmtarfilter --type-exclude CHRTYPE --type-exclude BLKTYPE Unfortunately, that utility is written in Python and sbuild currently does not depend on Python. I wonder if the same can be done in Perl? Maybe I should write a very crude tarball parser that is able to do just enough parsing... signature.asc Description: signature
Bug#1068382: sbuild: Support tarballs not including ./ when using the unshare chroot mode
El 4/4/24 a las 19:29, Johannes Schauer Marin Rodrigues escribió: Also I'm curious: what is your motivation for using unshare mode if you are creating your chroots using superuser privileges? And are you really storing your chroots in /srv instead of letting them get picked up automatically in ~/.cache/sbuild/unstable-arm64.tar? I am testing the unshare backend at the request of Jochen. As we speak, I am building all packages in bookworm using unshare. I already had a working setup for building packages using the other backends. In my setup, I run debootstrap in a master server, create the tarballs there, and the build nodes retrieve those tarballs using wget before they start to build packages. So in my setup the logical thing to do to test the unshare backend was to make symlinks to the already existing tarballs at /srv, since the nodes are not creating any tarballs. Thanks.
Bug#1068382: sbuild: Support tarballs not including ./ when using the unshare chroot mode
Hi, Quoting Santiago Vila (2024-04-04 15:24:13) > > how did you create that tarball? > > debootstrap to a directory > cd /chroot/directory > tar czvf /srv/whatever.tar.gz * > > Yes, I know what using "." instead of "*" would solve the problem, but as I > said, > sbuild already supports perfectly tarballs without ./ in the "file" backend, > so the consistent thing would be to support them for unshare as well. you can do this in less commands by using tar -C /chroot/directory -czvf /srv/whatever.tar.gz . In that case, the "*" wildcard would of course not work which is why usually, people use '.' instead of "*". Another reason against using the glob operator with tar is, that it will exclude hidden files and then you will end up with a tarball that does not contain everything in the current directory. Using '.' is really the safer option independent on whether you use -C or not. Also I'm curious: what is your motivation for using unshare mode if you are creating your chroots using superuser privileges? And are you really storing your chroots in /srv instead of letting them get picked up automatically in ~/.cache/sbuild/unstable-arm64.tar? > > Your addition of --anchored drops support for tarballs with members that > > start with ././ or with ./././ and so on. > > Yes, but those tarballs are a lot more uncommon, so if we had to choose > between > supporting "" and "./" or supporting "./" and "././" and "./././" etc, I guess > supporting "" and "./" would be preferred. > > So, well spotted, but I don't think that dropping support for ././ > would be a big deal. I think those tarballs are even more uncommon than yours, yes. But then you are also the first in six years that the unshare backend existed to have come across that problem. :) > > Your second patch is described as "Do not extract anything in /dev" but > > what it actually excludes is the directory itself and not just everything > > in it. > That's why I said "untested" :-) The point was to convey the idea, not the > implementation. Unfortunately the idea cannot work. Because the point of the exclude patterns is to exclude everything that is a character special file. The point is not to exclude everything in /dev. > > Maybe a better solution would be to pipe the tarballs through mmtarfilter > > and just remove all the device nodes from them. This avoids requiring any > > --exclude options for tar. > > Hmm, but if we get to such point, maybe we should really advocate for > debootstrap and friends to stop including any /dev/* files at all. I'd be very much in favour of doing that. With mmdebstrap you can create a chroot without device nodes by running this: mmdebstrap --variant=buildd --skip=output/mknod unstable ~/.cache/sbuild/unstable-arm64.tar The reason they are created in debootstrap (I think) is so that you can easily just chroot into them without having to think of bind-mounting /dev beforehand. Thanks! cheers, josch signature.asc Description: signature
Bug#1068382: sbuild: Support tarballs not including ./ when using the unshare chroot mode
El 4/4/24 a las 14:22, Johannes Schauer Marin Rodrigues escribió: Since I don't think a tarball without ./ is really "wrong" to the point that it needs to be recreated (this is in fact the very first in my life that a tarball without ./ causes any kind of trouble), I think it would be desirable to support those tarballs as well. how did you create that tarball? debootstrap to a directory cd /chroot/directory tar czvf /srv/whatever.tar.gz * Yes, I know what using "." instead of "*" would solve the problem, but as I said, sbuild already supports perfectly tarballs without ./ in the "file" backend, so the consistent thing would be to support them for unshare as well. So, we (Jochen and myself) wonder if any of the following patches would be acceptable to you. The first patch adds --anchored option to tar invocation so that the exclude patterns are matched from the beginning only (not anywhere in the filename), then adds the remaining eight exclude patterns for tarballs without "./". I could agree that the end result is not very nice, but it's simple, effective, and imo it's not really so much ugly. However, while we are at it, I wonder why it's necessary to uncompress anything in /dev at all these days. Would it work if everything in /dev is excluded? The second patch (untested) supports tarballs with or without ./ and at the same time simplifies the exclude patterns to just two. Your addition of --anchored drops support for tarballs with members that start with ././ or with ./././ and so on. Yes, but those tarballs are a lot more uncommon, so if we had to choose between supporting "" and "./" or supporting "./" and "././" and "./././" etc, I guess supporting "" and "./" would be preferred. So, well spotted, but I don't think that dropping support for ././ would be a big deal. Your second patch is described as "Do not extract anything in /dev" but what it actually excludes is the directory itself and not just everything in it. That's why I said "untested" :-) The point was to convey the idea, not the implementation. Maybe a better solution would be to pipe the tarballs through mmtarfilter and just remove all the device nodes from them. This avoids requiring any --exclude options for tar. Hmm, but if we get to such point, maybe we should really advocate for debootstrap and friends to stop including any /dev/* files at all. Thanks.
Bug#1068382: sbuild: Support tarballs not including ./ when using the unshare chroot mode
Hi, Quoting Santiago Vila (2024-04-04 12:17:14) > While trying to use the unshare backend I found this error: > > tar: dev/full: Cannot mknod: Operation not permitted > tar: dev/urandom: Cannot mknod: Operation not permitted > tar: dev/console: Cannot mknod: Operation not permitted > tar: dev/ptmx: Cannot mknod: Operation not permitted > tar: dev/random: Cannot mknod: Operation not permitted > [...] > > The reason (as Jochen identified quickly) is that my tarball did not > include ./ entries, so the exclude patterns in lib/Sbuild/ChrootUnshare.pm > had no effect. > > Since I don't think a tarball without ./ is really "wrong" to the point > that it needs to be recreated (this is in fact the very first in my life > that a tarball without ./ causes any kind of trouble), I think it would be > desirable to support those tarballs as well. how did you create that tarball? > So, we (Jochen and myself) wonder if any of the following patches would be > acceptable to you. > > The first patch adds --anchored option to tar invocation so that the exclude > patterns are matched from the beginning only (not anywhere in the filename), > then adds the remaining eight exclude patterns for tarballs without "./". > > I could agree that the end result is not very nice, but it's simple, > effective, and imo it's not really so much ugly. > > However, while we are at it, I wonder why it's necessary to uncompress > anything in /dev at all these days. Would it work if everything in /dev is > excluded? > > The second patch (untested) supports tarballs with or without ./ and at the > same time simplifies the exclude patterns to just two. Your addition of --anchored drops support for tarballs with members that start with ././ or with ./././ and so on. Your second patch is described as "Do not extract anything in /dev" but what it actually excludes is the directory itself and not just everything in it. Maybe a better solution would be to pipe the tarballs through mmtarfilter and just remove all the device nodes from them. This avoids requiring any --exclude options for tar. Thanks! cheers, josch signature.asc Description: signature
Bug#1068382: sbuild: Support tarballs not including ./ when using the unshare chroot mode
Package: sbuild
Version: 0.85.6
Severity: wishlist
Dear maintainer:
While trying to use the unshare backend I found this error:
tar: dev/full: Cannot mknod: Operation not permitted
tar: dev/urandom: Cannot mknod: Operation not permitted
tar: dev/console: Cannot mknod: Operation not permitted
tar: dev/ptmx: Cannot mknod: Operation not permitted
tar: dev/random: Cannot mknod: Operation not permitted
[...]
The reason (as Jochen identified quickly) is that my tarball did not
include ./ entries, so the exclude patterns in lib/Sbuild/ChrootUnshare.pm
had no effect.
Since I don't think a tarball without ./ is really "wrong" to the point
that it needs to be recreated (this is in fact the very first in my life
that a tarball without ./ causes any kind of trouble), I think it would be
desirable to support those tarballs as well.
So, we (Jochen and myself) wonder if any of the following patches
would be acceptable to you.
The first patch adds --anchored option to tar invocation so that
the exclude patterns are matched from the beginning only
(not anywhere in the filename), then adds the remaining eight
exclude patterns for tarballs without "./".
I could agree that the end result is not very nice, but it's
simple, effective, and imo it's not really so much ugly.
However, while we are at it, I wonder why it's necessary to
uncompress anything in /dev at all these days. Would it work
if everything in /dev is excluded?
The second patch (untested) supports tarballs with or
without ./ and at the same time simplifies the exclude
patterns to just two.
Thanks.commit d54e3303e4a212a790f736b2b9db072c6fe7b25e
Author: Santiago Vila
Date: Thu Apr 4 11:30:00 2024 +0200
lib/Sbuild/ChrootUnshare.pm:
Use tar's --anchored option to support tarballs including ./ and also those
not including ./
diff --git a/lib/Sbuild/ChrootUnshare.pm b/lib/Sbuild/ChrootUnshare.pm
index 91a7fa43..02d80936 100644
--- a/lib/Sbuild/ChrootUnshare.pm
+++ b/lib/Sbuild/ChrootUnshare.pm
@@ -166,6 +166,15 @@ sub begin_session {
print STDOUT "Unpacking $tarball to $rootdir...\n";
@cmd = (@unshare_cmd, 'tar',
+ '--anchored',
+ '--exclude=dev/urandom',
+ '--exclude=dev/random',
+ '--exclude=dev/full',
+ '--exclude=dev/null',
+ '--exclude=dev/console',
+ '--exclude=dev/zero',
+ '--exclude=dev/tty',
+ '--exclude=dev/ptmx',
'--exclude=./dev/urandom',
'--exclude=./dev/random',
'--exclude=./dev/full',
commit 47a0ed399f17d0cfc2a5af17bccd55880e5c7892
Author: Santiago Vila
Date: Thu Apr 4 11:30:00 2024 +0200
lib/Sbuild/ChrootUnshare.pm:
Do not extract anything in /dev
Use tar's --anchored option to support tarballs including ./ and also those
not including ./
diff --git a/lib/Sbuild/ChrootUnshare.pm b/lib/Sbuild/ChrootUnshare.pm
index 91a7fa43..3e554e9c 100644
--- a/lib/Sbuild/ChrootUnshare.pm
+++ b/lib/Sbuild/ChrootUnshare.pm
@@ -166,14 +166,9 @@ sub begin_session {
print STDOUT "Unpacking $tarball to $rootdir...\n";
@cmd = (@unshare_cmd, 'tar',
- '--exclude=./dev/urandom',
- '--exclude=./dev/random',
- '--exclude=./dev/full',
- '--exclude=./dev/null',
- '--exclude=./dev/console',
- '--exclude=./dev/zero',
- '--exclude=./dev/tty',
- '--exclude=./dev/ptmx',
+ '--anchored',
+ '--exclude=dev/',
+ '--exclude=./dev/',
'--directory', $rootdir,
'--extract'
);
