On Thu, Jan 30, 2014 at 03:42:13PM +0100, Julian Andres Klode wrote: > On Thu, Jan 30, 2014 at 12:27:21PM +0000, Wookey wrote: > > +++ Julian Andres Klode [2014-01-30 08:12 +0100]: > > > On Thu, Jan 30, 2014 at 03:13:16AM +0000, Wookey wrote: > > The problem is that in order to debootstrap you need all the packages in > > one repo so leaving the arch all packages in ftp.uk.debian.org means you > > can't debootstrap if you only uploaded the new-arch 'any' packages to > > the 'bootstrap' repo. It's also important to test that the arch-all > > build actually works, and not just the arch-any part so doing those > > builds and testing the results can be good. > > A work around might be to reorder sources.list entries. The order of > those entries determines from which source a package is retrieved, I > believe the first match takes precedence.
The first one parsed decides which size is expected – and usually this is also the one the package is acquired from, with the notable exception of not downloading from an unsigned archive if a signed is available… so, as this bootstrap archive is signed, is the key installed? > > It's fine for apt to consider these packages to be functionally > > equivalent, but it does need to check the correct checksum on download. > > It seems to me that this can be fixed by either adding size/hash to the > > hash as you suggest(making them 'different packages', or just separately > > ensuring that the checksum for the repo/file that was downloaded is > > used. Apt knows that there is more than one repo source for this > > package, but doesn't record that there might be more than one checksum? > > The fact that it can end up choosing one checksum and another source > > does seem wrong. Perhaps the code/object structure makes it hard to fix > > this this way and your fix is the only one that makes sense? > > It seems right to me in this case, because otherwise functional aspects > like dependencies could differ as well. And if APT uses the dependencies > from one source and then fetches the package from another source, but that > one has different dependencies, installing it would produce an error. This situation can't happen as you have yourself lined out that Depends will influence the CRC hash, so they would get recognized as different versions. That said, what could happen at the moment is that a package could differ just by Multi-Arch field. (minus hash collisions, but how likely is that…) > > > An alternative would be to change the cache-building algorithms to look > > > at SHA hashes and/or size and create different version entries in the > > > cache > > > if they are present in both versions, but different. SHA Hashes would > > > require > > > all repositories to use the same best checksum algorithm. > > > > I think just adding size to the hash would be cheap and easy and would > > largely solve this problem. Adding the hash would cover a few extra > > cases where the size came out the same too, but if it's difficult I'd be > > happy to have this mostly-solved, as it's a situation we normally try to > > avoid anyway. > > Adding the size to the hash is not possible, as dpkg does not store the > size for installed packages. This would mean that an installed package > always has a different hash than an available package, causing APT to > go crazy (it would try to "upgrade" all installed packages...). We could compare the size of the currently parsed version with the size of the version we compare it with at the moment through (as long as the current one isn't the status file one). See attached demo-patch. Something like that (but tested, this one isn't) could be introduced with the next abi break. It isn't bulletproof either, but a bit better. (I wonder if it would make sense to move the comparison entirely into such an on-demand handling rather than this generate CRC for everyone.) Best regards David Kalnischkies
diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc
index 68d544e..4fe5919 100644
--- a/apt-pkg/deb/deblistparser.cc
+++ b/apt-pkg/deb/deblistparser.cc
@@ -95,44 +95,51 @@ string debListParser::Version()
return Section.FindS("Version");
}
/*}}}*/
-// ListParser::NewVersion - Fill in the version structure /*{{{*/
-// ---------------------------------------------------------------------
-/* */
-bool debListParser::NewVersion(pkgCache::VerIterator &Ver)
+unsigned char debListParser::ParseMultiArch(bool const showErrors) /*{{{*/
{
- // Parse the section
- Ver->Section = UniqFindTagWrite("Section");
-
- // Parse multi-arch
+ unsigned char MA;
string const MultiArch = Section.FindS("Multi-Arch");
if (MultiArch.empty() == true)
- Ver->MultiArch = pkgCache::Version::None;
+ MA = pkgCache::Version::None;
else if (MultiArch == "same") {
// Parse multi-arch
if (ArchitectureAll() == true)
{
/* Arch all packages can't be Multi-Arch: same */
- _error->Warning("Architecture: all package '%s' can't be Multi-Arch: same",
- Section.FindS("Package").c_str());
- Ver->MultiArch = pkgCache::Version::None;
+ if (showErrors == true)
+ _error->Warning("Architecture: all package '%s' can't be Multi-Arch: same",
+ Section.FindS("Package").c_str());
+ MA = pkgCache::Version::None;
}
else
- Ver->MultiArch = pkgCache::Version::Same;
+ MA = pkgCache::Version::Same;
}
else if (MultiArch == "foreign")
- Ver->MultiArch = pkgCache::Version::Foreign;
+ MA = pkgCache::Version::Foreign;
else if (MultiArch == "allowed")
- Ver->MultiArch = pkgCache::Version::Allowed;
+ MA = pkgCache::Version::Allowed;
else
{
- _error->Warning("Unknown Multi-Arch type '%s' for package '%s'",
- MultiArch.c_str(), Section.FindS("Package").c_str());
- Ver->MultiArch = pkgCache::Version::None;
+ if (showErrors == true)
+ _error->Warning("Unknown Multi-Arch type '%s' for package '%s'",
+ MultiArch.c_str(), Section.FindS("Package").c_str());
+ MA = pkgCache::Version::None;
}
if (ArchitectureAll() == true)
- Ver->MultiArch |= pkgCache::Version::All;
+ MA |= pkgCache::Version::All;
+ return MA;
+}
+ /*}}}*/
+// ListParser::NewVersion - Fill in the version structure /*{{{*/
+// ---------------------------------------------------------------------
+/* */
+bool debListParser::NewVersion(pkgCache::VerIterator &Ver)
+{
+ // Parse the section
+ Ver->Section = UniqFindTagWrite("Section");
+ Ver->MultiArch = ParseMultiArch(true);
// Archive Size
Ver->Size = Section.FindULL("Size");
// Unpacked Size (in K)
@@ -315,6 +322,25 @@ unsigned short debListParser::VersionHash()
return Result;
}
/*}}}*/
+bool debListParser::SameVersion(unsigned short const Hash, /*{{{*/
+ pkgCache::VerIterator const &Ver)
+{
+ if (Hash != Ver->Hash)
+ return false;
+ // status file has no (Download)Size, but all others are fair game
+ // status file is parsed last, so the first version we encounter is
+ // probably also the version we have downloaded
+ unsigned long long const Size = Section.FindULL("Size");
+ if (Size != 0 && Size != Ver->Size)
+ return false;
+ // available everywhere, but easier to check here than to include in VersionHash
+ unsigned char MultiArch = ParseMultiArch(false);
+ if (MultiArch != Ver->MultiArch)
+ return false;
+ // for all practical proposes (we can check): same version
+ return true;
+}
+ /*}}}*/
// ListParser::ParseStatus - Parse the status field /*{{{*/
// ---------------------------------------------------------------------
/* Status lines are of the form,
diff --git a/apt-pkg/deb/deblistparser.h b/apt-pkg/deb/deblistparser.h
index 386d291..aca6fc6 100644
--- a/apt-pkg/deb/deblistparser.h
+++ b/apt-pkg/deb/deblistparser.h
@@ -63,6 +63,7 @@ class debListParser : public pkgCacheGenerator::ListParser
virtual std::string DescriptionLanguage();
virtual MD5SumValue Description_md5();
virtual unsigned short VersionHash();
+ virtual bool SameVersion(unsigned short const Hash, pkgCache::VerIterator const &Ver);
virtual bool UsePackage(pkgCache::PkgIterator &Pkg,
pkgCache::VerIterator &Ver);
virtual unsigned long Offset() {return iOffset;};
@@ -81,6 +82,9 @@ class debListParser : public pkgCacheGenerator::ListParser
debListParser(FileFd *File, std::string const &Arch = "");
virtual ~debListParser() {};
+
+ private:
+ unsigned char ParseMultiArch(bool const showErrors);
};
#endif
diff --git a/apt-pkg/pkgcache.cc b/apt-pkg/pkgcache.cc
index 52e814c..f8959aa 100644
--- a/apt-pkg/pkgcache.cc
+++ b/apt-pkg/pkgcache.cc
@@ -52,7 +52,7 @@ pkgCache::Header::Header()
/* Whenever the structures change the major version should be bumped,
whenever the generator changes the minor version should be bumped. */
MajorVersion = 8;
- MinorVersion = 1;
+ MinorVersion = 2;
Dirty = false;
HeaderSz = sizeof(pkgCache::Header);
diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc
index 7ce7aba..0df8475 100644
--- a/apt-pkg/pkgcachegen.cc
+++ b/apt-pkg/pkgcachegen.cc
@@ -350,7 +350,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
map_ptrloc *LastVer = &Pkg->VersionList;
void const * oldMap = Map.Data();
- unsigned long const Hash = List.VersionHash();
+ unsigned short const Hash = List.VersionHash();
if (Ver.end() == false)
{
/* We know the list is sorted so we use that fact in the search.
@@ -363,7 +363,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
if (Res > 0)
break;
// Versionstrings are equal - is hash also equal?
- if (Res == 0 && Ver->Hash == Hash)
+ if (Res == 0 && List.SameVersion(Hash, Ver) == true)
break;
// proceed with the next till we have either the right
// or we found another version (which will be lower)
@@ -552,12 +552,12 @@ bool pkgCacheGenerator::MergeFileProvides(ListParser &List)
if (Counter % 100 == 0 && Progress != 0)
Progress->Progress(List.Offset());
- unsigned long Hash = List.VersionHash();
+ unsigned short Hash = List.VersionHash();
pkgCache::VerIterator Ver = Pkg.VersionList();
Dynamic<pkgCache::VerIterator> DynVer(Ver);
for (; Ver.end() == false; ++Ver)
{
- if (Ver->Hash == Hash && Version == Ver.VerStr())
+ if (List.SameVersion(Hash, Ver) == true && Version == Ver.VerStr())
{
if (List.CollectFileProvides(Cache,Ver) == false)
return _error->Error(_("Error occurred while processing %s (%s%d)"),
@@ -1045,6 +1045,12 @@ bool pkgCacheGenerator::ListParser::NewProvides(pkgCache::VerIterator &Ver,
return true;
}
/*}}}*/
+bool pkgCacheGenerator::ListParser::SameVersion(unsigned short const Hash,/*{{{*/
+ pkgCache::VerIterator const &Ver)
+{
+ return Hash == Ver->Hash;
+}
+ /*}}}*/
// CacheGenerator::SelectFile - Select the current file being parsed /*{{{*/
// ---------------------------------------------------------------------
/* This is used to select which file is to be associated with all newly
diff --git a/apt-pkg/pkgcachegen.h b/apt-pkg/pkgcachegen.h
index 428e845..4991678 100644
--- a/apt-pkg/pkgcachegen.h
+++ b/apt-pkg/pkgcachegen.h
@@ -162,6 +162,12 @@ class pkgCacheGenerator::ListParser
virtual std::string DescriptionLanguage() = 0;
virtual MD5SumValue Description_md5() = 0;
virtual unsigned short VersionHash() = 0;
+ /** compare currently parsed version with given version
+ *
+ * \param Hash of the currently parsed version
+ * \param Ver to compare with
+ */
+ virtual bool SameVersion(unsigned short const Hash, pkgCache::VerIterator const &Ver);
virtual bool UsePackage(pkgCache::PkgIterator &Pkg,
pkgCache::VerIterator &Ver) = 0;
virtual unsigned long Offset() = 0;
signature.asc
Description: Digital signature
