Ubuntu
squid package

squid builds do not halt upon dh_auto_test failures

Bug #2004050 reported by Athos Ribeiro
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
squid (Ubuntu)
Fix Released
Undecided
Athos Ribeiro
Jammy
Fix Released
Undecided
Athos Ribeiro
Kinetic
Won't Fix
Undecided
Athos Ribeiro
Lunar
Fix Released
Undecided
Athos Ribeiro

Bug Description

[ Impact ]

squid builds do not fail when the upstream test suite (ran as part of dh_auto_test) fails.

Running such test suite and ensuring builds will fail when the tests fail will increase confidence in the packaging process and in the health of new packages. This will be specially useful for the MRE test plan being proposed in https://wiki.ubuntu.com/SquidUpdates.

[ Test Plan ]

The package build itself should suffice here since it runs the upstream test suite (and will fail if any of the upstream tests fail).
Additionally, one could run a local build removing the change related to using the installed binary for one of the tests, i.e., re-introducing the commented 0003-installed-binary-for-debian-ci.patch patch should force a test failure, which will make the build fail, confirming the intended changes.

[ Where problems could occur ]

The upstream testsuite may fail in different paths for different platforms (this is exactly why we need to disable LTO in s390x here). This could introduce test failures that were not predicted here and prevent the package to build, forcing us to either provide a new fix or revert our changes. To mitigate the matter, we will build the package for the supported platforms in a PPA before uploading the package. We will also run autopkgtests in these platforms to ensure no regressions were introduced, since we are also touching the dep8 test suite here.

Other than that, issues may arise from re-building the package against possible different versions of its dependencies. This would also be an issue for any next SRUs for the package, and we intend to stage this one (block-proposed) since it is a test related only change.

[ Other Info ]

This change was motivated by the discussions in https://lists.ubuntu.com/archives/ubuntu-release/2023-January/005528.html related to the squid MRE proposal in https://wiki.ubuntu.com/SquidUpdates.

As requested during the MP review process for this SRU, I am describing the change set applied below:

d/p/0003-installed-binary-for-debian-ci.patch was originally added to avoid any sed magic in d/t/upstream-test-suite. This was possible because test failures during the build process were being ignored. When no longer ignoring failures, we needed to drop the mentioned patch and apply some sed substitutions to d/t/upstream-test-suite so DEP8 tests use the installed binaries.

Enabling build failures upon failures in the test suit also triggers LTO related errors in s390x builds. We disable such errors as a workaround.

[ Original message ]

As pointed out in [1], as part of Squid's MRE request [2], squid builds do not fail when the upstream test suite (ran as part of dh_auto_test) fails.

Running such test suite and ensuring builds will fail when the tests fail will increase confidence in the packaging process and in the health of new packages. This will be specially useful for the MRE test plan described in [2].

We want to enable the described behavior in lunar (development) and then SRU it all the way down to focal (which shall also receive MREs in the future).

[1] https://lists.ubuntu.com/archives/ubuntu-release/2023-January/005528.html
[2] https://wiki.ubuntu.com/SquidUpdates

See original description

Related branches

~athos-ribeiro/ubuntu/+source/squid:re-enable-lto-s390x
git-ubuntu bot: Approve
Bryce Harrington (community): Approve
Canonical Server Reporter: Pending requested
Diff: 39 lines (+12/-4)
2 files modified
debian/changelog (+8/-0)
debian/rules (+4/-4)
~athos-ribeiro/ubuntu/+source/squid:halt-build-on-test-failures-kinetic
git-ubuntu bot: Approve
Bryce Harrington (community): Approve
Canonical Server Reporter: Pending requested
Diff: 124 lines (+34/-33)
5 files modified
debian/changelog (+14/-0)
debian/patches/series (+0/-1)
debian/rules (+16/-2)
debian/tests/upstream-test-suite (+4/-0)
dev/null (+0/-30)
~athos-ribeiro/ubuntu/+source/squid:halt-build-on-test-failures-jammy
git-ubuntu bot: Approve
Bryce Harrington (community): Approve
Canonical Server Reporter: Pending requested
Diff: 74 lines (+30/-3)
4 files modified
debian/changelog (+13/-0)
debian/patches/series (+3/-1)
debian/rules (+10/-2)
debian/tests/upstream-test-suite (+4/-0)
~athos-ribeiro/ubuntu/+source/squid:halt-build-on-test-failures-kinetic
Bryce Harrington (community): Needs Fixing
Canonical Server Reporter: Pending requested
Diff: 87 lines (+36/-3)
4 files modified
debian/changelog (+14/-0)
debian/patches/series (+3/-1)
debian/rules (+15/-2)
debian/tests/upstream-test-suite (+4/-0)
~athos-ribeiro/ubuntu/+source/squid:halt-build-on-test-failures
git-ubuntu bot: Approve
Bryce Harrington (community): Approve
Canonical Server Reporter: Pending requested
Diff: 87 lines (+36/-3)
4 files modified
debian/changelog (+14/-0)
debian/patches/series (+3/-1)
debian/rules (+15/-2)
debian/tests/upstream-test-suite (+4/-0)

