Please merge sbuild 0.83.1 from Debian unstable

[Dave Jones]

Please merge sbuild 0.83.1 from Debian unstable.

Updated changelog and diff against Debian unstable to be attached below.

[Dave Jones (waveform)]

Attaching patch against Debian unstable. For ease of review, relevant commits
and tags have been pushed to the following repository:

  https://code.launchpad.net/~waveform/ubuntu/+source/sbuild/+git/sbuild

Specifically:

* logical/0.81.2ubuntu6 represents our split-out delta on top of old/debian (0.81.2)
* logical/0.83.1ubuntu1 represents our rebased delta on top of new/debian (0.83.1)
* merge/0.83.1ubuntu1 just adds changelog and maintainer changes on top of logical/0.83.1ubuntu1

Hence, the following command may produce output useful to the purposes of review:

  git range-diff old/debian..logical/0.81.2ubuntu6 new/debian..logical/0.83.1ubuntu1

Test packages are built in:

  https://launchpad.net/~waveform/+archive/ubuntu/sbuild/+packages

[Dave Jones (waveform)]

Oh damn it, the autopkgtests are going to fail (wasn't testing on the right arch and hadn't noticed it skipping; just re-tested on amd64 and of course diffoscope's going to trip up on the stuff that it *thought* was previously testing and now is *actually* testing...).

Will fix in the morning, but I'll hold off on subscribing sponsors 'til this is actually fixed ...

[Dave Jones (waveform)]

Right, let's try this again...

Attaching patch against Debian unstable. For ease of review, relevant commits
and tags have been pushed to the following repository:

  https://code.launchpad.net/~waveform/ubuntu/+source/sbuild/+git/sbuild

Specifically:

* logical/0.81.2ubuntu6 represents our split-out delta on top of old/debian (0.81.2)
* logical/0.83.1ubuntu1 represents our rebased delta on top of new/debian (0.83.1)
* merge/0.83.1ubuntu1 just adds changelog and maintainer changes on top of logical/0.83.1ubuntu1

Hence, the following command may produce output useful to the purposes of review:

  git range-diff old/debian..logical/0.81.2ubuntu6 new/debian..logical/0.83.1ubuntu1

Test packages will be built in:

  https://launchpad.net/~waveform/+archive/ubuntu/sbuild

[Benjamin Drung (bdrung)]

Partial review:
* abs-path-revert.patch: Is there a bug report for it? If no, please file one. If yes, please reference it.
* no-pkg-mangle-deps.patch: plaese forward to Debian. This change does not hurt Debian.
* Can you adjust verify_deb to support gzip and xzst? Then this part can be upstreamed.

[Dave Jones (waveform)]

> * abs-path-revert.patch: Is there a bug report for it? If no, please file one. If yes, please reference it.

abs-path-revert.patch first appears in 2016 in version 0.67.0-2ubuntu2 (https://git.launchpad.net/ubuntu/+source/sbuild/commit/?h=import/0.67.0-2ubuntu2&id=5fbd980fc8930ad11dd4c21b02bbfdbd9816d6c5). Unfortunately there's no bug associated with it. I've opened one (LP: #1976257) but I'm afraid it's rather sparse on detail. I'll add it to the changelog.

> * no-pkg-mangle-deps.patch: plaese forward to Debian. This change does not hurt Debian.

The "Forwarded: not-needed" tag in no-pkg-mangle-deps.patch suggests that, at the very least, Debian are not (or should not be) interested in this patch. It is certainly true that it's setting an env variable that shouldn't affect Debian as pkgbinarymangler is Ubuntu-specific. However, I'd be extremely surprised if Debian would be interested in accepting a change which has the potential to affect them in future without any present justification beyond Ubuntu having a smaller delta.

> * Can you adjust verify_deb to support gzip and xzst? Then this part can be upstreamed.

In this case it isn't a change from gzip to xz (both of which Debian support as compressors for the archives within a deb), but a change from xz to zst. The latter is not currently supported in Debian's dpkg. However, I've reviewed the current status of the relevant threads [1][2] and it seems like zstd is likely to be added to Debian at some point in the fairly near future so it makes sense to have something that fits both Debian and Ubuntu. I'll revise the patch accordingly and re-post.

[1]: https://lists.debian.org/debian-devel/2018/04/msg00653.html
[2]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=892664

[Dave Jones (waveform)]

Okay, the change to check both xz and zstd style debs has kindly been accepted upstream [1] so I'm attaching a revised debdiff (3-1974177.debdiff) and I've updated the branches and tags in comment 1 with the new merge.

[1]: https://salsa.debian.org/debian/sbuild/-/merge_requests/18

[Benjamin Drung (bdrung)]

Thanks.

1) Following line in debian/tests/unshare will break Debian (since the script runs with "set -e"):

[ "$distro" = ubuntu ] && umask 022

Either use || or an if-clause. Besides that, you can just source /etc/os-release and then use the ID variable.

2) Running the unshare autopkgetest with a kinetic schroot fails:

