Handle by-hash URI construction more centrally

Individual items shouldn't concern themselves with these alternative
locations, we can deal with this more efficiently within the
infrastructure created for other alternative URIs now avoiding the need
to implement this in each item.

2 files changed, 74 insertions, 115 deletions

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index d0b92ec66..f52cd5b6c 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -398,26 +398,45 @@ bool pkgAcqTransactionItem::QueueURI(pkgAcquire::ItemDesc &Item)
       Status = StatDone;
       return false;
    }
+   // this ensures we rewrite only once and only the first step
+   auto const OldBaseURI = Target.Option(IndexTarget::BASE_URI);
+   if (OldBaseURI.empty() || APT::String::Startswith(Item.URI, OldBaseURI) == false)
+      return pkgAcquire::Item::QueueURI(Item);
+
+   std::vector<std::string> urls = {Item.URI};
    // If we got the InRelease file via a mirror, pick all indexes directly from this mirror, too
    if (TransactionManager->BaseURI.empty() == false && TransactionManager->UsedMirror.empty() == false &&
        URI::SiteOnly(Item.URI) != URI::SiteOnly(TransactionManager->BaseURI))
    {
-      // this ensures we rewrite only once and only the first step
-      auto const OldBaseURI = Target.Option(IndexTarget::BASE_URI);
-      if (OldBaseURI.empty() == false && APT::String::Startswith(Item.URI, OldBaseURI))
+      auto ExtraPath = Item.URI.substr(OldBaseURI.length());
+      auto newURI = flCombine(TransactionManager->BaseURI, std::move(ExtraPath));
+      if (IsGoodAlternativeURI(newURI))
       {
-	 auto const ExtraPath = Item.URI.substr(OldBaseURI.length());
-	 auto newURI = flCombine(TransactionManager->BaseURI, ExtraPath);
-	 if (IsGoodAlternativeURI(newURI))
-	 {
-	    PushAlternativeURI(std::string(Item.URI), {}, false);
-	    Item.URI = std::move(newURI);
-	    UsedMirror = TransactionManager->UsedMirror;
-	    if (Item.Description.find(" ") != string::npos)
-	       Item.Description.replace(0, Item.Description.find(" "), UsedMirror);
-	 }
+	 urls.push_back(std::move(newURI));
+	 UsedMirror = TransactionManager->UsedMirror;
+	 if (Item.Description.find(" ") != string::npos)
+	    Item.Description.replace(0, Item.Description.find(" "), UsedMirror);
       }
    }
+   // add URI and by-hash based on it
+   auto const Expected = GetExpectedHashes();
+   auto const TargetHash = Expected.find(nullptr);
+   auto const useByHash = AcquireByHash();
+   for (auto &&U : urls)
+   {
+      PushAlternativeURI(std::string(U), {}, false);
+      if (useByHash == false || unlikely(TargetHash == nullptr))
+	 continue;
+      auto const trailing_slash = U.find_last_of("/");
+      if (unlikely(trailing_slash == std::string::npos))
+	 continue;
+      auto byhashSuffix = "/by-hash/" + TargetHash->HashType() + "/" + TargetHash->HashValue();
+      U.replace(trailing_slash, U.length() - trailing_slash, std::move(byhashSuffix));
+      PushAlternativeURI(std::move(U), {}, false);
+   }
+   // the last URI added is the first one tried
+   if (unlikely(PopAlternativeURI(Item.URI) == false))
+      return false;
    return pkgAcquire::Item::QueueURI(Item);
 }
 /* The transition manager InRelease itself (or its older sisters-in-law
@@ -624,6 +643,26 @@ bool pkgAcqDiffIndex::TransactionState(TransactionStates const state)
    return true;
 }
 									/*}}}*/
+// pkgAcqTransactionItem::AcquireByHash and specialisations for child classes	/*{{{*/
+bool pkgAcqTransactionItem::AcquireByHash() const
+{
+   if (TransactionManager->MetaIndexParser == nullptr)
+      return false;
+   auto const useByHashConf = Target.Option(IndexTarget::BY_HASH);
+   if (useByHashConf == "force")
+      return true;
+   return StringToBool(useByHashConf) == true && TransactionManager->MetaIndexParser->GetSupportsAcquireByHash();
+}
+// pdiff patches have a unique name already, no need for by-hash
+bool pkgAcqIndexMergeDiffs::AcquireByHash() const
+{
+   return false;
+}
+bool pkgAcqIndexDiffs::AcquireByHash() const
+{
+   return false;
+}
+									/*}}}*/
 
 class APT_HIDDEN NoActionItem : public pkgAcquire::Item			/*{{{*/
 /* The sole purpose of this class is having an item which does nothing to
@@ -1453,7 +1492,6 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)			/*{{{*/
 
    std::set<std::string> targetsSeen;
    bool const hasReleaseFile = TransactionManager->MetaIndexParser != NULL;