CVE References

Athos Ribeiro (athos-ribeiro)
no longer affects: squid (Ubuntu Focal)
Athos Ribeiro (athos-ribeiro)
description: updated
tags: added: server-todo
Changed in squid (Ubuntu Lunar):
assignee: nobody → Athos Ribeiro (athos-ribeiro)
Changed in squid (Ubuntu Kinetic):
assignee: nobody → Athos Ribeiro (athos-ribeiro)
Changed in squid (Ubuntu Jammy):
assignee: nobody → Athos Ribeiro (athos-ribeiro)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package squid - 5.7-1ubuntu2

---------------
squid (5.7-1ubuntu2) lunar; urgency=medium

  * Make builds fail when upstream test suite fails (LP: #2004050):
    - d/p/series: do not rely on installed binaries for build time tests.
    - d/rules: halt build upon test failures.
    - d/rules: do not include additional configuration files during
      build time tests. This would lead to test failures due to missing
      paths.
    - d/t/upstream-test-suite: use installed squid binary for
      autopkgtest config file checks.
    - d/rules: disable LTO for s390x builds.

 -- Athos Ribeiro <email address hidden> Fri, 27 Jan 2023 11:06:05 -0300

Changed in squid (Ubuntu Lunar):
status: New → Fix Released
Athos Ribeiro (athos-ribeiro)
tags: added: block-proposed-jammy block-proposed-kinetic
Athos Ribeiro (athos-ribeiro)
Changed in squid (Ubuntu Jammy):
status: New → In Progress
Changed in squid (Ubuntu Kinetic):
status: New → In Progress
Revision history for this message
Steve Langasek (vorlon) wrote :

I'm certainly in favor of upstream test suite results being honored at build time. But I am uncomfortable with the change to disable LTO for the s390x *binaries* on account of the *test suite* failing to build when LTO is enabled.

I realize the flags for both the installed executables and the test suite are generated at the same time and it's awkward to disable LTO for just the test suite. Nevertheless, I think a more nuanced approach is warranted here instead of trading off the test suite vs the optimization of the installed binaries.

Does the build succeed if instead of disabling LTO, you pass -Wno-error=stringop-overread, disabling the specific compiler check that seems to be failing here?

Changed in squid (Ubuntu Kinetic):
status: In Progress → Incomplete
Changed in squid (Ubuntu Jammy):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-jammy
Revision history for this message
Steve Langasek (vorlon) wrote : Please test proposed package

Hello Athos, or anyone else affected,

Accepted squid into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/squid/5.2-1ubuntu4.4 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, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. 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.

Revision history for this message
Simon Déziel (sdeziel) wrote :

Not enough to mark it verification -done but on Jammy, this 5.2-1ubuntu4.4 update doesn't have any ill side effects.

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Builds in jammy are sane. Tests are running and passing during the build.

I also tried re-introducing the patch mentioned in the test plan described in this SRU bug to the package in jammy-proposed and a rebuilds fails as expected.

This should be enough to mark it verification-done.

tags: added: verification-done-jammy
removed: verification-needed-jammy
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

I filed another MP for re-enabling LTO on kinetic, and also a new FFe to do it in lunar as well:

https://bugs.launchpad.net/ubuntu/+source/squid/+bug/2011494

I am also entirely removing the patch in kinetic, as suggested.

Athos Ribeiro (athos-ribeiro)
description: updated
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

I addressed the issues raised here and re-uploaded the kinetic package.

Changed in squid (Ubuntu Kinetic):
status: Incomplete → In Progress
Revision history for this message
Steve Langasek (vorlon) wrote :

Hello Athos, or anyone else affected,

Accepted squid into kinetic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/squid/5.6-1ubuntu3.1 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, what testing has been performed on the package and change the tag from verification-needed-kinetic to verification-done-kinetic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-kinetic. 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 squid (Ubuntu Kinetic):
status: In Progress → Fix Committed
tags: added: verification-needed-kinetic
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Builds in kinetic are sane. Tests are running and passing during the build.

I also tried re-introducing the patch mentioned in the test plan described in this SRU bug to the package in kinetic-proposed and a rebuild fails as expected.

This should be enough to mark it verification-done.

Christian Ehrhardt  (paelzer)
tags: removed: server-todo
Athos Ribeiro (athos-ribeiro)
tags: added: verification-done verification-done-kinetic
removed: verification-needed verification-needed-kinetic
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Proposed package removed from archive

The version of squid in the proposed pocket of Kinetic that was purported to fix this bug report has been removed because the target series has reached its End of Life.

Changed in squid (Ubuntu Kinetic):
status: Fix Committed → Won't Fix
Revision history for this message
Andreas Hasenack (ahasenack) wrote : Please test proposed package

Hello Athos, or anyone else affected,

Accepted squid into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/squid/5.7-0ubuntu0.22.04.1 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, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. 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.

tags: added: verification-needed verification-needed-jammy
removed: verification-done verification-done-jammy
tags: removed: block-proposed-jammy
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

The package builds as expected complying with the test plan.

tags: added: verification-done verification-done-jammy
removed: verification-needed verification-needed-jammy
Athos Ribeiro (athos-ribeiro)
tags: removed: block-proposed-kinetic
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package squid - 5.7-0ubuntu0.22.04.1

---------------
squid (5.7-0ubuntu0.22.04.1) jammy; urgency=medium

  * New upstream version. (LP: #2013423):
    - Fix FATAL FwdState::noteDestinationsEnd exception. (LP: #1975399)
    - Fix regression that made the default value for the esi_parser
      configuration directive behave differently from its documented behavior.
      It now correctly uses libxml2 if available and falls back to libexpat
      otherwise.
    - Fix unexpected dispatch of client CA certificates to https_port clients
      when OpenSSL SSL_MODE_NO_AUTO_CHAIN mode is on.
    - Add OpenSSL 3.0 support for features that were already supported by
      squid. No new OpenSSL 3.0 feature support added at this time.
    - The configuration directive ssl_engine is no longer recognized. Since
      this option is not implemented for the OpenSSL 3 used in Ubuntu 22.04
      LTS, this is not a functional regression. Now, instead of failing with
      "FATAL: Your OpenSSL has no SSL engine support", it fails with "FATAL:
      bad configuration: Cannot use ssl_engine in Squid built with OpenSSL 3.0
      or newer".
    - For a comprehensive list of changes, please see
      http://www.squid-cache.org/Versions/v5/ChangeLog.html.
  * d/p/close-tunnel-if-to-server-conn-closes-after-client.patch: remove
    upstreamed patch.
    [ Fixed in 5.4 ]
  * d/p/0004-Change-default-Makefiles-for-debian.patch: remove upstreamed
    patch.
    [ Fixed in 5.5 ]
  * d/p/CVE-2021-46784.patch: remove upstreamed patch.
    [ Fixed in 5.6 ]
  * d/p/CVE-2022-41317.patch: drop patch to fix typo in manager ACL.
    [ Fixed in 5.7 ]
  * d/p/CVE-2022-41318.patch: drop patch to fix NTLM decoder truncated strings.
    [ Fixed in 5.7 ]
  * d/p/openssl3-*.patch: drop downstream OpenSSL 3 support patch.
    [ Fixed in 5.7 ]
  * d/p/99-ubuntu-ssl-cert-snakeoil.patch: refresh patch.

squid (5.2-1ubuntu4.4) jammy; urgency=medium

  * Make builds fail when upstream test suite fails (LP: #2004050):
    - d/p/series: do not rely on installed binaries for build time tests.
    - d/rules: halt build upon test failures.
    - d/rules: do not include additional configuration files during
      build time tests. This would lead to test failures due to missing
      paths.
    - d/t/upstream-test-suite: use installed squid binary for
      autopkgtest config file checks.

 -- Athos Ribeiro <email address hidden> Thu, 30 Mar 2023 17:06:59 -0300

Changed in squid (Ubuntu Jammy):
status: Fix Committed → Fix Released
Revision history for this message
Andreas Hasenack (ahasenack) wrote : Update Released

The verification of the Stable Release Update for squid 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.

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

Other bug subscribers

Remote bug watches

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