autopkgtest [12:48:47]: test unshare: [-----------------------
+ [ -z x ]
+ grep -q ^1$ /proc/sys/kernel/unprivileged_userns_clone
+ dpkg --print-architecture
[...]
+ mmdebstrap --mode=unshare --variant=apt kinetic /tmp/autopkgtest.4hxHnh/autopkgtest_tmp/chroot.tar
W: unshare syscall failed: Operation not permitted

Does it need more checks or does it need to declare to be better isolated?

[Benjamin Drung (bdrung)]

Scrape my comment 1. This construct still works with "set -e" on Debian.

Besides that, I prepared some merges for Debian:
* Run unshare autopkgtest against target release: https://salsa.debian.org/debian/sbuild/-/merge_requests/19
* Set NO_PKG_MANGLE=1 when building dummy packages: https://salsa.debian.org/debian/sbuild/-/merge_requests/20
* Revert "debian/tests/build-procenv: add deb-src repository": https://salsa.debian.org/debian/sbuild/-/merge_requests/21

[Dave Jones (waveform)]

> 1) Following line in debian/tests/unshare will break Debian (since
> the script runs with "set -e"):
>
> [ "$distro" = ubuntu ] && umask 022
>
> Either use || or an if-clause. Besides that, you can just source
> /etc/os-release and then use the ID variable.

It only breaks Debian if the distro=$(...) line (22) isn't
incorporated (which, were this change to be upstreamed, it should be).
As to sourcing /etc/os-release, that's effectively what it's doing in
that line (admittedly via lsb_release, but that seems to me a
reasonably standard means of querying os-release).

> 2) Running the unshare autopkgtest with a kinetic schroot fails:
>
> autopkgtest [12:48:47]: test unshare: [-----------------------
> + [ -z x ]
> + grep -q ^1$ /proc/sys/kernel/unprivileged_userns_clone
> + dpkg --print-architecture
> [...]
> + mmdebstrap --mode=unshare --variant=apt kinetic /tmp/autopkgtest.4hxHnh/autopkgtest_tmp/chroot.tar
> W: unshare syscall failed: Operation not permitted
>
> Does it need more checks or does it need to declare to be better
> isolated?

Good point, it needs more isolation. Knowing that the unshare-wrapper
test involved isolation-machine I'd only tested under qemu.
Incidentally, that one winds up skipped on Ubuntu because
linux-image-amd64 is not installable (and skip-not-installable is
set), which is presumably why we added the (unwrapped) unshare test in
the first place. Still, that test works happily with a Debian qemu
setup.

I've subsequently tested with LXD as well, and that works with the
(unwrapped) unshare test, but requires a privileged container with
nesting to execute successfully:

$ autopkgtest-build-lxd images:ubuntu/kinetic/amd64
$ autopkgtest . -- lxd autopkgtest/ubuntu/kinetic/amd64 -c security.privileged=true -c security.nesting=true

So perhaps isolation-machine is more appropriate there anyway. I'll
amend d/t/control accordingly.

[Dave Jones (waveform)]

Argh, didn't notice your comment -- and then realized I'd confused set -e with set -u (which I tend to use as well)! Ignore my response to your (ignored) comment 1 :)

