Package: sbuild
Version: 0.79.0-1ubuntu1
Severity: normal
Tags: patch
Dear Maintainer,
While working on merging the current sbuild from Debian unstable to
Ubuntu I noticed something a bit odd in the "unshare" test (both in our
delta with Debian, but also in the Debian original):
At the top of verify() in unshare [1], the routine attempts to assign
its arguments to some variables:
verify_src="${1+no}"
verify_bin="${1+no}"
Unfortunately, there's a couple of errors here. Firstly only the first
argument is used ($2 never gets a look in). But secondly, the "+"
expansion ("use alternative value") means that, if $1 is "" the result
is "" but otherwise the result is "no". So verify_src and verify_bin can
only *ever* be "no"; they can never be "yes". This means the dscverify
sections later on never get called.
Our delta in Ubuntu compounds these mistakes by also making the orig and
deb sections optional and ... also using $1 and the "+" expansion [2].
Oops!
Firstly, instead of relying on a bunch of boolean yes/no parameters I
think it might be nicer to simply list the things we'd like verify to
check (e.g. "verify orig deb dsc"), which I've done in a local branch.
Once that change was in place I also discovered that dscverify still
never ran because it never gets installed in the system setup by the
qemu-wrapper script because devscripts is missing from EXTRA_DEPS there.
Finally, once this was all working I discovered that the .dsc file
should *not* be checked when sbuild is run *without* the "--source"
option because, although the .dsc file is produced in this case, it
isn't signed (presumably because it doesn't get listed in the .changes)
file. To fix this I separated the .dsc and .changes checks into their
own sub-routines and modified the calls to verify() accordingly.
I'm attaching a patch which fixes the issue, but once I've got a bug
number I'll propose the changes as an MR on salsa for convenience.
[1]:
https://salsa.debian.org/debian/sbuild/-/blob/main/debian/tests/unshare#L71
[2]:
https://git.launchpad.net/ubuntu/+source/sbuild/tree/debian/tests/unshare?h=applied/ubuntu/devel&id=23ce4fa3e9bbe83ca70a23224c2c3f8829078227#n80
Best regards,
Dave Jones.
diff --git a/debian/tests/unshare b/debian/tests/unshare
index 2c45a1fe..b9013eb1 100755
--- a/debian/tests/unshare
+++ b/debian/tests/unshare
@@ -20,6 +20,7 @@ chmod 700 "${AUTOPKGTEST_TMP}/gpghome"
export GNUPGHOME="${AUTOPKGTEST_TMP}/gpghome"
verify_orig() {
+ echo "verifying test-pkg_1.0.tar.xz" >&2
cat << END | base64 -d > "${AUTOPKGTEST_TMP}/expected"
/Td6WFoAAATm1rRGAgAhARYAAAB0L+Wj4Cf/BBZdADoZSs4dfiUjFYSOxzYxnd+/m6AlVEVOGf2j
nT6NK0F9XZ7LLydbY3I//WjMOM2RFpGUqZ8R8Q8lLmydB5SLN5ZQSPW3OJjHlzxVQmv2v3KUyPxo
@@ -47,6 +48,7 @@ END
}
verify_deb() {
+ echo "verifying test-pkg_1.0_all.deb" >&2
cat << END | base64 -d > "${AUTOPKGTEST_TMP}/expected"
ITxhcmNoPgpkZWJpYW4tYmluYXJ5ICAgMTQ2NzMxMDUxMiAgMCAgICAgMCAgICAgMTAwNjQ0ICA0
ICAgICAgICAgYAoyLjAKY29udHJvbC50YXIueHogIDE0NjczMTA1MTIgIDAgICAgIDAgICAgIDEw
@@ -68,26 +70,31 @@ END
rm "${AUTOPKGTEST_TMP}/expected"
}
-verify() {
- verify_src="${1+no}"
- verify_bin="${1+no}"
- echo "verifying test-pkg_1.0.tar.xz" >&2
- verify_orig
- echo "verifying test-pkg_1.0_all.deb" >&2
- verify_deb
+verify_dsc() {
# we shouldn't have to manually pass the keyring because the path is an
# implementation detail of gnupg (it used to be named pubring.gpg in
# the past) but dscverify ignores GNUPGHOME, see Debian bug #981008
- if [ "$verify_bin" = "yes" ]; then
- echo "verifying test-pkg_1.0.dsc" >&2
- dscverify --keyring="${AUTOPKGTEST_TMP}/gpghome/pubring.kbx"
"${AUTOPKGTEST_TMP}/test-pkg_1.0.dsc"
- echo "verifying test-pkg_1.0_${nativearch}.changes" >&2
- dscverify --keyring="${AUTOPKGTEST_TMP}/gpghome/pubring.kbx"
"${AUTOPKGTEST_TMP}/test-pkg_1.0_${nativearch}.changes"
- fi
- if [ "$verify_src" = "yes" ]; then
- echo "verifying test-pkg_1.0_source.changes" >&2
- dscverify --keyring="${AUTOPKGTEST_TMP}/gpghome/pubring.kbx"
"${AUTOPKGTEST_TMP}/test-pkg_1.0_source.changes"
- fi
+ echo "verifying test-pkg_1.0.dsc" >&2
+ dscverify --keyring="${AUTOPKGTEST_TMP}/gpghome/pubring.kbx" \
+ "${AUTOPKGTEST_TMP}/test-pkg_1.0.dsc"
+}
+
+verify_bin_changes() {
+ echo "verifying test-pkg_1.0_${nativearch}.changes" >&2
+ dscverify --keyring="${AUTOPKGTEST_TMP}/gpghome/pubring.kbx" \
+ "${AUTOPKGTEST_TMP}/test-pkg_1.0_${nativearch}.changes"
+}
+
+verify_src_changes() {
+ echo "verifying test-pkg_1.0_source.changes" >&2
+ dscverify --keyring="${AUTOPKGTEST_TMP}/gpghome/pubring.kbx" \
+ "${AUTOPKGTEST_TMP}/test-pkg_1.0_source.changes"
+}
+
+verify() {
+ for thing in $*; do
+ verify_$thing
+ done
# remove verified files, so that we make sure not to accidentally
# verify anything from an earlier build
rm "${AUTOPKGTEST_TMP}/test-pkg_1.0_all.deb" \
@@ -211,7 +218,7 @@ mmdebstrap --mode=unshare --variant=apt unstable
"${AUTOPKGTEST_TMP}/chroot.tar"
env --chdir="${AUTOPKGTEST_TMP}/test-pkg-1.0/" dpkg-buildpackage --build=full
env --chdir="${AUTOPKGTEST_TMP}/test-pkg-1.0/" dpkg-buildpackage --target=clean
-verify no yes
+verify orig deb dsc bin_changes
# FIXME use installed sbuild
@@ -221,13 +228,13 @@ env --chdir="${AUTOPKGTEST_TMP}/test-pkg-1.0/" sbuild \
--keyid="sbuild fake uploader <fake-uploa...@debian.org>" \
--source \
--no-run-lintian --no-run-autopkgtest
-verify no yes
+verify orig deb dsc bin_changes
env --chdir="${AUTOPKGTEST_TMP}/test-pkg-1.0/" sbuild \
--chroot="${AUTOPKGTEST_TMP}/chroot.tar" --chroot-mode=unshare \
--keyid="sbuild fake uploader <fake-uploa...@debian.org>" \
--no-run-lintian --no-run-autopkgtest
-verify no yes
+verify orig deb bin_changes
# Test running sbuild on the dsc
env --chdir="${AUTOPKGTEST_TMP}/test-pkg-1.0/" dpkg-source --build .
@@ -236,14 +243,14 @@ env --chdir="${AUTOPKGTEST_TMP}" sbuild \
--keyid="sbuild fake uploader <fake-uploa...@debian.org>" \
--source \
--no-run-lintian --no-run-autopkgtest -d unstable test-pkg_1.0.dsc
-verify no yes
+verify orig deb dsc bin_changes
env --chdir="${AUTOPKGTEST_TMP}/test-pkg-1.0/" dpkg-source --build .
env --chdir="${AUTOPKGTEST_TMP}" sbuild \
--chroot="${AUTOPKGTEST_TMP}/chroot.tar" --chroot-mode=unshare \
--keyid="sbuild fake uploader <fake-uploa...@debian.org>" \
--no-run-lintian --no-run-autopkgtest -d unstable test-pkg_1.0.dsc
-verify no yes
+verify orig deb bin_changes
rm "${AUTOPKGTEST_TMP}/test-pkg_1.0_${nativearch}"*.build
diff --git a/debian/tests/unshare-qemuwrapper b/debian/tests/unshare-qemuwrapper
index e7e202c6..00e9691f 100755
--- a/debian/tests/unshare-qemuwrapper
+++ b/debian/tests/unshare-qemuwrapper
@@ -24,7 +24,7 @@
set -exu
-EXTRA_DEPS=gnupg,sbuild,mmdebstrap,build-essential,uidmap,fakeroot,diffoscope
+EXTRA_DEPS=gnupg,sbuild,mmdebstrap,build-essential,uidmap,fakeroot,diffoscope,devscripts
SCRIPT=./debian/tests/unshare
[ -e debian/tests/control ]
