[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#740843: apt: fails to upgrade the fglrx family of packages with multiarch



On Wed, Mar 05, 2014 at 02:27:08PM +0000, Simon McVittie wrote:
> The test set of packages are created by the attached files (put them all
> in the same directory and run make), and are a simplified version of the
> original situation involving fglrx:
> 
>     foo-driver amd64 installed
>         Depends: libfoo (= VERSION)
>         Recommends: libgl1-foo-glx (= VERSION)
>         Breaks: libgl1-foo-glx (<< VERSION)
>         Breaks: libgl1-foo-glx (>> VERSION)

(I wanted to suggest != relation here as it is what apt does for the
 implicit breaks for Multi-Arch, but noticed that isn't a blessed
 relation so nothing beside apt is supporting it… "interesting")


Relational speaking, isn't libgl1-foo-glx breaking the foo-driver
and not the other way around? Also, if I wanted to install
libgl1-foo-glx:i386, wouldn't I install foo-driver:i386 ?
Personally, I would prefer reverting to the previous depends, but that
isn't my business… but: think of all the kittens^Wpartial upgrades.


>     libgl1-foo-glx both amd64 and i386 installed
>         Depends: libfoo (= VERSION)
> 
>     libfoo both amd64 and i386 installed
> 
> The only configured apt repository at the time of the upgrade contains
> packages of the same names, with the same relationships, but a higher
> version. The Makefile rules for "test_apt_in_chroot" and
> "test_uu_in_chroot" indicate the specifics of how I tested this, in a
> small chroot environment (build-essential + aptitude + sudo + vim).

Thanks for the testcase! If you deal more often with this kind of tests
you can have a look at our tests in test/integration/. It allows 'real'
minimal tests. It doesn't have uu support, but I guess Michael would
actually like to have it.


> == expected result ==
> 
> I would expect the problem resolution to involve temporarily
> deconfiguring foo-driver and libgl1-foo-glx, upgrading both flavours of
> libfoo, configuring libgl1-foo-glx, and finally configuring foo-driver.

jftr: foo-driver shouldn't be deconfigured (this isn't done by apt, but
by dpkg automatically based on dependencies - but foo-driver hasn't any).
The solution is to first unpack all the coupled packages in one go and
configure it after that (again in one go).
If the breaks would be conflicts the apt would need to be more active in
regards to foo-driver: It would need to temporarily remove it…


> == actual result ==
> 
> However, what actually happens is that apt gets part-way through the
> upgrade, but does not deconfigure foo-driver in favour of the new
> libgl1-foo-glx:
[…]
> This seems to be multiarch-related: not installing the i386 packages
> leads to the upgrade working fine.
> 

Lets start with the good news: I have a patch attached fixing this.
Even better: It removes lines of code rather than adding some new.
The "bad" thing is that I am usually quiet scared then it comes to
changes to the ordering (as the testing happens at release+1 time).
Even worse, the change isn't hidden in some deep layer nothing ever
encounters, but basically a top-level change which will effect (now
that Multi-Arch is everywhere) basically every (bigger) run.

On the "pro-side", it is reverting partly commit 634985f8 which was
written in Jul 2011 as part of a GSoC project which based on the commit
message was an unrelated change in this commit.
I have cc'ed Christopher just in case he can shine some light on this
(we will ask for a refund otherwise, so no pressure ;) SCNR.). I have
the impression that this was needed to cover cases which are now covered
in other areas and so has most of the time no effect or 'subtil' effects
which are okay most of the time, but in rare cases like here explode.

The effect is that it breaks an unpack chain with configures and for
this to work forces some additional actions to be done now rather than
at a later time (This can be seen in the testcases I modified) but
"forgets" to handle some.


So, I guess I should mention now what is actually wrong:
APT computes a (pre-)order for unpacking which is basically a topological
sorting. Based on this list it will unpack each package, but will deal
with loops, conflicts, pre-depends and co which it encounters along the
way immediately. One of those 'problems' are M-A:same packages which are
installed for multiple architectures. All fine so far.
The problem is what is done then we reach a package via the list
which is already unpacked. The code used to and I propose again to do
nothing. What the code does at the moment is configuring the package
(which means dependencies must be unpacked and configured as well and
 reverse-dependencies must be dealt with. The later is actually the
 problem here). This is not wrong per se, but makes the process harder
than needed.

Alternatively, we could limit the ignore to M-A:same packages. The
testcases behave the same as remaining cases are triggered so late that
nothing happens. My take is "just" that the SmartConfigure call is wrong
here in all cases as I can't imagine a valid case.