-   bool const metaBaseSupportsByHash = hasReleaseFile && TransactionManager->MetaIndexParser->GetSupportsAcquireByHash();
    bool hasHashes = true;
    auto IndexTargets = TransactionManager->MetaIndexParser->GetIndexTargets();
    if (hasReleaseFile && verify == false)
@@ -1589,15 +1627,6 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)			/*{{{*/
 	 if (types.empty() == false)
 	 {
 	    std::ostringstream os;
-	    // add the special compressiontype byhash first if supported
-	    std::string const useByHashConf = Target.Option(IndexTarget::BY_HASH);
-	    bool useByHash = false;
-	    if(useByHashConf == "force")
-	       useByHash = true;
-	    else
-	       useByHash = StringToBool(useByHashConf) == true && metaBaseSupportsByHash;
-	    if (useByHash == true)
-	       os << "by-hash ";
 	    std::copy(types.begin(), types.end()-1, std::ostream_iterator<std::string>(os, " "));
 	    os << *types.rbegin();
 	    Target.Options["COMPRESSIONTYPES"] = os.str();
@@ -2243,8 +2272,6 @@ pkgAcqDiffIndex::pkgAcqDiffIndex(pkgAcquire * const Owner,
 	 CompressionExtensions = os.str();
       }
    }
-   if (Target.Option(IndexTarget::COMPRESSIONTYPES).find("by-hash") != std::string::npos)
-      CompressionExtensions = "by-hash " + CompressionExtensions;
    Init(GetDiffIndexURI(Target), GetDiffIndexFileName(Target.Description), Target.ShortDesc);
 
    if(Debug)
@@ -2255,7 +2282,7 @@ void pkgAcqDiffIndex::QueueOnIMSHit() const				/*{{{*/
 {
    // list cleanup needs to know that this file as well as the already
    // present index is ours, so we create an empty diff to save it for us
-   new pkgAcqIndexDiffs(Owner, TransactionManager, Target, Target.URI);
+   new pkgAcqIndexDiffs(Owner, TransactionManager, Target);
 }
 									/*}}}*/
 static bool RemoveFileForBootstrapLinking(std::string &ErrorText, std::string const &For, std::string const &Boot)/*{{{*/
@@ -2655,31 +2682,14 @@ void pkgAcqDiffIndex::Done(string const &Message,HashStringList const &Hashes,	/
    }
    else
    {
-      // we have something, queue the diffs
-      string::size_type const last_space = Description.rfind(" ");
-      if(last_space != string::npos)
-	 Description.erase(last_space, Description.size()-last_space);
-
-      std::string indexURI = Desc.URI;
-      auto const byhashidx = indexURI.find("/by-hash/");
-      if (byhashidx != std::string::npos)
-	 indexURI = indexURI.substr(0, byhashidx - strlen(".diff"));
-      else
-      {
-	 auto end = indexURI.length() - strlen(".diff/Index");
-	 if (CurrentCompressionExtension != "uncompressed")
-	    end -= (1 + CurrentCompressionExtension.length());
-	 indexURI = indexURI.substr(0, end);
-      }
-
       if (pdiff_merge == false)
-	 new pkgAcqIndexDiffs(Owner, TransactionManager, Target, indexURI, available_patches);
+	 new pkgAcqIndexDiffs(Owner, TransactionManager, Target, available_patches);
       else
       {
 	 diffs = new std::vector<pkgAcqIndexMergeDiffs*>(available_patches.size());
 	 for(size_t i = 0; i < available_patches.size(); ++i)
 	    (*diffs)[i] = new pkgAcqIndexMergeDiffs(Owner, TransactionManager,
-						    Target, indexURI,
+						    Target,
 						    available_patches[i],
 						    diffs);
       }
@@ -2708,9 +2718,8 @@ pkgAcqDiffIndex::~pkgAcqDiffIndex()
 pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcquire *const Owner,
 				   pkgAcqMetaClearSig *const TransactionManager,
 				   IndexTarget const &Target,
-				   std::string const &indexURI,
 				   vector<DiffInfo> const &diffs)
-    : pkgAcqBaseIndex(Owner, TransactionManager, Target), indexURI(indexURI),
+    : pkgAcqBaseIndex(Owner, TransactionManager, Target),
       available_patches(diffs)
 {
    DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
@@ -2718,7 +2727,6 @@ pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcquire *const Owner,
    Debug = _config->FindB("Debug::pkgAcquire::Diffs",false);
 
    Desc.Owner = this;
-   Description = Target.Description;
    Desc.ShortDesc = Target.ShortDesc;
 
    if(available_patches.empty() == true)
@@ -2835,8 +2843,8 @@ bool pkgAcqIndexDiffs::QueueNextDiff()					/*{{{*/
    }
 
    // queue the right diff
