Please merge sbuild 0.85.0 from Debian unstable.

[Dave Jones]

Please merge sbuild 0.85.0 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.84.1ubuntu3 represents our split-out delta on
  top of old/debian (0.84.1)
* logical/0.85.0ubuntu1 represents our rebased delta on
  top of new/debian (0.85.0)
* merge/0.85.0ubuntu1 represents the re-worked delta based
  on several upstream patches

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

git range-diff --creation-factor=95 old/debian..logical/0.84.1ubuntu3 new/debian..merge/0.85.0ubuntu1

At this point, all elements of the delta are currently represented
by MRs in https://salsa.debian.org/debian/sbuild/-/merge_requests
and thus the merge is likely to become a sync at some point in the
future (many thanks to bdrung for all the upstreaming work).

[Dave Jones (waveform)]

Please be aware I've been unable to get autopkgtest passing locally, so this is *somewhat* untested (specifically the build-procenv test works, but the unshare and unshare-qemuwrapper tests didn't work as the autopkgtest VM environment was unable to install dpkg for some reason).

[Simon Quigley (tsimonq2)]

Hi Dave!

I'm willing to give this a try, and sponsor any additional uploads if needed.

In the changelog I made two minor modifications:
 - I removed the newline in between the names and their respective changelog entries. This may be personal preference, but I've never seen a newline separating those before.
 - "Removed patches obsoleted/merged by upstream:" and "Removed obsolete patches:" are certainly redundant, I've removed the latter.

Thanks!

[Dave Jones (waveform)]

> I'm willing to give this a try, and sponsor any additional uploads if needed.

Thanks!

> In the changelog I made two minor modifications:
>  - I removed the newline in between the names and their respective changelog entries. This may be personal preference, but I've never seen a newline separating those before.

Doh -- not sure what I was thinking there

> - "Removed patches obsoleted/merged by upstream:" and "Removed obsolete patches:" are certainly redundant, I've removed the latter.

Given Debian is the upstream the difference is certainly nebulous, but I could make a (weak) argument about whether Debian's action or our own had removed the need for the patch. Sometimes makes a difference in the more complex merges which is why the two headings are part of my merge template (which was originally copied from ... somewhere I've forgotten). Anyway, perfectly happy for people to simplify!

[Simon Quigley (tsimonq2)]

Hi Dave!

You were right, the autopkgtests don't pass on Lunar at the moment.

Is this something you're actively working on, or would you like me to step in and take a look?

Thanks again!

[Dave Jones (waveform)]

Hi Simon -- I've tried to find some time to look at this, but I've failed this week! If you want to have a crack at it, please do :)

[Dave Jones (waveform)]

I found a bit of time to dig further into this and I'm suspicious that in the diffoscope output (at the end of [1]) only the header and footer of the zstd output has changed, but not the compressed content. To that end I went and dug into what changed in libzstd between kinetic and lunar and it seems that while the version of libzstd remained the same, the build-system changed (from traditional Makefile to cmake).

In the process I'm suspicious that there appears to be an error in d/rules [2]. Lines 18-26 define DH_AUTO_CONFIGURE_OPTS:

DH_AUTO_CONFIGURE_OPTS := \
 -DCMAKE_LIBRARY_ARCHITECTURE=${arch} \
 -DZSTD_LEGACY_SUPPORT:BOOL=ON \

ifneq (,$(filter nocheck,$(DEB_BUILD_OPTIONS)))
 DH_AUTO_CONFIGURE_OPTS += -DBUILD_TESTING:BOOL=OFF
else
 DH_AUTO_CONFIGURE_OPTS += -DBUILD_TESTING:BOOL=ON
endif