The alternative-alternative is of course to deal with reverse-
dependencies. The code currently only does care about conflicts which
are kinda a different beast in this regard (remember the temp remove).
My guess is that situations in which we would have to look at rev-deps
can't happen as these are covered by the pre-order. I at least couldn't
produce a situation in which it wasn't sufficient in this regard.

"What could possibly go wrong?"
Or in other words: What do you think, Michael?


Best regards

David Kalnischkies
diff --git a/apt-pkg/packagemanager.cc b/apt-pkg/packagemanager.cc
index 4d08fb3..5d6bc6b 100644
--- a/apt-pkg/packagemanager.cc
+++ b/apt-pkg/packagemanager.cc
@@ -622,6 +622,8 @@ bool pkgPackageManager::SmartUnPack(PkgIterator Pkg, bool const Immediate, int c
         clog << " (replace version " << Pkg.CurrentVer().VerStr() << " with " << InstallVer.VerStr() << ")";
       if (PkgLoop)
         clog << " (Only Perform PreUnpack Checks)";
+      if (Immediate)
+	 clog << " immediately";
       clog << endl;
    }
 
@@ -963,21 +965,14 @@ pkgPackageManager::OrderResult pkgPackageManager::OrderInstall()
    for (pkgOrderList::iterator I = List->begin(); I != List->end(); ++I)
    {
       PkgIterator Pkg(Cache,*I);
-      
+
       if (List->IsNow(Pkg) == false)
       {
-         if (!List->IsFlag(Pkg,pkgOrderList::Configured) && !NoImmConfigure) {
-            if (SmartConfigure(Pkg, 0) == false && Debug)
-               _error->Warning("Internal Error, Could not configure %s",Pkg.FullName().c_str());
-            // FIXME: The above warning message might need changing
-         } else {
-	    if (Debug == true)
-	       clog << "Skipping already done " << Pkg.FullName() << endl;
-	 }
+	 if (Debug == true)
+	    clog << "Skipping already done " << Pkg.FullName() << endl;
 	 continue;
-	 
       }
-      
+
       if (List->IsMissing(Pkg) == true)
       {
 	 if (Debug == true)
diff --git a/test/integration/framework b/test/integration/framework
index 83deafe..d661717 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1010,11 +1010,15 @@ testequalor2() {
 	shift 2
 	msgtest "Test for equality OR of" "$*"
 	$* >$COMPAREAGAINST 2>&1 || true
-	(checkdiff $COMPAREFILE1 $COMPAREAGAINST 1> /dev/null ||
-		checkdiff $COMPAREFILE2 $COMPAREAGAINST 1> /dev/null) && msgpass ||
-		( echo "\n${CINFO}Diff against OR 1${CNORMAL}" "$(checkdiff $COMPAREFILE1 $COMPAREAGAINST)" \
-		       "\n${CINFO}Diff against OR 2${CNORMAL}" "$(checkdiff $COMPAREFILE2 $COMPAREAGAINST)" &&
-		  msgfail )
+	if checkdiff $COMPAREFILE1 $COMPAREAGAINST 1> /dev/null || checkdiff $COMPAREFILE2 $COMPAREAGAINST 1> /dev/null; then
+		msgpass
+	else
+		echo -n "\n${CINFO}Diff against OR 1${CNORMAL}"
+		checkdiff $COMPAREFILE1 $COMPAREAGAINST || true
+		echo -n "${CINFO}Diff against OR 2${CNORMAL}"
+		checkdiff $COMPAREFILE2 $COMPAREAGAINST || true
+		msgfail
+	fi
 }
 
 testshowvirtual() {
diff --git a/test/integration/test-bug-740843-versioned-up-down-breaks b/test/integration/test-bug-740843-versioned-up-down-breaks
new file mode 100755
index 0000000..81fc237
--- /dev/null
+++ b/test/integration/test-bug-740843-versioned-up-down-breaks
@@ -0,0 +1,41 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+setupenvironment
+configarchitecture 'amd64' 'i386'
+
+insertinstalledpackage 'foo-driver' 'amd64' '1' 'Depends: libfoo (= 1)
+Recommends: libgl1-foo-glx (= 1)
+Breaks: libgl1-foo-glx (<< 1), libgl1-foo-glx (>> 1)'
+insertinstalledpackage 'libgl1-foo-glx' 'amd64,i386' '1' 'Depends: libfoo (= 1)
+Multi-Arch: same'
+insertinstalledpackage 'libfoo' 'amd64,i386' '1' 'Multi-Arch: same'
+
+buildsimplenativepackage 'foo-driver' 'amd64' '2' 'stable' 'Depends: libfoo (= 2)
+Recommends: libgl1-foo-glx (= 2)
+Breaks: libgl1-foo-glx (<< 2), libgl1-foo-glx (>> 2)'
+buildsimplenativepackage 'libgl1-foo-glx' 'amd64,i386' '2' 'stable' 'Depends: libfoo (= 2)
+Multi-Arch: same'
+buildsimplenativepackage 'libfoo' 'amd64,i386' '2' 'stable' 'Multi-Arch: same'
+
+setupaptarchive
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following packages will be upgraded:
+  foo-driver libfoo libfoo:i386 libgl1-foo-glx libgl1-foo-glx:i386
+5 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
+Inst libgl1-foo-glx [1] (2 stable [amd64]) [libgl1-foo-glx:amd64 on libgl1-foo-glx:i386] [libgl1-foo-glx:i386 on libgl1-foo-glx:amd64] [foo-driver:amd64 on libgl1-foo-glx:amd64] [libgl1-foo-glx:i386 foo-driver:amd64 ]
+Inst libgl1-foo-glx:i386 [1] (2 stable [i386]) [foo-driver:amd64 on libgl1-foo-glx:amd64] [foo-driver:amd64 on libgl1-foo-glx:i386] [foo-driver:amd64 ]
+Inst foo-driver [1] (2 stable [amd64]) []
+Inst libfoo:i386 [1] (2 stable [i386]) [libfoo:amd64 on libfoo:i386] [libfoo:i386 on libfoo:amd64] [libfoo:amd64 ]
+Inst libfoo [1] (2 stable [amd64])
+Conf libfoo:i386 (2 stable [i386])
+Conf libfoo (2 stable [amd64])
+Conf libgl1-foo-glx:i386 (2 stable [i386])
+Conf libgl1-foo-glx (2 stable [amd64])
+Conf foo-driver (2 stable [amd64])' aptget dist-upgrade -s
+
+testsuccess aptget dist-upgrade -y -o Debug::pkgPackageManager=1 -o Debug::pkgOrderList=1
diff --git a/test/integration/test-bug-multiarch-upgrade b/test/integration/test-bug-multiarch-upgrade
index dc3725d..c29e1f9 100755
--- a/test/integration/test-bug-multiarch-upgrade
+++ b/test/integration/test-bug-multiarch-upgrade
@@ -25,5 +25,5 @@ The following packages will be upgraded:
 2 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
 Inst libcups2 [1] (2 unstable [amd64]) [libcups2:amd64 on libcups2:i386] [libcups2:i386 on libcups2:amd64] [libcups2:i386 ]
 Inst libcups2:i386 [1] (2 unstable [i386])
-Conf libcups2 (2 unstable [amd64])
-Conf libcups2:i386 (2 unstable [i386])' aptget install -s libcups2:i386
+Conf libcups2:i386 (2 unstable [i386])
+Conf libcups2 (2 unstable [amd64])' aptget install -s libcups2:i386
diff --git a/test/integration/test-ignore-provides-if-versioned-breaks b/test/integration/test-ignore-provides-if-versioned-breaks
index f8b4544..745f7d2 100755
--- a/test/integration/test-ignore-provides-if-versioned-breaks
+++ b/test/integration/test-ignore-provides-if-versioned-breaks
@@ -142,9 +142,9 @@ The following packages will be upgraded:
 2 upgraded, 2 newly installed, 0 to remove and 2 not upgraded.
 Inst foo-same:amd64 [2.0] (4.0 unstable [amd64]) [foo-same:amd64 on foo-same:i386] [foo-same:i386 on foo-same:amd64] [foo-same:i386 ]
 Inst foo-same [2.0] (4.0 unstable [i386])
-Conf foo-same:amd64 (4.0 unstable [amd64])
-Conf foo-same (4.0 unstable [i386])
 Inst foo-same-breaker-3 (1.0 unstable [i386])
 Inst foo-same-provider (1.0 unstable [i386])
+Conf foo-same (4.0 unstable [i386])
+Conf foo-same:amd64 (4.0 unstable [amd64])
 Conf foo-same-breaker-3 (1.0 unstable [i386])
 Conf foo-same-provider (1.0 unstable [i386])' aptget install foo-same-provider foo-same-breaker-3 -s
diff --git a/test/integration/test-ignore-provides-if-versioned-conflicts b/test/integration/test-ignore-provides-if-versioned-conflicts
index 44eafcf..a072527 100755
--- a/test/integration/test-ignore-provides-if-versioned-conflicts
+++ b/test/integration/test-ignore-provides-if-versioned-conflicts
@@ -142,9 +142,9 @@ The following packages will be upgraded:
 2 upgraded, 2 newly installed, 0 to remove and 2 not upgraded.
 Inst foo-same:amd64 [2.0] (4.0 unstable [amd64]) [foo-same:amd64 on foo-same:i386] [foo-same:i386 on foo-same:amd64] [foo-same:i386 ]
 Inst foo-same [2.0] (4.0 unstable [i386])
-Conf foo-same:amd64 (4.0 unstable [amd64])
-Conf foo-same (4.0 unstable [i386])
 Inst foo-same-breaker-3 (1.0 unstable [i386])
 Inst foo-same-provider (1.0 unstable [i386])
+Conf foo-same (4.0 unstable [i386])
+Conf foo-same:amd64 (4.0 unstable [amd64])
 Conf foo-same-breaker-3 (1.0 unstable [i386])
 Conf foo-same-provider (1.0 unstable [i386])' aptget install foo-same-provider foo-same-breaker-3 -s
diff --git a/test/integration/test-prevent-markinstall-multiarch-same-versionscrew b/test/integration/test-prevent-markinstall-multiarch-same-versionscrew
index fed12da..d647856 100755
--- a/test/integration/test-prevent-markinstall-multiarch-same-versionscrew
+++ b/test/integration/test-prevent-markinstall-multiarch-same-versionscrew
@@ -55,14 +55,14 @@ Remv out-of-sync-gone-foreign:i386 [1]
 Remv out-of-sync-gone-native [1]
 Inst fine [1] (2 unstable [amd64]) [fine:amd64 on fine:i386] [fine:i386 on fine:amd64] [fine:i386 ]
 Inst fine:i386 [1] (2 unstable [i386])
-Conf fine (2 unstable [amd64])
-Conf fine:i386 (2 unstable [i386])
 Inst fine-installed [1] (2 unstable [amd64]) [fine-installed:amd64 on fine-installed:i386] [fine-installed:i386 on fine-installed:amd64] [fine-installed:i386 ]
 Inst fine-installed:i386 [1] (2 unstable [i386])
-Conf fine-installed (2 unstable [amd64])
-Conf fine-installed:i386 (2 unstable [i386])
 Inst out-of-sync-gone-foreign [1] (2 unstable [amd64])
 Inst out-of-sync-gone-native:i386 [1] (2 unstable [i386])
+Conf fine:i386 (2 unstable [i386])
+Conf fine (2 unstable [amd64])
+Conf fine-installed:i386 (2 unstable [i386])
+Conf fine-installed (2 unstable [amd64])
 Conf out-of-sync-gone-foreign (2 unstable [amd64])
 Conf out-of-sync-gone-native:i386 (2 unstable [i386])' aptget dist-upgrade -s #-o Debug::pkgDepCache::Marker=1
 
diff --git a/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages b/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages
index 5856cd7..c1d454f 100755
--- a/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages
+++ b/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages
@@ -39,9 +39,9 @@ Inst libreoffice-core [3] (4 sid [amd64]) [libreoffice-core:amd64 on libreoffice
 Inst libreoffice-common [3] (4 sid [all]) []
 Inst ure (4 sid [amd64])
 Conf ure (4 sid [amd64])
-Conf libreoffice-style-galaxy (4 sid [amd64])
 Conf libreoffice-common (4 sid [all])
 Conf libreoffice-core (4 sid [amd64])
+Conf libreoffice-style-galaxy (4 sid [amd64])
 Conf libreoffice (4 sid [amd64])' 'Reading package lists...
 Building dependency tree...
 The following NEW packages will be installed:
@@ -55,7 +55,7 @@ Inst libreoffice-core [3] (4 sid [amd64]) [libreoffice-common:amd64 on libreoffi
 Inst libreoffice-common [3] (4 sid [all]) []
 Inst ure (4 sid [amd64])
 Conf ure (4 sid [amd64])
-Conf libreoffice-style-galaxy (4 sid [amd64])
 Conf libreoffice-common (4 sid [all])
 Conf libreoffice-core (4 sid [amd64])
+Conf libreoffice-style-galaxy (4 sid [amd64])
 Conf libreoffice (4 sid [amd64])' aptget dist-upgrade -s

Attachment: signature.asc
Description: Digital signature


Reply to: