dpkg 1.19.0.5ubuntu2.2 build did not recreate 'configure' file, losing changes in 'configure.ac'

After discussion with cjwatson in #ubuntu-devel, it looks like this is just a dpkg-specific packaging/build-rules issue; by default, packages will use dh_autoreconf, which will call autoreconf -f to force (re)creation of the configure file. The dpkg package has its own debian/rules call to autoreconf, however, and it doesn't pass -f.

This has been fixed (maybe unintentionally) upstream by commit c72f539b979a0c8647d2a6c62ee45565cd243b3d which moves the call to autoreconf into an 'autogen' file, but also more importantly changes the call from 'autoreconf -v -i' to 'autoreconf -f -i'.

> This has been fixed (maybe unintentionally) upstream

actually I'm not sure if that's true, as the call to autoreconf might not happen at all due to the explicit debian/rules rule for 'configure'. Still investigating.

This bug was fixed in the package dpkg - 1.19.7ubuntu2

---------------
dpkg (1.19.7ubuntu2) eoan; urgency=medium

  * d/rules: always run dh_autoreconf (LP: #1842947)

 -- Dan Streetman <email address hidden> Thu, 05 Sep 2019 17:05:14 -0400

> [impact]

> [...]

> However at version 1.19.0.5ubuntu2.2, the 'configure' file was not recreated during build. Thus, dpkg was not built linked against libzstd.

Please could you explain, for SRU purposes, why not building dpkg linked with libzstd is a problem for stable releases? I have little doubt that it is, but to review this SRU I need to know why. This isn't immediately obvious to me and so I think it should be documented here; otherwise an SRU is not justified.

> [regression potential]

> TBD

Please also complete this section.

> Please also complete this section.

sorry, forgot that, updated it

> Please could you explain, for SRU purposes, why not building dpkg linked with libzstd is a problem for stable releases

updated impact section with explanation

Hello Dan, or anyone else affected,

Accepted dpkg into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/dpkg/1.19.0.5ubuntu2.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

libzstd support is not enabled in xenial, thus there has been no regression in SRU; is there other evidence that the configure script used at build time in xenial is out of date and warrants an SRU to that release?

For disco, it appears this is a latent bug and some FUTURE SRU of dpkg could trip over the problem. This seems like a candidate for accepting but using the block-proposed-disco tag.

Hello Dan, or anyone else affected,

Accepted dpkg into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/dpkg/1.19.6ubuntu1.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

> libzstd support is not enabled in xenial, thus there has been no regression in SRU; is there other evidence that the configure script used at build time in xenial is out of date and warrants an SRU to that release?

not that i'm aware of, as you said in comment 9 this is a danger to future srus.

All autopkgtests for the newly accepted dpkg (1.19.0.5ubuntu2.3) for bionic have finished running.
The following regressions have been reported in tests triggered by the package:

apport/2.20.9-0ubuntu7.7 (i386, amd64)
haproxy/1.8.8-1ubuntu0.4 (armhf)
apache2/2.4.29-1ubuntu4.10 (armhf)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/bionic/update_excuses.html#dpkg

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

All autopkgtests for the newly accepted dpkg (1.19.6ubuntu1.2) for disco have finished running.
The following regressions have been reported in tests triggered by the package:

autopkgtest/5.10ubuntu1 (i386, amd64)
lintian/2.12.0 (ppc64el)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/disco/update_excuses.html#dpkg

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

verified that 'dh_autoreconf' was run during bionic and disco builds, e.g.:

https://launchpadlibrarian.net/442816059/buildlog_ubuntu-bionic-amd64.dpkg_1.19.0.5ubuntu2.3_BUILDING.txt.gz

https://launchpadlibrarian.net/442816498/buildlog_ubuntu-disco-amd64.dpkg_1.19.6ubuntu1.2_BUILDING.txt.gz

autopkgtest analysis:

bionic all tests pass; on disco, only 'autopkgtest' fails for amd64/i386, due to bug 1845037, and that failure began before this package was uploaded so is unrelated, and seems to be a bug in a recent snap release of lxd.

This bug was fixed in the package dpkg - 1.19.0.5ubuntu2.3

---------------
dpkg (1.19.0.5ubuntu2.3) bionic; urgency=medium

  * d/rules: always run dh_autoreconf (LP: #1842947)

 -- Dan Streetman <email address hidden> Thu, 05 Sep 2019 17:05:14 -0400

The verification of the Stable Release Update for dpkg has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

> not that i'm aware of, as you said in comment 9 this is a danger to future srus.

Only if the uploader runs autoreconf manually, right? IOW, it won't happen by accident?

If so, then I'm not sure it makes sense to upload this to Xenial, even with block-proposed-xenial. Say for example that a critical vulnerability is discovered and the security team need to patch it urgently (seems unlikely for dpkg, but let's go with it). If we accept this SRU then an undetected regression introduced by running autoreconf would have been staged, the security team would base on that, and then it'd get released, hitting users at large. OTOH, if we do nothing now, then the security team will likely be able to patch without running autoreconf, and any latent regression activated by running autoreconf will not get released because autoreconf didn't get run.

Only if a future update requires autoreconf (or autoreconf gets run at upload time without knowledge of this issue) at upload stage would this SRU be beneficial. That seems unlikely to me, given that we don't intend to make feature changes to dpkg.

Therefore, I'm not sure that an SRU to Xenial makes sense.

What am I missing?

> > not that i'm aware of, as you said in comment 9 this is a danger to future srus.
>
> Only if the uploader runs autoreconf manually, right? IOW, it won't happen by accident?

No, running autoreconf manually (and including the updated configure file in the deb source) would actually avoid this problem.

The problem is when the configure.ac file is updated, but the configure file isn't. If make thinks the configure file is up to date then it won't run autoreconf and changes to the configure.ac file won't be picked up in the build. For example running 'touch configure' before dpkg-buildpackage -S, or maybe running 'cp -r' if that somehow left the 'configure' file timestsamp newer than the 'configure.ac' file timestamp.

> If we accept this SRU then an undetected regression introduced by running autoreconf
> would have been staged, the security team would base on that, and then it'd get
> released, hitting users at large.

I don't follow this

So we're still in deadlock, or is the situation clear now with xenial? I don't want to review this package every Friday..

> So we're still in deadlock, or is the situation clear now with xenial

I assume you're asking @racb, not me, right?

My opinion is the same as I stated it in comment 18. I think there's a risk that a latent problem might be exposed by running autoreconf and changing the build configuration. I think such a problem is unlikely to be triggered by a regular SRU or a security update, since they will generally patch without manually running *or needing to run) autoreconf, and AIUI it won't automatically run either. Therefore I don't see the benefit of landing this change in Xenial. I don't see a specific problem it would fix, nor do I see a significant future risk of a problem being exposed.

Dan says he doesn't follow my logic, so perhaps I'm missing something. If so, I'm happy for another SRU team member to make and follow a different decision.

> AIUI it won't automatically run either

that's exactly the problem, the configure.ac file might be updated to fix a bug, but the build might not run autoreconf to actually update the configure file, and so the package would be built *without* including the added fix - in fact, it might even *regress* the package if the configure file checked in contains less fixes than the configure.ac file from the last build (and autoreconf isn't run).