However, the call to dh_auto_configure on line 44 uses DH_CMAKE_CONFIGURE_OPTS instead (which doesn't appear to be defined anywhere):

 dh_auto_configure \
  --buildsystem=cmake \
  --sourcedir=$(CURDIR)/build/cmake \
  -- \
  $(DH_CMAKE_CONFIGURE_OPTS)

This is relatively compelling because ZSTD_LEGACY_SUPPORT defines whether zstd can deal with "legacy frames" [3], and it appears to be a difference in the frame output that diffoscope is noticing here (although it would seem from the linked document that it has to do more with *decoding* those frames than producing them ... but let's take a look anyway).

Unfortunately, in both cases (looking at the build-logs for kinetic [4] and lunar [5]) it appears ZSTD_LEGACY_SUPPORT winds up at its default of "5" anyway so theoretically they should be the same. Just to be certain, I re-built libzstd with DH_CMAKE_CONFIGURE_OPTS replaced by DH_AUTO_CONFIGURE_OPTS and re-ran the autopkgtest. As suggested by the build logs, the same failure occurred. Regardless, I've filed the change upstream as a merge request [6] because I suspect it's wrong anyway.

Next step is to dig into what the byte-level changes *actually* mean (I started working on this but my findings so far don't make a lot of sense).

[1]: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar/lunar/arm64/s/sbuild/20230204_040321_7a8f5@/log.gz

[2]: https://salsa.debian.org/pkg-rpm-team/libzstd/-/blob/4d32f1100aff4a2e5d0dc41b27ef4f7156da18c3/debian/rules

[3]: https://fuchsia.googlesource.com/third_party/zstd/+/refs/tags/v1.4.4/programs/README.md

[4]: https://launchpadlibrarian.net/599067195/buildlog_ubuntu-kinetic-amd64.libzstd_1.5.2+dfsg-1_BUILDING.txt.gz

[5]: https://launchpadlibrarian.net/643274654/buildlog_ubuntu-lunar-amd64.libzstd_1.5.2+dfsg2-3_BUILDING.txt.gz

[6]: https://salsa.debian.org/pkg-rpm-team/libzstd/-/merge_requests/1

[Dave Jones (waveform)]

Oh except, of course, my test-run with autopkgtest didn't actually use the libzstd1 I was injecting because it's building its own environment within the test environment. Might be easier to just test-build debs with different libzstd1 versions directly...

[Dave Jones (waveform)]

Confirmed this is due to changes in the output of libzstd1. The current variant of libzstd1 in lunar (1.5.4) produces entirely different output in the diffoscope diff of the generated deb. Compare the diffoscope output in [1] to [2]. I reproduced these differences in an otherwise up to date lunar VM by downgrading the libzstd1 package from 1.5.4+dfsg2-4 to 1.5.2+dfsg-1.

[1]: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar/lunar/arm64/s/sbuild/20230204_040321_7a8f5@/log.gz

[2]: https://autopkgtest.ubuntu.com/results/autopkgtest-lunar/lunar/arm64/s/sbuild/20230320_055722_83889@/log.gz

I theorize, that the last successful version was 1.4.8+dfsg-3build1 (last seen in kinetic and current in jammy). However I can't currently test with that as dpkg has depended on libzstd1 >= 1.5.2 for a while now.

Anyway, the crux of the problem is that the unshare test is checking for binary reproducibility of output, and zstd doesn't guarantee that (does any compressor guarantee that, or have we just got lucky so far?). Solutions:

1) Drop the diffoscope part of the unshare test

2) Update the diffoscope part of the unshare test to the "current zstd output" (and presumably keep doing this with each new zstd version)

3) Find some way of beating zstd into producing the same output as before (if that's possible)

The first is easiest and quickest but nasty, the second still fairly quick'n'easy but potentially painful in future (and seems somewhat against the spirit of the test anyway). The third is potentially impossible and if possible, potentially hard.

Incidentally, all of the above presumably apply to Debian too now zstd deb support has made its way upstream.

Comments?

[Dave Jones (waveform)]

After some discussion with the team it appears the only sensible option here is 2: update the representation of the test deb within the unshare test. The attached debdiff applies this and fixes a couple of other lintian errors (obsolete dependencies); I've tested this locally in a lunar VM successfully, and a test package is building in ppa:waveform/sbuild (https://launchpad.net/~waveform/+archive/ubuntu/sbuild) if anyone else wants a try.

I'll forward all these changes up to Debian, but if someone could have a look at sponsoring this in the meantime, I'd be most grateful.

[Dave Jones (waveform)]

Hmm, let's try that again with the correct diff...

Note to self: stop using debdiff!

[Benjamin Drung (bdrung)]

Sponsored 0.85.0ubuntu2 upload.

[Launchpad Janitor (janitor)]

This bug was fixed in the package sbuild - 0.85.0ubuntu2

---------------
sbuild (0.85.0ubuntu2) lunar; urgency=medium

  * d/t/unshare: Updated test-pkg_1.0_all.deb content for zstd to match
    output from zstd 1.5.4
  * d/t/control: Fix obsolete dependency on lsb-release (should now be
    lsb-release-minimal)
  * d/control: Fix obsolete dependency on lsb-base (should now be
    sysvinit-utils)
  * Bump standards version to 4.6.2

sbuild (0.85.0ubuntu1) lunar; urgency=medium

  [ Dave Jones ]
  * Merge from Debian unstable (LP: #2003201). Remaining changes:
    - Fix autopkgtest `unshare-qemuwrapper` on lunar-proposed
      - d/t/unshare-qemuwrapper: copy apt preferences into the vm's chroot.
      - d/t/unshare: use mmdebstrap --verbose to aid debugging.
  * Removed patches obsoleted/merged by upstream:
    - 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.
    - d/t/build-procenv: Make skippable when mknod fails, as in unprivileged
      container case (as Ubuntu's autopkgtest uses in the armhf case)
    - debian/tests/unshare: Run the test, but skip it if user namespaces are
      not supported
    - unshare test requires isolation-machine to execute

  [ Benjamin Drung ]
  * d/t/get_default_release.py: Rely on VERSION_CODENAME from /etc/os-release
    on non-Debian systems if the variable is set.
  * d/t/check-unshare-support: Add a check for unshare support. If unshare is
    supported, run the unshare autopkgtest directly. Otherwise run
    unshare-qemuwrapper.

sbuild (0.85.0) unstable; urgency=medium

  [ Johannes Schauer Marin Rodrigues ]
  * lib/Sbuild/Conf.pm: replace deprecated
    Dpkg::Build::Info::get_build_env_allowed by
    Dpkg::BuildInfo::get_build_env_allowed (Closes: #1027719)
  * debian/control: bump libdpkg-perl dep to 1.21.14 because of
    Dpkg::BuildInfo::get_build_env_allowed
  * lib/Sbuild/ResolverBase.pm: copying debs can take a long time -- output log
    messages
  * the sbuild dummy repository should not be considered to create a deb-src
    entry automatically
  * lib/Sbuild/ChrootUnshare.pm: guard against empty /etc/subuid and
    /etc/subgid
  * debian/tests/unshare: uncompress source package before comparing to ignore
    differences in the compression (dpkg recently started using threaded xz
    compression for source packages)
  * add get_default_release.py to figure out the archive used by the currently
    running system
  * debian/tests/unshare: put common sbuild invocation into a function
  * debian/tests/unshare: do not run apt upgrade nor apt update
  * debian/tests/control: add dependency on python3-apt
  * debian/tests/unshare-qemuwrapper: do not cleanup apt list files or
    otherwise we have to call apt update again later
  * lib/Sbuild/Build.pm: reduce number of self->get_conf() calls
  * debian/control: add debootstrap as dependency of
    sbuild-debian-developer-setup
  * bin/sbuild-createchroot: set MERGED_USR to 'auto' letting debootstrap
    decide
  * bin/sbuild-debian-developer-setup: inform the user where the chroot is and
    that the current user has already been added to the sbuild ...