merge sources.list lines based on Release filename

Merging by URI means that having sources lines with different URI
methods results in 'strange' warning and error messages, which aren't
very friendly from a user point of view as not encoding the method in
the filename is effectivly an implementation detail.

Merging by filename removes these messages and makes everything "work"
even if it isn't working the way it is configured as the indexes aren't
acquired over the method given, but over the first method for this
release file (which argueably is an implementation detail stemming from
the filename encoding, too).

So either direction isn't perfectly "right", but personally I prefer
"magic" over strange error messages (and doing a full-circle detection
of this with its own messages which would need to be translated feels
like way too much effort for dubious gain).

Closes: 826944

diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc
index f756cdb1f4acfc09312b0e4c7d900878eed394e1..c70c39a45e49daef26e0adfea61c3835d3233822 100644 (file)
--- a/apt-pkg/deb/debmetaindex.cc
+++ b/apt-pkg/deb/debmetaindex.cc
@@ -80,20 +80,19 @@ std::string debReleaseIndex::MetaIndexFile(const char *Type) const
    return _config->FindDir("Dir::State::lists") +
       URItoFileName(MetaIndexURI(Type));
 }
    return _config->FindDir("Dir::State::lists") +
       URItoFileName(MetaIndexURI(Type));
 }

-
-std::string debReleaseIndex::MetaIndexURI(const char *Type) const
+static std::string constructMetaIndexURI(std::string URI, std::string const &Dist, char const * const Type)

 {
 {

-   std::string Res;
-

    if (Dist == "/")
    if (Dist == "/")

-      Res = URI;
+      ;

    else if (Dist[Dist.size()-1] == '/')
    else if (Dist[Dist.size()-1] == '/')

-      Res = URI + Dist;
+      URI += Dist;

    else
    else

-      Res = URI + "dists/" + Dist + "/";
-   
-   Res += Type;
-   return Res;
+      URI += "dists/" + Dist + "/";
+   return URI + Type;
+}
+std::string debReleaseIndex::MetaIndexURI(const char *Type) const
+{
+   return constructMetaIndexURI(URI, Dist, Type);

 }
                                                                        /*}}}*/
 // ReleaseIndex Con- and Destructors                                   /*{{{*/
 }
                                                                        /*}}}*/
 // ReleaseIndex Con- and Destructors                                   /*{{{*/


@@ -919,27 +918,30 @@ class APT_HIDDEN debSLTypeDebian : public pkgSourceList::Type             /*{{{*/
                           std::string const &Dist, std::string const &Section,
                           bool const &IsSrc, std::map<std::string, std::string> const &Options) const
    {
                           std::string const &Dist, std::string const &Section,
                           bool const &IsSrc, std::map<std::string, std::string> const &Options) const
    {

-      debReleaseIndex *Deb = NULL;
-      for (std::vector<metaIndex *>::const_iterator I = List.begin();
-          I != List.end(); ++I)
+      debReleaseIndex * Deb = nullptr;
+      std::string const FileName = URItoFileName(constructMetaIndexURI(URI, Dist, "Release"));
+      for (auto const &I: List)

       {
         // We only worry about debian entries here
       {
         // We only worry about debian entries here

-        if (strcmp((*I)->GetType(), "deb") != 0)
+        if (strcmp(I->GetType(), "deb") != 0)
+           continue;
+
+        auto const D = dynamic_cast<debReleaseIndex*>(I);
+        if (unlikely(D == nullptr))

            continue;
 
            continue;
 

-        /* This check insures that there will be only one Release file
+        /* This check ensures that there will be only one Release file

            queued for all the Packages files and Sources files it
            corresponds to. */
            queued for all the Packages files and Sources files it
            corresponds to. */

-        if ((*I)->GetURI() == URI && (*I)->GetDist() == Dist)
+        if (URItoFileName(D->MetaIndexURI("Release")) == FileName)

         {
         {

-           Deb = dynamic_cast<debReleaseIndex*>(*I);
-           if (Deb != NULL)
-              break;
+           Deb = D;
+           break;

         }
       }
 
       // No currently created Release file indexes this entry, so we create a new one.
         }
       }
 
       // No currently created Release file indexes this entry, so we create a new one.

-      if (Deb == NULL)
+      if (Deb == nullptr)

       {
         Deb = new debReleaseIndex(URI, Dist);
         List.push_back(Deb);
       {
         Deb = new debReleaseIndex(URI, Dist);
         List.push_back(Deb);

diff --git a/test/integration/test-different-methods-for-same-source b/test/integration/test-different-methods-for-same-source
new file mode 100755 (executable)
index 0000000..7b78841
--- /dev/null
+++ b/test/integration/test-different-methods-for-same-source
@@ -0,0 +1,33 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64'
+
+insertpackage 'stable' 'foo' 'all' '1'
+insertsource 'stable' 'foo' 'all' '1'
+setupaptarchive --no-update
+
+# install a slowed down file: otherwise its to fast to reproduce combining
+NEWMETHODS="$(readlink -f rootdir)/usr/lib/apt/methods"
+OLDMETHODS="$(readlink -f rootdir/usr/lib/apt/methods)"
+rm "$NEWMETHODS"
+mkdir "$NEWMETHODS"
+backupIFS="$IFS"
+IFS="$(printf "\n\b")"
+for METH in $(find "$OLDMETHODS" ! -type d); do
+       ln -s "$OLDMETHODS/$(basename "$METH")" "$NEWMETHODS"
+done
+IFS="$backupIFS"
+ln -s "${OLDMETHODS}/http" "${NEWMETHODS}/http-ng"
+
+changetowebserver
+sed -i -e 's# http:# http-ng:#' $(find rootdir/etc/apt/sources.list.d -name '*-deb-src.list')
+
+testsuccess apt update
+cp rootdir/tmp/testsuccess.output update.log
+# all requests are folded into the first Release file
+testsuccess grep ' http-ng://' update.log
+testfailure grep ' http://' update.log