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

Bug#765458: apt: broken cdrom support, breaking installation from weekly ISO images



Hi,

On Wed, Oct 15, 2014 at 11:47:44AM +0200, Cyril Brulebois wrote:
> [ X-D-Cc: debian-boot@, please keep it in the loop when replying. ]

[Done, although I don't see the header… (bad mutt, bad).]


> we received several bug reports about weekly installation images being
> unable to find a kernel package to install on the freshly debootstrapped
> system. I've been able to replicate this issue with apt 1.9.0.2. Various

Is the "apt-get update" call from which you have included the output
a recent addition or was it 'always' there?

I am asking as 'apt-cdrom add' actually does the work of copying the
indexes to disk, which (should) mean that 'apt-get update' is a no-op,
making the call useless if cdroms are really the only possible source in
that stage.

That 'update' isn't really supposed to be called here is reinforced by
the ugly warnings/errors you showcase and which existed since ever. Our
only testcase covering apt-cdrom also doesn't include such a call…

Why that is the case, I have no idea, as I would expect at least some
people to have multiple sources, including cdrom, which would call for
an update, so that should really work without being scary (= assuming
warnings from apt are regarded as scary…).

The irony is that the suspected bad-boy 80174528 actually fixes this
longstanding problem as the regression was that apt exited nonzero, not
that it printed warnings (so much for scary).

The problematic commit is b0f4b486 (and therefore not a regression in
a security fix – everyone rejoice): While fine by itself, merged with
the suspected bad-boy we still have no warnings and a successful exit,
but our helpful list-cleanup kicks in removing files from the lists/
directory which seem to be orphaned given that it is looking e.g. for
a Packages.gz file and not for Packages as the part fixing up the
filename is skipped if a cdrom source is encountered.


The attached patch should merge both in a better working way, at least
that is what the testcase is promising me – which I have extended a bit
to cover a bit more ground, too. Nothing near proper testing though, so
someone giving it a proper testspin would be nice, but if that is too
hard I guess Michael could just upload it and let the world test it for
us (now that he doesn't have to fear another security upload).  ;)


Best regards

David Kalnischkies
commit 5afcfe2a51a9e47e95023b99bcab065d1975e950
Author: David Kalnischkies <david@kalnischkies.de>
Date:   Wed Oct 15 15:56:53 2014 +0200

    don't cleanup cdrom files in apt-get update
    
    Regression from merging 801745284905e7962aa77a9f37a6b4e7fcdc19d0 and
    b0f4b486e6850c5f98520ccf19da71d0ed748ae4. While fine by itself, merged
    the part fixing the filename is skipped if a cdrom source is
    encountered, so that our list-cleanup removes what seems to be orphaned
    files.
    
    Closes: 765458

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 2401364..253cbda 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -1144,16 +1144,12 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
    else
       Local = true;
 
-   // do not reverify cdrom sources as apt-cdrom may rewrite the Packages
-   // file when its doing the indexcopy
-   if (RealURI.substr(0,6) == "cdrom:" &&
-       StringToBool(LookupTag(Message,"IMS-Hit"),false) == true)
-      return;
-
    // The files timestamp matches, for non-local URLs reverify the local
    // file, for local file, uncompress again to ensure the hashsum is still
    // matching the Release file
-   if (!Local && StringToBool(LookupTag(Message,"IMS-Hit"),false) == true)
+   bool const IsCDROM = RealURI.substr(0,6) == "cdrom:";
+   if ((Local == false || IsCDROM == true) &&
+	 StringToBool(LookupTag(Message,"IMS-Hit"),false) == true)
    {
       // set destfile to the final destfile
       if(_config->FindB("Acquire::GzipIndexes",false) == false)
@@ -1162,7 +1158,10 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
          DestFile += URItoFileName(RealURI);
       }
 
-      ReverifyAfterIMS(FileName);
+      // do not reverify cdrom sources as apt-cdrom may rewrite the Packages
+      // file when its doing the indexcopy
+      if (IsCDROM == false)
+	 ReverifyAfterIMS(FileName);
       return;
    }
    string decompProg;
diff --git a/test/integration/test-apt-cdrom b/test/integration/test-apt-cdrom
index 8d8fdf1..44eccb7 100755
--- a/test/integration/test-apt-cdrom
+++ b/test/integration/test-apt-cdrom
@@ -66,22 +66,51 @@ CD_LABEL="$(echo "$ident" | grep "^Stored label:" | head -n1 | sed "s/^[^:]*: //
 testequal "CD::${CD_ID} \"${CD_LABEL}\";
 CD::${CD_ID}::Label \"${CD_LABEL}\";" cat rootdir/var/lib/apt/cdroms.list
 
-testequal 'Reading package lists...
+testcdromusage() {
+	touch rootdir/var/lib/apt/extended_states
+
+	testequal 'Reading package lists...
 Building dependency tree...
+Reading state information...
 The following NEW packages will be installed:
   testing
 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
 Inst testing (0.8.15 stable [amd64])
 Conf testing (0.8.15 stable [amd64])' aptget install testing -s
 
-testequal 'Reading package lists...
+	testdpkgnotinstalled testing
+	testsuccess aptget install testing -y
+	testdpkginstalled testing
+	testsuccess aptget purge testing -y
+	testdpkgnotinstalled testing
+
+	testequal 'Reading package lists...
 Building dependency tree...
+Reading state information...
 The following NEW packages will be installed:
   testing:i386
 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
 Inst testing:i386 (0.8.15 stable [i386])
 Conf testing:i386 (0.8.15 stable [i386])' aptget install testing:i386 -s
 
+	testdpkgnotinstalled testing:i386
+	testsuccess aptget install testing:i386 -y
+	testdpkginstalled testing:i386
+	testsuccess aptget purge testing:i386 -y
+	testdpkgnotinstalled testing:i386
+
+	rm -f testing_0.8.15_amd64.deb
+	testsuccess aptget download testing
+	testsuccess test -s testing_0.8.15_amd64.deb
+	rm -f testing_0.8.15_amd64.deb
+
+	rm -f testing_0.8.15.dsc
+	testsuccess aptget source testing --dsc-only -d
+	testsuccess test -s testing_0.8.15.dsc
+	rm -f testing_0.8.15.dsc
+}
+testcdromusage
+
 # check Idempotence of apt-cdrom (and disabling of Translation dropping)
 testequal "$CDROM_PRE
 Found 2 package indexes, 1 source indexes, 2 translation indexes and 1 signatures
@@ -101,7 +130,15 @@ $CDROM_POST" aptcdromlog add
 msgtest 'Test for the english description translation of' 'testing'
 aptcache show testing -o Acquire::Languages=en | grep -q '^Description-en: ' && msgpass || msgfail
 
-# check that we really can install from a 'cdrom'
-testdpkgnotinstalled testing
-testsuccess aptget install testing -y
-testdpkginstalled testing
+# ensure cdrom method isn't trying to mount the cdrom
+mv rootdir/media/cdrom-unmounted rootdir/media/cdrom-ejected
+# ensure an update doesn't mess with cdrom sources
+testsuccess aptget update
+testfileequal rootdir/tmp/testsuccess.output 'Reading package lists...'
+mv rootdir/media/cdrom-ejected rootdir/media/cdrom-unmounted
+testcdromusage
+
+# and again to check that it withstands the temptation even if it could mount
+testsuccess aptget update
+testfileequal rootdir/tmp/testsuccess.output 'Reading package lists...'
+testcdromusage

Attachment: signature.asc
Description: Digital signature


Reply to: