Hi Sam,
TL;DR: My -9.1 NMU should fix pam. Post mortem.
On Tue, Oct 24, 2023 at 09:25:02AM -0600, Sam Hartman wrote:
> >>>>> "Helmut" == Helmut Grohne <hel...@subdivi.de> writes:
>
> Helmut> pam fails to build from source in unstable, because quilt no
> Helmut> longer recognizes the QUILT_PATCHES_DIR variable and
> Helmut> therefore does not find a series file. Renaming it to
> Helmut> QUILT_PATCHES fixes the build.
>
> I applied your patch, and it broke everything. Shame on me for not
> testing, but I think your analysis is wrong. As far as I can tell
> dh_quilt_patch sets QUILT_PATCHES from QUILT_PATCH_DIR, which is what
> pam's debian/rules sets.
I am very sorry for not having tested it either and thus having caused
this mess. I fully agree that my original analysis is completely wrong.
> In particular, your patch causes dh_quilt_patch to think there are no
> patches and to apply no patches.
Yes and this causes things such as su or logging in to no longer work,
i.e. very bad.
> All this mess will go away with the PAM 1.5.3 packaging: I'm moving to a
> normal debian/patches and dpkg will handle things.
Glad to hear that.
> Can I get you to look at this in more depth and help me understand
>
> 1) Whether the buildhs in unstable are really broken (I think they are
> not)
>
> 2) What was broken about your builds that caused you to raise the issue.
The -7 package of pam really does FTBFS in plain unstable. You reverted
the broken -8 upload with a -9 upload that has the same source as -7 and
that -9 upload failed to build on buildds:
https://buildd.debian.org/status/fetch.php?pkg=pam&arch=amd64&ver=1.5.2-9&stamp=1698164563&raw=0
| dpkg-buildpackage: info: source package pam
| dpkg-buildpackage: info: source version 1.5.2-9
| dpkg-buildpackage: info: source distribution unstable
| dpkg-source --before-build .
| dpkg-buildpackage: info: host architecture amd64
| fakeroot debian/rules clean
| dh clean --with quilt,autoreconf
| dh_quilt_unpatch
| No series file found
| dh_quilt_unpatch: error: quilt --quiltrc /dev/null pop -a || test $? = 2
returned exit code 1
| make: *** [debian/rules:22: clean] Error 25
| dpkg-buildpackage: error: fakeroot debian/rules clean subprocess returned
exit status 2
This also means that your -9 upload does not fix people's machines. You
agreed to me NMUing pam on IRC and therefore I uploaded a -9.1. The
corresponding .debdiff is attached to this mail.
I had a vague memory that quilt changed recently and indeed, I filed a
similar FTBFS for libxau #1030781 earlier. I also knew that libxau now
did build, so I concluded that the problem would be with pam rather than
quilt this time and since I happened to not remember QUILT_PATCH_DIR,
but QUILT_PATCHES I tried swapping it and that happened to "work"
(except it broke everything).
Quite obviously, we need to dig deeper. So dh_quilt_unpatch fails with
exit code 1 here. This feels related to what happened in quilt where the
changelog is talking a lot about those exits, but for some reason it
still gets it wrong here. Invoking tools at a lower level
QUILT_PATCHES=debian/patches-applied quilt push
also gives "No series file found" and this is getting fishy. Quite
clearly, debian/patches-applied/series does exist. Instead, quilt is
looking at debian/patches, just why? As this is a "3.0 (quilt)" source
package, we already have a .pc directory and this has a
.pc/.quilt_patches pointing at debian/patches. Now when quilt sees this,
it ignores QUILT_PATCHES and uses the thing from .pc/.quilt_patches
instead and then we know what happens. I don't quite understand why this
happens. If you unpack pam on older releases, you also get this
.pc/.quilt_patches pointing at debian/patches, so my guess is that this
is another behaviour change in quilt.
Anyway, the way pam does this is quite unique. Claiming to be a "3.0
(quilt)" package and then using quilt in a different way is not exactly
surprising to cause strange things to happen and this probably is the
reason for why you want to move to a normal debian/patches. So what my
upload does here is check for a .pc directory pointing at debian/patches
right before running dh_quilt_unpatch and deleting such a directory
(since we know that there is no debian/patches in pam yet). Once we do
that, dh_quilt_unpatch recognizes the real series file and just works.
>From there the build proceeds as usual and I was able to use
autopkgtests to see that -8 is broken and -9.1 has passing autopkgtests
(thank you). I'm therefore hopeful that this -9.1 upload significantly
improves the situation. Of course, you'll have to remove this workaround
once you move move patches to debian/patches.
Again, sorry for the breakage. I hope that Tobias can complete my
post-mortem with a quilt perspective.
Helmut
diff --minimal -Nru pam-1.5.2/debian/changelog pam-1.5.2/debian/changelog
--- pam-1.5.2/debian/changelog 2023-10-24 17:19:43.000000000 +0200
+++ pam-1.5.2/debian/changelog 2023-10-24 19:38:53.000000000 +0200
@@ -1,3 +1,16 @@
+pam (1.5.2-9.1) unstable; urgency=medium
+
+ * Non-maintainer upload acked by Sam Hartman.
+ * Really fix quilt-related FTBFS: (Closes: #1054505)
+ pam is a 3.0 (quilt) source package and has a .pc directory after unpack
+ despite having no debian/patches. Even when setting QUILT_PATCH_DIR or
+ QUILT_PATCHES, quilt is now mislead to using the non-existent
+ debian/patches and this makes dh_quilt_unpatch fail, so we delete that
+ directory unless it corresponds to the real debian/patches-applied that we
+ want to be used.
+
+ -- Helmut Grohne <hel...@subdivi.de> Tue, 24 Oct 2023 19:38:53 +0200
+
pam (1.5.2-9) unstable; urgency=low
* Revert 1.5.2-8 upload; as far as I can tell the change is incorrect,
diff --minimal -Nru pam-1.5.2/debian/rules pam-1.5.2/debian/rules
--- pam-1.5.2/debian/rules 2023-10-24 17:19:43.000000000 +0200
+++ pam-1.5.2/debian/rules 2023-10-24 19:38:53.000000000 +0200
@@ -21,6 +21,12 @@
%:
dh $@ --with quilt,autoreconf
+# Since this is a 3.0 (quilt) source package, it has a .pc directory pointing
+# to debian/patches after initial unpack. This misleads quilt and makes
+# dh_quilt_unpatch fail hard.
+execute_before_dh_quilt_unpatch:
+ if test "`cat .pc/.quilt_patches 2>/dev/null`" = debian/patches; then
rm -Rf .pc; fi
+
# avoid libaudit-dev when bootstrapping
ifneq (,$(filter stage1,$(DEB_BUILD_PROFILES)))
CONFIGURE_OPTS += --disable-audit