-   Desc.URI = indexURI + ".diff/" + available_patches[0].file + ".gz";
-   Desc.Description = Description + " " + available_patches[0].file + string(".pdiff");
+   Desc.URI = Target.URI + ".diff/" + available_patches[0].file + ".gz";
+   Desc.Description = Target.Description + " " + available_patches[0].file + string(".pdiff");
    DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI + ".diff/" + available_patches[0].file), Target);
 
    if(Debug)
@@ -2888,7 +2896,7 @@ void pkgAcqIndexDiffs::Done(string const &Message, HashStringList const &Hashes,
 	 // see if there is more to download
 	 if(available_patches.empty() == false)
 	 {
-	    new pkgAcqIndexDiffs(Owner, TransactionManager, Target, indexURI, available_patches);
+	    new pkgAcqIndexDiffs(Owner, TransactionManager, Target, available_patches);
 	    Finish();
 	 } else {
 	    DestFile = PatchedFile;
@@ -2917,19 +2925,17 @@ pkgAcqIndexDiffs::~pkgAcqIndexDiffs() {}
 pkgAcqIndexMergeDiffs::pkgAcqIndexMergeDiffs(pkgAcquire *const Owner,
 					     pkgAcqMetaClearSig *const TransactionManager,
 					     IndexTarget const &Target,
-					     std::string const &indexURI,
 					     DiffInfo const &patch,
 					     std::vector<pkgAcqIndexMergeDiffs *> const *const allPatches)
-    : pkgAcqBaseIndex(Owner, TransactionManager, Target), indexURI(indexURI),
+    : pkgAcqBaseIndex(Owner, TransactionManager, Target),
       patch(patch), allPatches(allPatches), State(StateFetchDiff)
 {
    Debug = _config->FindB("Debug::pkgAcquire::Diffs",false);
-   Description = Target.Description;
 
    Desc.Owner = this;
    Desc.ShortDesc = Target.ShortDesc;
-   Desc.URI = indexURI + ".diff/" + patch.file + ".gz";
-   Desc.Description = Description + " " + patch.file + ".pdiff";
+   Desc.URI = Target.URI + ".diff/" + patch.file + ".gz";
+   Desc.Description = Target.Description + " " + patch.file + ".pdiff";
    DestFile = GetPartialFileNameFromURI(Target.URI + ".diff/" + patch.file + ".gz");
 
    if(Debug)
@@ -3085,60 +3091,28 @@ pkgAcqIndex::pkgAcqIndex(pkgAcquire * const Owner,
 }
 									/*}}}*/
 // AcqIndex::Init - deferred Constructor				/*{{{*/
-static void NextCompressionExtension(std::string &CurrentCompressionExtension, std::string &CompressionExtensions, bool const preview)
+void pkgAcqIndex::Init(string const &URI, string const &URIDesc,
+                       string const &ShortDesc)
 {
+   Stage = STAGE_DOWNLOAD;
+
+   DestFile = GetPartialFileNameFromURI(URI);
    size_t const nextExt = CompressionExtensions.find(' ');
    if (nextExt == std::string::npos)
    {
       CurrentCompressionExtension = CompressionExtensions;
-      if (preview == false)
-	 CompressionExtensions.clear();
+      CompressionExtensions.clear();
    }
    else
    {
       CurrentCompressionExtension = CompressionExtensions.substr(0, nextExt);
-      if (preview == false)
-	 CompressionExtensions = CompressionExtensions.substr(nextExt+1);
+      CompressionExtensions = CompressionExtensions.substr(nextExt+1);
    }
-}
-void pkgAcqIndex::Init(string const &URI, string const &URIDesc,
-                       string const &ShortDesc)
-{
-   Stage = STAGE_DOWNLOAD;
-
-   DestFile = GetPartialFileNameFromURI(URI);
-   NextCompressionExtension(CurrentCompressionExtension, CompressionExtensions, false);
 
    if (CurrentCompressionExtension == "uncompressed")
    {
       Desc.URI = URI;
    }
-   else if (CurrentCompressionExtension == "by-hash")
-   {
-      NextCompressionExtension(CurrentCompressionExtension, CompressionExtensions, true);
-      if(unlikely(CurrentCompressionExtension.empty()))
-	 return;
-      if (CurrentCompressionExtension != "uncompressed")
-      {
-	 Desc.URI = URI + '.' + CurrentCompressionExtension;
-	 DestFile = DestFile + '.' + CurrentCompressionExtension;
-      }
-      else
-	 Desc.URI = URI;
-
-      HashStringList const Hashes = GetExpectedHashes();
-      HashString const * const TargetHash = Hashes.find(NULL);
-      if (unlikely(TargetHash == nullptr))
-	 return;
-      std::string const ByHash = "/by-hash/" + TargetHash->HashType() + "/" + TargetHash->HashValue();
-      size_t const trailing_slash = Desc.URI.find_last_of("/");
-      if (unlikely(trailing_slash == std::string::npos))
-	 return;
-      Desc.URI = Desc.URI.replace(
-	    trailing_slash,
-	    Desc.URI.substr(trailing_slash+1).size()+1,
-	    ByHash);
-   }
    else if (unlikely(CurrentCompressionExtension.empty()))
       return;
    else
diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h
index 814596850..3a5a518c2 100644
--- a/apt-pkg/acquire-item.h
+++ b/apt-pkg/acquire-item.h
@@ -407,7 +407,7 @@ class APT_HIDDEN pkgAcqTransactionItem: public pkgAcquire::Item		/*{{{*/
    virtual HashStringList GetExpectedHashes() const APT_OVERRIDE;
    virtual std::string GetMetaKey() const;
    virtual bool HashesRequired() const APT_OVERRIDE;
-
+   virtual bool AcquireByHash() const;
 
    pkgAcqTransactionItem(pkgAcquire * const Owner, pkgAcqMetaClearSig * const TransactionManager, IndexTarget const &Target) APT_NONNULL(2, 3);
    virtual ~pkgAcqTransactionItem();
@@ -730,11 +730,6 @@ class APT_HIDDEN pkgAcqDiffIndex : public pkgAcqIndex
    /** \brief If \b true, debugging information will be written to std::clog. */
    bool Debug;
 
-   /** \brief A description of the Packages file (stored in
-    *  pkgAcquire::ItemDesc::Description).
-    */
-   std::string Description;
-
    /** \brief Get the full pathname of the final file for the current URI */
    virtual std::string GetFinalFilename() const APT_OVERRIDE;
 
@@ -792,8 +787,6 @@ class APT_HIDDEN pkgAcqDiffIndex : public pkgAcqIndex
  */
 class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
 {
-   std::string const indexURI;
-
    protected:
 
    /** \brief If \b true, debugging output will be written to
@@ -801,9 +794,6 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
     */
    bool Debug;
 
-   /** \brief description of the file being downloaded. */
-   std::string Description;
-
    /** \brief information about the current patch */
    struct DiffInfo const patch;
 
@@ -839,6 +829,7 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
    virtual std::string DescURI() const APT_OVERRIDE {return Target.URI + "Index";};
    virtual HashStringList GetExpectedHashes() const APT_OVERRIDE;
    virtual bool HashesRequired() const APT_OVERRIDE;
+   virtual bool AcquireByHash() const APT_OVERRIDE;
 
    /** \brief Create an index merge-diff item.
     *
@@ -853,9 +844,8 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
     *  check if it was the last one to complete the download step
     */
    pkgAcqIndexMergeDiffs(pkgAcquire *const Owner, pkgAcqMetaClearSig *const TransactionManager,
-			 IndexTarget const &Target,
-			 std::string const &indexURI, DiffInfo const &patch,
-			 std::vector<pkgAcqIndexMergeDiffs *> const *const allPatches) APT_NONNULL(2, 3, 7);
+			 IndexTarget const &Target, DiffInfo const &patch,
+			 std::vector<pkgAcqIndexMergeDiffs *> const *const allPatches) APT_NONNULL(2, 3, 6);
    virtual ~pkgAcqIndexMergeDiffs();
 };
 									/*}}}*/
@@ -872,8 +862,6 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
  */
 class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
 {
-   std::string const indexURI;
-
    private:
 
    /** \brief Queue up the next diff download.
@@ -907,9 +895,6 @@ class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
     */
    bool Debug;
 
-   /** A description of the file being downloaded. */
-   std::string Description;
-
    /** The patches that remain to be downloaded, including the patch
     *  being downloaded right now.  This list should be ordered so
     *  that each diff appears before any diff that depends on it.
@@ -945,6 +930,7 @@ class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
    virtual std::string DescURI() const APT_OVERRIDE {return Target.URI + "IndexDiffs";};
    virtual HashStringList GetExpectedHashes() const APT_OVERRIDE;
    virtual bool HashesRequired() const APT_OVERRIDE;
+   virtual bool AcquireByHash() const APT_OVERRIDE;
 
    /** \brief Create an index diff item.
     *
@@ -961,7 +947,6 @@ class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
     */
    pkgAcqIndexDiffs(pkgAcquire *const Owner, pkgAcqMetaClearSig *const TransactionManager,
 		    IndexTarget const &Target,
-		    std::string const &indexURI,
 		    std::vector<DiffInfo> const &diffs = std::vector<DiffInfo>()) APT_NONNULL(2, 3);
    virtual ~pkgAcqIndexDiffs();
 };