[SRU] apt-add-repository adds duplicate commented/disabled source lines

Bug #1311056 reported by Jan Groenewald on 2014-04-22
34
This bug affects 3 people
Affects Status Importance Assigned to Milestone
python-apt (Ubuntu)
High
Unassigned
Xenial
Medium
Dave Jones
Bionic
High
Dave Jones
Eoan
High
Dave Jones

Bug Description

Impact
======

Under most circumstances, the impact is minimal (a few extra redundant comment lines in apt sources. However, if users are automating source removal / addition on a machine (as in comment 11), there is the potential to wind up with an excessively large (and thus slow to parse) apt sources configuration.

Test packages for the supported releases are available from the following PPA:

https://launchpad.net/~waveform/+archive/ubuntu/python-apt

Built from the source which can be found in the following branches:

https://code.launchpad.net/~waveform/ubuntu/+source/python-apt/+git/python-apt/+ref/sru-dupe-ppa-xenial

https://code.launchpad.net/~waveform/ubuntu/+source/python-apt/+git/python-apt/+ref/sru-dupe-ppa-bionic

https://code.launchpad.net/~waveform/ubuntu/+source/python-apt/+git/python-apt/+ref/sru-dupe-ppa-eoan

Test Case
=========

* sudo add-apt-repository -y ppa:deadsnakes/ppa
* cat /etc/apt/sources.list.d/deadsnakes*.list
* Note the presence of one uncommented "deb" line, and one commented "deb-src" line
* sudo add-apt-repository -y ppa:deadsnakes/ppa
* cat /etc/apt/sources.list.d/deadsnakes*.list
* Note the uncommented "deb" line is still there but the commented "deb-src" line has now been duplicated
* sudo add-apt-repository ppa:waveform/python-apt
* sudo apt upgrade # update python-apt to fixed version
* sudo add-apt-repository -y ppa:deadsnakes/ppa
* cat /etc/apt/sources.list.d/deadsnakes*.list
* Note there has been no further duplication of the commented "deb-src" line

Regression Potential
====================

Minimal; test cases have been added to cover the duplication case, and to cover the enabling of sources (which was not covered by existing tests, but was part of the code altered to fix the duplication case), and insertion of sources at a position (again, not covered by existing tests but modified as part of the fix). The test case has been used successfully on all targeted releases (xenial, bionic, and eoan).

Original Description
====================

Trusty Tahr 14.04

0 root@osprey:/etc/apt/sources.list.d#cat aims-aims-desktop-trusty.list
deb http://ppa.launchpad.net/aims/aims-desktop/ubuntu trusty main
# deb-src http://ppa.launchpad.net/aims/aims-desktop/ubuntu trusty main
0 root@osprey:/etc/apt/sources.list.d#apt-add-repository -y ppa:aims/aims-desktop
gpg: keyring `/tmp/tmp0ufdhnmv/secring.gpg' created
gpg: keyring `/tmp/tmp0ufdhnmv/pubring.gpg' created
gpg: requesting key BE796FF2 from hkp server keyserver.ubuntu.com
gpg: /tmp/tmp0ufdhnmv/trustdb.gpg: trustdb created
gpg: key BE796FF2: public key "Launchpad PPA for AIMS" imported
gpg: Total number processed: 1
gpg: imported: 1 (RSA: 1)
OK
0 root@osprey:/etc/apt/sources.list.d#cat aims-aims-desktop-trusty.list deb http://ppa.launchpad.net/aims/aims-desktop/ubuntu trusty main
# deb-src http://ppa.launchpad.net/aims/aims-desktop/ubuntu trusty main
# deb-src http://ppa.launchpad.net/aims/aims-desktop/ubuntu trusty main
0 root@osprey:/etc/apt/sources.list.d#

That deb-src line should have stayed commented out, and not been duplicated. (Commented deb lines should of course be uncommented, as already fixed per https://bugs.launchpad.net/ubuntu/+source/python-apt/+bug/1042916 .)

Thank you for taking the time to report this bug and helping to make Ubuntu better. It seems that your bug report is not filed about a specific source package though, rather it is just filed against Ubuntu in general. It is important that bug reports be filed about source packages so that people interested in the package can find the bugs about it. You can find some hints about determining what package your bug might be about at https://wiki.ubuntu.com/Bugs/FindRightPackage. You might also ask for help in the #ubuntu-bugs irc channel on Freenode.

To change the source package that this bug is filed about visit https://bugs.launchpad.net/ubuntu/+bug/1311056/+editstatus and add the package name in the text box next to the word Package.

[This is an automated message. I apologize if it reached you inappropriately; please just reply to this message indicating so.]

tags: added: bot-comment
Brian Murray (brian-murray) wrote :

I think this is a duplicate.

affects: ubuntu → software-properties (Ubuntu)
Jan Groenewald (jan-aims) wrote :

Of which bug?

Changed in software-properties (Ubuntu):
importance: Undecided → Low
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in software-properties (Ubuntu):
status: New → Confirmed
Brian Murray (brian-murray) wrote :

It's actually python3-apt's sourceslist.py which does the duplicate checking and software properties is passing a type, '# deb-src', which I think is an abuse of what python-apt expects. I believe it'd want a 'deb' or 'deb-src' type. So we end up in this strange situation where are not searching for the right type and cannot see if the entry we want to add is a disabled one.

Brian Murray (brian-murray) wrote :

Here's a patch to software-properties.

Changed in software-properties (Ubuntu):
status: Confirmed → Triaged
Brian Murray (brian-murray) wrote :

And here's the patch for python-apt.

Changed in python-apt (Ubuntu):
status: New → Triaged
importance: Undecided → Low
Brian Murray (brian-murray) wrote :

Julian - Do you have an opinion on this change to python-apt to handle adding disabled sources?

tags: added: patch
Julian Andres Klode (juliank) wrote :

Analysis *before* situation: The loop after the __find call did not really match the __find call before - it was looking only for non-disabled sources, but then had an if case for disabled ones to enable them if the components match - which was never reached.

Now, I think the patch is slightly wrong: The if case is reachable now, so it would enable an existing disabled source even if we passed disabled=True. The for loop thus needs 4 cases now, the existing ones corresponding to disabled=False and two variants of them for disabled=True (might be possible to write with less cases).

Julian Andres Klode (juliank) wrote :

This might be enough to fix it - I just compare the values against each other rather than doing 4 explicit cases. I call bool() on them first before comparing them as we want a boolean comparison here, and that should work even if you pass None or "" or whatever I guess.

diff --git a/aptsources/sourceslist.py b/aptsources/sourceslist.py
index 89cef642..d4e52c4d 100644
--- a/aptsources/sourceslist.py
+++ b/aptsources/sourceslist.py
@@ -332,10 +332,10 @@ class SourcesList(object):
         for source in sources:
             # if there is a repo with the same (type, uri, dist) just add the
             # components
- if source.disabled and set(source.comps) == set(comps):
- source.disabled = False
+ if bool(source.disabled) != bool(disabled) and set(source.comps) == set(comps):
+ source.disabled = bool(disabled)
                 return source
- elif not source.disabled:
+ elif bool(disabled) == bool(source.disabled):
                 source.comps = uniq(source.comps + comps)
                 return source
         # there isn't any matching source, so create a new line and parse it

Peter Sabaini (peter-sabaini) wrote :

We're seeing this in the context of a charm where add-apt-repository is called periodically. We ended up with a sources.list file weighing in at 2.6Mb, which created problems for the regex engine there. In light of this can I ask to reconsider the "low" importance of this bug?

FTR. this is Bionic, software-properties ver 0.96.24.32.7

tags: added: canonical-bootstack
Peter Sabaini (peter-sabaini) wrote :

clarification: breaking regex parsing at /usr/lib/python3/dist-packages/aptsources/sourceslist.py that is

Changed in python-apt (Ubuntu):
importance: Low → Medium
importance: Medium → Critical
importance: Critical → High
Peter Sabaini (peter-sabaini) wrote :

Subscribing field-medium, we're seeing this repeatedly

Julian Andres Klode (juliank) wrote :

Can you check the patch I posted in comment #10?

tags: added: id-5d65087107d6ae450d988462
Peter Sabaini (peter-sabaini) wrote :

Julian,

unfort. testing the patch above against a bionic system didn't help, I'm still getting duplicate deb-src entries

# deb-src http://ppa.launchpad.net/juju/stable/ubuntu bionic main
# deb-src http://ppa.launchpad.net/juju/stable/ubuntu bionic main
# deb-src http://ppa.launchpad.net/juju/stable/ubuntu bionic main
# deb-src http://ppa.launchpad.net/juju/stable/ubuntu bionic main

Thanks.

Dave Jones (waveform) wrote :

I have an updated patch available from the following branch for focal:

    https://code.launchpad.net/~waveform/ubuntu/+source/python-apt/+git/python-apt/+ref/fix-dupe-ppa

Currently building a test package in the following PPA:

    https://launchpad.net/~waveform/+archive/ubuntu/python-apt/+packages

I realize most of the people affected by this are on older distributions, so I'm just in the process of back-porting the patch to bionic, xenial, and trusty, and will provide test builds in the same PPA once that's done.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package python-apt - 1.9.3ubuntu2

---------------
python-apt (1.9.3ubuntu2) focal; urgency=medium

  * Revert "apt.Cache: cache apt.package.Origin objects by id"

 -- Julian Andres Klode <email address hidden> Tue, 17 Dec 2019 19:24:36 +0100

Changed in python-apt (Ubuntu):
status: Triaged → Fix Released
no longer affects: software-properties (Ubuntu)
no longer affects: software-properties (Ubuntu Xenial)
no longer affects: software-properties (Ubuntu Bionic)
no longer affects: software-properties (Ubuntu Eoan)
Dave Jones (waveform) on 2020-01-30
description: updated
summary: - apt-add-repository adds duplicate commented/disabled source lines
+ [SRU] apt-add-repository adds duplicate commented/disabled source lines
Dave Jones (waveform) on 2020-01-30
description: updated
Dave Jones (waveform) on 2020-02-12
description: updated
Mathew Hodson (mhodson) on 2020-02-16
Changed in python-apt (Ubuntu Xenial):
importance: Undecided → Medium
Changed in python-apt (Ubuntu Eoan):
importance: Undecided → High
Changed in python-apt (Ubuntu Bionic):
importance: Undecided → High
Dave Jones (waveform) on 2020-03-30
Changed in python-apt (Ubuntu Eoan):
assignee: nobody → Dave Jones (waveform)
Changed in python-apt (Ubuntu Xenial):
assignee: nobody → Dave Jones (waveform)
Changed in python-apt (Ubuntu Bionic):
assignee: nobody → Dave Jones (waveform)
Changed in python-apt (Ubuntu Xenial):
status: New → In Progress
Changed in python-apt (Ubuntu Bionic):
status: New → In Progress
Changed in python-apt (Ubuntu Eoan):
status: New → In Progress

Hello Jan, or anyone else affected,

Accepted python-apt into eoan-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-apt/1.9.0ubuntu1.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-eoan to verification-done-eoan. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-eoan. 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 python-apt (Ubuntu Eoan):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-eoan
Łukasz Zemczak (sil2100) wrote :

Hello Jan, or anyone else affected,

Accepted python-apt into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-apt/1.6.5ubuntu0.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, what testing has been performed on the package 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 python-apt (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed-bionic
Changed in python-apt (Ubuntu Xenial):
status: In Progress → Fix Committed
tags: added: verification-needed-xenial
Łukasz Zemczak (sil2100) wrote :

Hello Jan, or anyone else affected,

Accepted python-apt into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-apt/1.1.0~beta1ubuntu0.16.04.9 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-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. 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.

All autopkgtests for the newly accepted python-apt (1.1.0~beta1ubuntu0.16.04.9) for xenial have finished running.
The following regressions have been reported in tests triggered by the package:

apport/2.20.1-0ubuntu2.23 (i386)
aptdaemon/1.1.1+bzr982-0ubuntu14.2 (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/xenial/update_excuses.html#python-apt

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

Thank you!

All autopkgtests for the newly accepted python-apt (1.6.5ubuntu0.3) for bionic have finished running.
The following regressions have been reported in tests triggered by the package:

unattended-upgrades/unknown (armhf)
aptdaemon/1.1.1+bzr982-0ubuntu19.2 (armhf)
python-aptly/0.12.10-1 (arm64)

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#python-apt

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

Thank you!

All autopkgtests for the newly accepted python-apt (1.9.0ubuntu1.4) for eoan have finished running.
The following regressions have been reported in tests triggered by the package:

ubiquity/19.10.21 (armhf)
aptdaemon/1.1.1+bzr982-0ubuntu28.1 (arm64, 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/eoan/update_excuses.html#python-apt

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

Thank you!

Dave Jones (waveform) on 2020-05-28
tags: added: verification-done-bionic
removed: verification-needed-bionic
Dave Jones (waveform) on 2020-05-28
tags: added: verification-done-xenial
removed: verification-needed-xenial
tags: added: verification-done-eoan
removed: verification-needed-eoan
Dave Jones (waveform) wrote :

Successfully verified fixes on xenial, bionic, and eoan. The autopkgtest regressions on xenial were due to flaky tests on i386 and armhf; these have been re-run successfully. The regressions on bionic look like a transient network failure (would be grateful if someone could attempt a re-run on those). The same is true of one of the regressions on eoan; the other *may* be (would be grateful if someone could attempt a re-run of those too).

Dave Jones (waveform) wrote :

All regressions now cleared after a re-run (thanks!), and verifications done.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package python-apt - 1.9.0ubuntu1.4

---------------
python-apt (1.9.0ubuntu1.4) eoan; urgency=medium

  * Don't duplicate disabled sources during add() (LP: #1311056)

 -- Dave Jones <email address hidden> Fri, 24 Jan 2020 22:08:17 +0000

Changed in python-apt (Ubuntu Eoan):
status: Fix Committed → Fix Released

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

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package python-apt - 1.6.5ubuntu0.3

---------------
python-apt (1.6.5ubuntu0.3) bionic; urgency=medium

  * Don't duplicate disabled sources during add() (LP: #1311056)

 -- Dave Jones <email address hidden> Fri, 24 Jan 2020 22:06:25 +0000

Changed in python-apt (Ubuntu Bionic):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package python-apt - 1.1.0~beta1ubuntu0.16.04.9

---------------
python-apt (1.1.0~beta1ubuntu0.16.04.9) xenial; urgency=medium

  * Don't duplicate disabled sources during add() (LP: #1311056)

 -- Dave Jones <email address hidden> Fri, 24 Jan 2020 22:05:23 +0000

Changed in python-apt (Ubuntu Xenial):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers