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

Bug #1842947 reported by Dan Streetman on 2019-09-05
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
dpkg (Debian)
Fix Committed
Unknown
dpkg (Ubuntu)
Medium
Dan Streetman
Xenial
Medium
Dan Streetman
Bionic
Medium
Dan Streetman
Disco
Medium
Dan Streetman
Eoan
Medium
Dan Streetman

Bug Description

[impact]

dpkg at version 1.19.0.5ubuntu2 had support for zstd added:
https://launchpad.net/ubuntu/+source/dpkg/1.19.0.5ubuntu2

part of that change was to update the 'configure.ac' file with zstd support, e.g.:
http://launchpadlibrarian.net/366237303/dpkg_1.19.0.5ubuntu1_1.19.0.5ubuntu2.diff.gz

note that the 'configure' file was not updated - which *should* be ok, as it should be recreated from the 'configure.ac' file during build. For the build of that version and the next (1.19.0.5ubuntu2.1), the 'configure' file was correctly recreated during build.

However at version 1.19.0.5ubuntu2.2, the 'configure' file was not recreated during build. Thus, dpkg was not built linked against libzstd.
This regresses the ability of dpkg to uncompress zstd-compressed packages, unless the zstd utility is installed on the local system. Since dpkg does not list the zstd package as a dep, it may not be installed on all users' systems who want to install a zstd-compressed package.

[test case]

on bionic system:

$ sudo apt install ubuntu-dev-tools
$ pull-lp-source dpkg 1.19.0.5ubuntu2.2
$ cd dpkg-1.19.0.5ubuntu2.2/
$ sudo apt build-dep .
$ dpkg-buildpackage

and verify if dpkg-deb is linked against libzstd:
$ ldd build-tree/dpkg-deb/dpkg-deb | grep zstd

or extract it from the deb itself and check:
$ dpkg-deb -x ../dpkg_1.19.0.5ubuntu2.2_amd64.deb ../deb-files
$ ldd ../deb-files/usr/bin/dpkg-deb | grep zstd

simply touching the 'configure.ac' file (to bring its timestamp newer than the 'configure' file) causes the build to work correctly:

$ mkdir no-touch
$ cd no-touch
$ dpkg-source -x ~/dpkg_1.19.0.5ubuntu2.2.dsc
$ cd dpkg-1.19.0.5ubuntu2.2/
$ dpkg-buildpackage
$ ldd build-tree/dpkg-deb/dpkg-deb | grep zstd
$

$ mkdir touch
$ cd touch
$ dpkg-source -x ~/dpkg_1.19.0.5ubuntu2.2.dsc
$ cd dpkg-1.19.0.5ubuntu2.2/
$ touch configure.ac
$ dpkg-buildpackage
$ ldd build-tree/dpkg-deb/dpkg-deb | grep zstd
 libzstd.so.1 => /usr/lib/x86_64-linux-gnu/libzstd.so.1 (0x00007f8c1d8af000)

[regression potential]

this forces autoreconf to be run for each build, which may add some small amount of time to the build. Other than that, the regression potential seems small, since autoreconf should be getting run for each build, and was for most (but not all) builds. Any regression would almost certainly involve a failure to build the package, or a failure to pick up new configure.ac changes correctly.

[other info]

this might not be an issue specifically with dpkg itself, it could be an issue with debhelper and other tooling that is responsible for calling autoconf or autoreconf during build. Or possibly a problem with the dpkg debian/rules or other related build config.

Or, simply including the 'configure' file in the package source might be considered a bug, since it's an intermediate build file that really shouldn't be included. However, it's included in many source packages, including in debian, and removing it from all of them seems unlikely and/or unwieldy. Additionally, for "normal" packages that use quilt (i.e., aren't native), any changes to the 'configure.ac' file would be done with a patch, meaning the pre-build process would always make the 'configure.ac' file newer than the 'configure' file.

Maybe for native packages, autoconf/autoreconf should always be called with -f, or maybe the 'configure' file should be removed from native packages.

Dan Streetman (ddstreet) on 2019-09-05
description: updated
Dan Streetman (ddstreet) wrote :

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'.

Dan Streetman (ddstreet) wrote :

> 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.

Dan Streetman (ddstreet) on 2019-09-05
Changed in dpkg (Ubuntu Xenial):
assignee: nobody → Dan Streetman (ddstreet)
Changed in dpkg (Ubuntu Bionic):
assignee: nobody → Dan Streetman (ddstreet)
Changed in dpkg (Ubuntu Disco):
assignee: nobody → Dan Streetman (ddstreet)
Changed in dpkg (Ubuntu Eoan):
assignee: nobody → Dan Streetman (ddstreet)
Changed in dpkg (Ubuntu Disco):
importance: Undecided → Medium
Changed in dpkg (Ubuntu Eoan):
importance: Undecided → Medium
Changed in dpkg (Ubuntu Xenial):
importance: Undecided → Medium
Changed in dpkg (Ubuntu Disco):
status: New → In Progress
Changed in dpkg (Ubuntu Xenial):
status: New → In Progress
Changed in dpkg (Ubuntu Bionic):
importance: Undecided → Medium
status: New → In Progress
Changed in dpkg (Ubuntu Eoan):
status: New → In Progress
Changed in dpkg (Debian):
status: Unknown → New
Dan Streetman (ddstreet) on 2019-09-06
tags: added: sts
Launchpad Janitor (janitor) wrote :

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

Changed in dpkg (Ubuntu Eoan):
status: In Progress → Fix Released
Robie Basak (racb) wrote :

> [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.

Robie Basak (racb) wrote :

> [regression potential]

> TBD

Please also complete this section.

Dan Streetman (ddstreet) wrote :

> 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

description: updated
description: updated

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.

Changed in dpkg (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-bionic
Steve Langasek (vorlon) wrote :

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?

Changed in dpkg (Ubuntu Xenial):
status: In Progress → Incomplete
Steve Langasek (vorlon) wrote :

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.

tags: added: block-proposed-disco
Changed in dpkg (Ubuntu Disco):
status: In Progress → Fix Committed
tags: added: verification-needed-disco
Steve Langasek (vorlon) wrote :

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.

Dan Streetman (ddstreet) wrote :

> 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!

Dan Streetman (ddstreet) wrote :
tags: added: verification-done verification-done-bionic verification-done-disco
removed: verification-needed verification-needed-bionic verification-needed-disco
Dan Streetman (ddstreet) wrote :

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.

Launchpad Janitor (janitor) wrote :

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

Changed in dpkg (Ubuntu Bionic):
status: Fix Committed → Fix Released

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.

Robie Basak (racb) wrote :

> 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?

Dan Streetman (ddstreet) wrote :

> > 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

Changed in dpkg (Debian):
status: New → Fix Committed
Timo Aaltonen (tjaalton) wrote :

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

Dan Streetman (ddstreet) wrote :

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

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

Robie Basak (racb) wrote :

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.

Dan Streetman (ddstreet) wrote :

> 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.

Dan Streetman (ddstreet) wrote :

> > 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.

Changed in dpkg (Ubuntu Xenial):
status: Incomplete → In Progress
Dan Streetman (ddstreet) wrote :

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?

Dan Streetman (ddstreet) wrote :

> 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.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.