From dff555d40bb9776b5b809e06527e46b15e78736c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 26 Oct 2017 01:09:48 +0200 Subject: implement Acquire::Retries support for all items Moving the Retry-implementation from individual items to the worker implementation not only gives every file retry capability instead of just a selected few but also avoids needing to implement it in each item (incorrectly). --- apt-pkg/acquire-item.cc | 66 +++++++++++++++++++---------------------------- apt-pkg/acquire-item.h | 5 ++++ apt-pkg/acquire-worker.cc | 34 +++++++++++++++++------- 3 files changed, 56 insertions(+), 49 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index b3eb75d16..8c11337ac 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -684,6 +684,11 @@ class pkgAcquire::Item::Private { public: std::vector PastRedirections; + unsigned int Retries; + + Private() : Retries(_config->FindI("Acquire::Retries", 0)) + { + } }; APT_IGNORE_DEPRECATED_PUSH pkgAcquire::Item::Item(pkgAcquire * const owner) : @@ -707,6 +712,11 @@ std::string pkgAcquire::Item::Custom600Headers() const /*{{{*/ return std::string(); } /*}}}*/ +unsigned int &pkgAcquire::Item::ModifyRetries() /*{{{*/ +{ + return d->Retries; +} + /*}}}*/ std::string pkgAcquire::Item::ShortDesc() const /*{{{*/ { return DescURI(); @@ -778,6 +788,13 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con Dequeue(); } + FailMessage(Message); + + if (QueueCounter > 1) + Status = StatIdle; +} +void pkgAcquire::Item::FailMessage(string const &Message) +{ string const FailReason = LookupTag(Message, "FailReason"); enum { MAXIMUM_SIZE_EXCEEDED, HASHSUM_MISMATCH, WEAK_HASHSUMS, REDIRECTION_LOOP, OTHER } failreason = OTHER; if ( FailReason == "MaximumSizeExceeded") @@ -851,9 +868,6 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con ReportMirrorFailureToCentral(*this, FailReason, ErrorText); else ReportMirrorFailureToCentral(*this, ErrorText, ErrorText); - - if (QueueCounter > 1) - Status = StatIdle; } /*}}}*/ // Acquire::Item::Start - Item has begun to download /*{{{*/ @@ -899,7 +913,7 @@ void pkgAcquire::Item::Done(string const &/*Message*/, HashStringList const &Has } } Status = StatDone; - ErrorText = string(); + ErrorText.clear(); Owner->Dequeue(this); } /*}}}*/ @@ -3217,8 +3231,6 @@ pkgAcqArchive::pkgAcqArchive(pkgAcquire * const Owner,pkgSourceList * const Sour StoreFilename(StoreFilename), Vf(Version.FileList()), Trusted(false) { - Retries = _config->FindI("Acquire::Retries",0); - if (Version.Arch() == 0) { _error->Error(_("I wasn't able to locate a file for the %s package. " @@ -3453,17 +3465,6 @@ void pkgAcqArchive::Failed(string const &Message,pkgAcquire::MethodConfig const Status = StatIdle; if (QueueNext() == false) { - // This is the retry counter - if (Retries != 0 && - Cnf->LocalOnly == false && - StringToBool(LookupTag(Message,"Transient-Failure"),false) == true) - { - Retries--; - Vf = Version.FileList(); - if (QueueNext() == true) - return; - } - StoreFilename = string(); Status = StatError; } @@ -3754,14 +3755,12 @@ pkgAcqChangelog::~pkgAcqChangelog() /*{{{*/ /*}}}*/ // AcqFile::pkgAcqFile - Constructor /*{{{*/ -pkgAcqFile::pkgAcqFile(pkgAcquire * const Owner,string const &URI, HashStringList const &Hashes, - unsigned long long const Size,string const &Dsc,string const &ShortDesc, +APT_IGNORE_DEPRECATED_PUSH +pkgAcqFile::pkgAcqFile(pkgAcquire *const Owner, string const &URI, HashStringList const &Hashes, + unsigned long long const Size, string const &Dsc, string const &ShortDesc, const string &DestDir, const string &DestFilename, - bool const IsIndexFile) : - Item(Owner), d(NULL), IsIndexFile(IsIndexFile), ExpectedHashes(Hashes) + bool const IsIndexFile) : Item(Owner), d(NULL), Retries(0), IsIndexFile(IsIndexFile), ExpectedHashes(Hashes) { - Retries = _config->FindI("Acquire::Retries",0); - if(!DestFilename.empty()) DestFile = DestFilename; else if(!DestDir.empty()) @@ -3791,6 +3790,7 @@ pkgAcqFile::pkgAcqFile(pkgAcquire * const Owner,string const &URI, HashStringLis QueueURI(Desc); } +APT_IGNORE_DEPRECATED_POP /*}}}*/ // AcqFile::Done - Item downloaded OK /*{{{*/ void pkgAcqFile::Done(string const &Message,HashStringList const &CalcHashes, @@ -3840,24 +3840,10 @@ void pkgAcqFile::Done(string const &Message,HashStringList const &CalcHashes, } } /*}}}*/ -// AcqFile::Failed - Failure handler /*{{{*/ -// --------------------------------------------------------------------- -/* Here we try other sources */ -void pkgAcqFile::Failed(string const &Message, pkgAcquire::MethodConfig const * const Cnf) +void pkgAcqFile::Failed(string const &Message, pkgAcquire::MethodConfig const *const Cnf) /*{{{*/ { - Item::Failed(Message,Cnf); - - // This is the retry counter - if (Retries != 0 && - Cnf->LocalOnly == false && - StringToBool(LookupTag(Message,"Transient-Failure"),false) == true) - { - --Retries; - QueueURI(Desc); - Status = StatIdle; - return; - } - + // FIXME: Remove this pointless overload on next ABI break + Item::Failed(Message, Cnf); } /*}}}*/ string pkgAcqFile::Custom600Headers() const /*{{{*/ diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index a5c7d848a..7705f3ccb 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -174,6 +174,7 @@ class pkgAcquire::Item : public WeakPointable /*{{{*/ * \sa pkgAcqMethod */ virtual void Failed(std::string const &Message,pkgAcquire::MethodConfig const * const Cnf); + APT_HIDDEN void FailMessage(std::string const &Message); /** \brief Invoked by the acquire worker to check if the successfully * fetched object is also the objected we wanted to have. @@ -238,6 +239,8 @@ class pkgAcquire::Item : public WeakPointable /*{{{*/ * no trailing newline. */ virtual std::string Custom600Headers() const; + // Retries should really be a member of the Item, but can't be for ABI reasons + APT_HIDDEN unsigned int &ModifyRetries(); /** \brief A "descriptive" URI-like string. * @@ -994,6 +997,7 @@ class pkgAcqArchive : public pkgAcquire::Item * * Set from Acquire::Retries. */ + APT_DEPRECATED_MSG("Unused member. See pkgAcqItem::Retries.") unsigned int Retries; /** \brief \b true if this version file is being downloaded from a @@ -1171,6 +1175,7 @@ class pkgAcqFile : public pkgAcquire::Item /** \brief How many times to retry the download, set from * Acquire::Retries. */ + APT_DEPRECATED_MSG("Unused member. See pkgAcqItem::Retries.") unsigned int Retries; /** \brief Should this file be considered a index file */ diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 39158aed7..a763d9242 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -506,6 +506,9 @@ bool pkgAcquire::Worker::RunMessages() Itm = nullptr; bool errTransient = false, errAuthErr = false; + if (StringToBool(LookupTag(Message, "Transient-Failure"), false) == true) + errTransient = true; + else { std::string const failReason = LookupTag(Message, "FailReason"); { @@ -522,15 +525,28 @@ bool pkgAcquire::Worker::RunMessages() for (auto const Owner: ItmOwners) { - if (errAuthErr && Owner->GetExpectedHashes().empty() == false) - Owner->Status = pkgAcquire::Item::StatAuthError; - else if (errTransient) - Owner->Status = pkgAcquire::Item::StatTransientNetworkError; - auto SavedDesc = Owner->GetItemDesc(); - if (isDoomedItem(Owner) == false) - Owner->Failed(Message,Config); - if (Log != nullptr) - Log->Fail(SavedDesc); + if (errTransient == true && Config->LocalOnly == false && Owner->ModifyRetries() != 0) + { + --Owner->ModifyRetries(); + Owner->FailMessage(Message); + auto SavedDesc = Owner->GetItemDesc(); + if (Log != nullptr) + Log->Fail(SavedDesc); + if (isDoomedItem(Owner) == false) + OwnerQ->Owner->Enqueue(SavedDesc); + } + else + { + if (errAuthErr && Owner->GetExpectedHashes().empty() == false) + Owner->Status = pkgAcquire::Item::StatAuthError; + else if (errTransient) + Owner->Status = pkgAcquire::Item::StatTransientNetworkError; + auto SavedDesc = Owner->GetItemDesc(); + if (isDoomedItem(Owner) == false) + Owner->Failed(Message, Config); + if (Log != nullptr) + Log->Fail(SavedDesc); + } } ItemDone(); -- cgit v1.2.3