detect and deal with indextarget duplicates

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.

diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc
index 419de12e8669817499b0acc012d79d562e89bd11..b381f5f85bad2e22c2078b520bf1998c650872c4 100644 (file)
--- 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<std::string> Targets;
       std::vector<std::string> 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<IndexTarget> 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<std::string> const &Targets,
         std::vector<std::string> const &Architectures,
         std::vector<std::string> 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 bba0e344f87fd3b6576874ba886d230eeebabaf8..419cbdc9dc72e958a23c40ff64062f36a5f7c433 100644 (file)
--- 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<std::string> const &Targets,
         std::vector<std::string> const &Architectures,
         std::vector<std::string> Languages,

diff --git a/apt-pkg/indexfile.cc b/apt-pkg/indexfile.cc
index 1ed7984101f03bf8a2a418ce21cd07c140462620..db57faf07db852b3e6e2ef2d67162d337305be68 100644 (file)
--- 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 f267247c17409046eb910f906e4a8a1bbc98adf7..a09c39057518f220f2613a3827a4f887a16d3681 100644 (file)
--- 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 b083da936d6ff19ab4c439fff667ec68647ec581..31d87a403a24b711a9a6a7f3d820daaecc3bf03e 100644 (file)
--- a/apt-pkg/sourcelist.cc
+++ b/apt-pkg/sourcelist.cc
@@ -119,6 +119,12 @@ bool pkgSourceList::Type::ParseStanza(vector<metaIndex *> &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<metaIndex *> &List,
    // Parse option field if it exists
    // e.g.: [ option1=value1 option2=value2 ]
    map<string, string> 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 (executable)
index 0000000..dbfc0eb
--- /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 <<EOF
+Acquire::IndexTargets::deb::Contents1 {
+       MetaKey "\$(COMPONENT)/Contents-\$(ARCHITECTURE)";
+       ShortDescription "Contents";
+       Description "\$(RELEASE) Contents";
+       DefaultEnabled "false";
+};
+Acquire::IndexTargets::deb::Contents2 {
+       MetaKey "\$(COMPONENT)/Contents-\$(ARCHITECTURE)";
+       ShortDescription "Contents";
+       Description "\$(RELEASE) Contents";
+       DefaultEnabled "false";
+};
+Acquire::IndexTargets::deb::Contentsflat {
+       MetaKey "Contents-\$(ARCHITECTURE)";
+       ShortDescription "Contents";
+       Description "\$(RELEASE) Contents";
+       DefaultEnabled "false";
+};
+EOF
+
+mkdir -p ./rootdir/var/lib/apt/lists
+APTLISTS="$(readlink -f ./rootdir/var/lib/apt/lists)"
+APTETC="$(readlink -f ./rootdir/etc/apt)"
+cat > rootdir/etc/apt/sources.list <<EOF
+deb http://example.org/debian stable main rocks
+deb-src http://example.org/debian stable main rocks
+EOF
+testsuccessequal "${APTLISTS}/example.org_debian_dists_stable_main_source_Sources
+${APTLISTS}/example.org_debian_dists_stable_rocks_source_Sources
+${APTLISTS}/example.org_debian_dists_stable_main_binary-amd64_Packages
+${APTLISTS}/example.org_debian_dists_stable_main_binary-i386_Packages
+${APTLISTS}/example.org_debian_dists_stable_main_i18n_Translation-en
+${APTLISTS}/example.org_debian_dists_stable_rocks_binary-amd64_Packages
+${APTLISTS}/example.org_debian_dists_stable_rocks_binary-i386_Packages
+${APTLISTS}/example.org_debian_dists_stable_rocks_i18n_Translation-en" aptget indextargets --no-release-info --format '$(FILENAME)'
+
+cat >> rootdir/etc/apt/sources.list <<EOF
+deb http://example.org/debian stable main rocks
+EOF
+testwarningequal "${APTLISTS}/example.org_debian_dists_stable_main_source_Sources
+${APTLISTS}/example.org_debian_dists_stable_rocks_source_Sources
+${APTLISTS}/example.org_debian_dists_stable_main_binary-amd64_Packages
+${APTLISTS}/example.org_debian_dists_stable_main_binary-i386_Packages
+${APTLISTS}/example.org_debian_dists_stable_main_i18n_Translation-en
+${APTLISTS}/example.org_debian_dists_stable_rocks_binary-amd64_Packages
+${APTLISTS}/example.org_debian_dists_stable_rocks_binary-i386_Packages
+${APTLISTS}/example.org_debian_dists_stable_rocks_i18n_Translation-en
+W: Target Packages (main/binary-amd64/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Packages (main/binary-i386/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Translations (main/i18n/Translation-en) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Packages (rocks/binary-amd64/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Packages (rocks/binary-i386/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Translations (rocks/i18n/Translation-en) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3" aptget indextargets --no-release-info --format '$(FILENAME)'
+
+cat >> rootdir/etc/apt/sources.list <<EOF
+deb [target=Contents1,Contents2,Contentsflat arch=amd64] http://example.org/debian stable main rocks
+deb-src [target=Contents1,Contents2,Contentsflat arch=amd64] http://example.org/debian stable main rocks
+EOF
+testwarningequal "${APTLISTS}/example.org_debian_dists_stable_main_source_Sources
+${APTLISTS}/example.org_debian_dists_stable_rocks_source_Sources
+${APTLISTS}/example.org_debian_dists_stable_main_binary-amd64_Packages
+${APTLISTS}/example.org_debian_dists_stable_main_binary-i386_Packages
+${APTLISTS}/example.org_debian_dists_stable_main_i18n_Translation-en
+${APTLISTS}/example.org_debian_dists_stable_rocks_binary-amd64_Packages
+${APTLISTS}/example.org_debian_dists_stable_rocks_binary-i386_Packages
+${APTLISTS}/example.org_debian_dists_stable_rocks_i18n_Translation-en
+${APTLISTS}/example.org_debian_dists_stable_main_Contents-amd64
+${APTLISTS}/example.org_debian_dists_stable_Contents-amd64
+${APTLISTS}/example.org_debian_dists_stable_rocks_Contents-amd64
+W: Target Packages (main/binary-amd64/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Packages (main/binary-i386/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Translations (main/i18n/Translation-en) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Packages (rocks/binary-amd64/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Packages (rocks/binary-i386/Packages) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Translations (rocks/i18n/Translation-en) is configured multiple times in ${APTETC}/sources.list:1 and ${APTETC}/sources.list:3
+W: Target Contents2 wants to acquire the same file (main/Contents-amd64) as Contents1 from source ${APTETC}/sources.list:4
+W: Target Contents2 wants to acquire the same file (rocks/Contents-amd64) as Contents1 from source ${APTETC}/sources.list:4" aptget indextargets --no-release-info --format '$(FILENAME)'