From 3090ae6972fd0e15767a96708c248f3ab87502f2 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 30 Aug 2015 22:34:28 +0200 Subject: [PATCH] detect and deal with indextarget duplicates MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Multiple targets downloading the same file is bad™ as it leads us to all sorts of problems like the acquire system breaking or simply a problem of which settings to use for them. Beside that this is most likely a mistake and silently ignoring it doesn't help the user realizing his mistake… On the other hand, we have 'duplicates' which are 'created' by how we create indextargets, so we have to prevent those from being created to but do not emit a warning for them as this is an implementation detail. And then, there is the absolute and most likely user mistake: Having the same target(s) activated in multiple entries. --- apt-pkg/deb/debmetaindex.cc | 61 ++++++++++++- apt-pkg/deb/debmetaindex.h | 3 +- apt-pkg/indexfile.cc | 1 + apt-pkg/indexfile.h | 1 + apt-pkg/sourcelist.cc | 11 +++ ...st-apt-acquire-additional-files-duplicates | 86 +++++++++++++++++++ 6 files changed, 160 insertions(+), 3 deletions(-) create mode 100755 test/integration/test-apt-acquire-additional-files-duplicates diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc index 419de12e8..b381f5f85 100644 --- a/apt-pkg/deb/debmetaindex.cc +++ b/apt-pkg/deb/debmetaindex.cc @@ -33,6 +33,7 @@ class APT_HIDDEN debReleaseIndexPrivate /*{{{*/ public: struct APT_HIDDEN debSectionEntry { + std::string sourcesEntry; std::string Name; std::vector Targets; std::vector Architectures; @@ -186,6 +187,58 @@ static void GetIndexTargetsFor(char const * const Type, std::string const &URI, LongDesc = SubstVar(LongDesc, std::string("$(") + O->first + ")", O->second); } + { + auto const dup = std::find_if(IndexTargets.begin(), IndexTargets.end(), [&](IndexTarget const &IT) { + return MetaKey == IT.MetaKey && baseURI == IT.Option(IndexTarget::BASE_URI) && + E->sourcesEntry == IT.Option(IndexTarget::SOURCESENTRY) && *T == IT.Option(IndexTarget::CREATED_BY); + }); + if (dup != IndexTargets.end()) + { + if (tplMetaKey.find("$(ARCHITECTURE)") == std::string::npos) + break; + continue; + } + } + + { + auto const dup = std::find_if(IndexTargets.begin(), IndexTargets.end(), [&](IndexTarget const &IT) { + return MetaKey == IT.MetaKey && baseURI == IT.Option(IndexTarget::BASE_URI) && + E->sourcesEntry == IT.Option(IndexTarget::SOURCESENTRY) && *T != IT.Option(IndexTarget::CREATED_BY); + }); + if (dup != IndexTargets.end()) + { + std::string const dupT = dup->Option(IndexTarget::CREATED_BY); + std::string const dupEntry = dup->Option(IndexTarget::SOURCESENTRY); + //TRANSLATOR: an identifier like Packages; Releasefile key indicating + // a file like main/binary-amd64/Packages; another identifier like Contents; + // filename and linenumber of the sources.list entry currently parsed + _error->Warning(_("Target %s wants to acquire the same file (%s) as %s from source %s"), + T->c_str(), MetaKey.c_str(), dupT.c_str(), dupEntry.c_str()); + if (tplMetaKey.find("$(ARCHITECTURE)") == std::string::npos) + break; + continue; + } + } + + { + auto const dup = std::find_if(IndexTargets.begin(), IndexTargets.end(), [&](IndexTarget const &T) { + return MetaKey == T.MetaKey && baseURI == T.Option(IndexTarget::BASE_URI) && + E->sourcesEntry != T.Option(IndexTarget::SOURCESENTRY); + }); + if (dup != IndexTargets.end()) + { + std::string const dupEntry = dup->Option(IndexTarget::SOURCESENTRY); + //TRANSLATOR: an identifier like Packages; Releasefile key indicating + // a file like main/binary-amd64/Packages; filename and linenumber of + // two sources.list entries + _error->Warning(_("Target %s (%s) is configured multiple times in %s and %s"), + T->c_str(), MetaKey.c_str(), dupEntry.c_str(), E->sourcesEntry.c_str()); + if (tplMetaKey.find("$(ARCHITECTURE)") == std::string::npos) + break; + continue; + } + } + // not available in templates, but in the indextarget Options.insert(std::make_pair("BASE_URI", baseURI)); Options.insert(std::make_pair("REPO_URI", URI)); @@ -194,6 +247,7 @@ static void GetIndexTargetsFor(char const * const Type, std::string const &URI, Options.insert(std::make_pair("PDIFFS", UsePDiffs ? "yes" : "no")); Options.insert(std::make_pair("DEFAULTENABLED", DefaultEnabled ? "yes" : "no")); Options.insert(std::make_pair("COMPRESSIONTYPES", CompressionTypes)); + Options.insert(std::make_pair("SOURCESENTRY", E->sourcesEntry)); IndexTarget Target( MetaKey, @@ -227,7 +281,8 @@ std::vector debReleaseIndex::GetIndexTargets() const return IndexTargets; } /*}}}*/ -void debReleaseIndex::AddComponent(bool const isSrc, std::string const &Name,/*{{{*/ +void debReleaseIndex::AddComponent(std::string const &sourcesEntry, /*{{{*/ + bool const isSrc, std::string const &Name, std::vector const &Targets, std::vector const &Architectures, std::vector Languages, @@ -236,7 +291,7 @@ void debReleaseIndex::AddComponent(bool const isSrc, std::string const &Name,/*{ if (Languages.empty() == true) Languages.push_back("none"); debReleaseIndexPrivate::debSectionEntry const entry = { - Name, Targets, Architectures, Languages, usePDiffs + sourcesEntry, Name, Targets, Architectures, Languages, usePDiffs }; if (isSrc) d->DebSrcEntries.push_back(entry); @@ -768,7 +823,9 @@ class APT_HIDDEN debSLTypeDebian : public pkgSourceList::Type /*{{{*/ UsePDiffs = StringToBool(opt->second); } + auto const entry = Options.find("sourceslist-entry"); Deb->AddComponent( + entry->second, IsSrc, Section, mytargets, diff --git a/apt-pkg/deb/debmetaindex.h b/apt-pkg/deb/debmetaindex.h index bba0e344f..419cbdc9d 100644 --- a/apt-pkg/deb/debmetaindex.h +++ b/apt-pkg/deb/debmetaindex.h @@ -59,7 +59,8 @@ class APT_HIDDEN debReleaseIndex : public metaIndex virtual bool IsTrusted() const APT_OVERRIDE; - void AddComponent(bool const isSrc, std::string const &Name, + void AddComponent(std::string const &sourcesEntry, + bool const isSrc, std::string const &Name, std::vector const &Targets, std::vector const &Architectures, std::vector Languages, diff --git a/apt-pkg/indexfile.cc b/apt-pkg/indexfile.cc index 1ed798410..db57faf07 100644 --- a/apt-pkg/indexfile.cc +++ b/apt-pkg/indexfile.cc @@ -147,6 +147,7 @@ std::string IndexTarget::Option(OptionKeys const EnumKey) const /*{{{*/ APT_CASE(PDIFFS); APT_CASE(DEFAULTENABLED); APT_CASE(COMPRESSIONTYPES); + APT_CASE(SOURCESENTRY); #undef APT_CASE case FILENAME: return _config->FindDir("Dir::State::lists") + URItoFileName(URI); case EXISTING_FILENAME: diff --git a/apt-pkg/indexfile.h b/apt-pkg/indexfile.h index f267247c1..a09c39057 100644 --- a/apt-pkg/indexfile.h +++ b/apt-pkg/indexfile.h @@ -88,6 +88,7 @@ class IndexTarget /*{{{*/ PDIFFS, COMPRESSIONTYPES, DEFAULTENABLED, + SOURCESENTRY, }; std::string Option(OptionKeys const Key) const; bool OptionBool(OptionKeys const Key) const; diff --git a/apt-pkg/sourcelist.cc b/apt-pkg/sourcelist.cc index b083da936..31d87a403 100644 --- a/apt-pkg/sourcelist.cc +++ b/apt-pkg/sourcelist.cc @@ -119,6 +119,12 @@ bool pkgSourceList::Type::ParseStanza(vector &List, /*{{{*/ Options[m->second.first] = option; } + { + std::string entry; + strprintf(entry, "%s:%i", Fd.Name().c_str(), i); + Options["sourceslist-entry"] = entry; + } + // now create one item per suite/section string Suite = Tags.FindS("Suites"); Suite = SubstVar(Suite,"$(ARCH)",_config->Find("APT::Architecture")); @@ -186,6 +192,11 @@ bool pkgSourceList::Type::ParseLine(vector &List, // Parse option field if it exists // e.g.: [ option1=value1 option2=value2 ] map Options; + { + std::string entry; + strprintf(entry, "%s:%i", File.c_str(), CurLine); + Options["sourceslist-entry"] = entry; + } if (Buffer != 0 && Buffer[0] == '[') { ++Buffer; // ignore the [ diff --git a/test/integration/test-apt-acquire-additional-files-duplicates b/test/integration/test-apt-acquire-additional-files-duplicates new file mode 100755 index 000000000..dbfc0eb74 --- /dev/null +++ b/test/integration/test-apt-acquire-additional-files-duplicates @@ -0,0 +1,86 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' 'i386' + +cat > rootdir/etc/apt/apt.conf.d/content-target.conf < rootdir/etc/apt/sources.list <> rootdir/etc/apt/sources.list <> rootdir/etc/apt/sources.list <