> I don't see a specific problem it would fix, nor do I see a significant future
> risk of a problem being exposed.

the [test case] section shows how this problem can manifest; and this *actually did happen* with the bionic build, as explained in [impact] section.

I honestly don't understand the resistance to what seems completely safe and correct to me, nor do I understand if my [impact] and [test case] sections are unclear, so maybe I'm missing something in @racb's logic.

On Fri, Dec 13, 2019 at 01:58:19PM -0000, Dan Streetman wrote:
> > I don't see a specific problem it would fix, nor do I see a significant future
> > risk of a problem being exposed.
>
> the [test case] section shows how this problem can manifest; and this
> *actually did happen* with the bionic build, as explained in [impact]
> section.

Because of an attempt at a feature addition, right? And that was picked
up at verification time? I don't think we expect to make any further
feature additions to Xenial now?

> I honestly don't understand the resistance to what seems completely safe
> and correct to me, nor do I understand if my [impact] and [test case]
> sections are unclear, so maybe I'm missing something in @racb's logic.

"We never assume that any change, no matter how obvious, is completely
free of regression risk." --https://wiki.ubuntu.com/StableReleaseUpdates

Changing the build time configuration of a package seems ripe for
unexpected and unrelated regressions to me. Further, dpkg is a critical
package and a regression in it may not be easy for users to resolve just
by us releasing a further update.

I suppose we could stage a change in proposed, because we do now have
that mechanism, but depending on the nature of a future patch, taking
the risk on this change might be unnecessary in the future so I don't
think that would mitigate my concern here.

Your change also seems in principle "completely safe and correct" to me,
but take a look at recent 'regression-update' tagged bugs to see how
other SRUs that seemed "completely safe and correct" resulted in
regressions.

Carefully weighing up the benefits and the risks and reaching a
different conclusion is perfectly fine. It concerns me that rather than
doing this you seem to be denying the existence of any possibility of
regression risk just because it seems "completely safe and correct" at
the moment. That's not how regression risk works.

> > AIUI it won't automatically run either
>
> that's exactly the problem,

just to clarify; autoreconf *is* run during most builds, and *should* be run, as the 'configure' file is (or may be) behind the 'configure.ac' file, since patches to fix bugs go into the configure.ac file, not the configure file.

The problem happens when the build *doesn't* run autoreconf, because the timestamp of 'configure' is newer/equal to the timestamp of 'configure.ac'. Then the build runs using the old 'configure' file, losing all fixes that have been put into the 'configure.ac' file.

This patch simply changes the build to always run autoreconf.

I'm moving this back to In Progress; both @racb and myself have made our arguments, can someone please just either accept or reject this for Xenial?

> And that was picked up at verification time?

no, the breakage was *not* caught at verification time. It was released and then reported by a Canonical UA customer to us.

> Changing the build time configuration of a package

to clarify, again, forcing autoreconf to run at build time isn't changing anything. autoreconf needs to run at build time to properly create the 'configure' file from the 'configure.ac' file. This only makes sure autoreconf always runs.

Removing myself as bug owner, and marking as 'incomplete' as this is waiting on someone from ~ubuntu-sru to reject or accept the upload.

Incomplete should only be used when the bug is missing information which is not the case here.

I asked Steve for a second opinion, and he agreed with me that this upload should be rejected. Please see the reasons already given.

An upload of dpkg to xenial-proposed has been rejected from the upload queue for the following reason: "Discussion in LP: #1842947".