[Benjamin Drung (bdrung)]

Can you forward the Ubuntu umask change to Debian?

[Dave Jones (waveform)]

I can try!

[Dave Jones (waveform)]

I've opened an MR [1] with upstream's sbuild to fix the umask difference (and traced the difference in umasks to Debian's common-session PAM configuration lacking pam_umask.so).

Attaching an updated debdiff with the isolation-machine change (this has also been uploaded to the aforementioned sbuild PPA).

[1]: https://salsa.debian.org/debian/sbuild/-/merge_requests/24

[Benjamin Drung (bdrung)]

I'll sponsor this version. Attached a patch with the changes that are not yet submitted to Debian.

[Launchpad Janitor (janitor)]

This bug was fixed in the package sbuild - 0.83.1ubuntu1

---------------
sbuild (0.83.1ubuntu1) kinetic; urgency=medium

  * Merge from Debian unstable (LP: #1974177). Remaining changes:
    - d/t/control: add isolation-machine to the test flags
    - no-pkg-mangle-deps.patch: Set NO_PKG_MANGLE=1 when building dummy
      packages, as pkgbinarymangler's dpkg-deb expects to be run from a source
      package.
    - abs-path-revert.patch: Revert upstream commit that breaks lp-buildd by
      causing symlinks to files not ending in .dsc to no longer be buildable.
      (LP: #1976257)
    - debian/tests/control: Skip unshare-qemuwrapper test where linux-image-
      amd64 is missing. It is missing in Ubuntu making the test fail.
    - debian/tests/unshare: Run the test, but skip it if user namespaces are
      not supported
    - debian/tests/unshare*: Test with current release
  * Removed obsolete patches/changes:
    - debian/tests/control: Don't depend on linux-image-amd64 package
      It does not exist on Ubuntu.
    - debian/tests/unshare-qemuwrapper: Don't use linux-image-amd64 package in
      bootrstrapping test on Ubuntu
    - debian/tests/unshare-qemuwrapper: Drop delta
  * Fix the unshare tests
    - Make dscverify available by adding devscripts to EXTRA_DEPS
    - Ensure dscverify tests actually run by re-writing verify()
    - Remove the release=unstable check (there's no reason we shouldn't
      check the orig-tarball and deb output)
    - unshare test requires isolation-machine to execute
  * Fix build-procenv to work under Ubuntu by removing unstable deb-src
    reference
  * Fix the diffoscope tests
    - Change the expected .deb output to match xz or zstd compression
    - Use "unstable" as d/changelog release name so the test can remain
      static across releases
    - Patch umask differences otherwise diffoscope barfs on differing group
      modes in the generated orig-tar

sbuild (0.83.1) unstable; urgency=medium

  [ Johannes Schauer Marin Rodrigues ]
  * bin/sbuild-cross-resolver: we don't need the English module (closes:
    #1005957)
  * debian/tests/unshare: run with -x
  * lib/Sbuild/Utility.pm: add warning about missing libwww-perl (closes:
    #1009859)

  [ Jochen Sprickerhof ]
  * lib/Sbuild/ChrootUnshare.pm: mkdir for bind mounts

  [ Daniel Kahn Gillmor ]
  * Use --batch with gpg when importing secret key (Closes: #1010171)

sbuild (0.83.0) unstable; urgency=medium

  * sbuild-qemu (package):
    - Support for new architectures: armhf, arm64, ppc64el
    - Move vmdb2 to Depends
    - Depend on autopkgtest (>= 5.17~)
  * sbuild-qemu:
    - Fix looking for images when --arch was specified
    - sbuild-qemu: rename --ram to --ram-size. This matches the
      autopkgtest-virt-qemu option
    - Reduce default CPU count to 2, rather than equal to host cores. This is
      more likely to work on all platforms
  * sbuild-qemu-create:
    - Update shared dir setup (deprecated QEMU syntax). Existing images will no
      longer work and will need to be recreated
  * sbuild-qemu-update:
    - Fix QEMU launch issues (network, serial)
    - Reduce memory requirement. 1GiB is sufficient for apt-get operations
  * sbuild-qe...