From e05672e88678f520b2db59599e939345ad0b6e53 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 31 Jul 2014 09:53:13 +0200 Subject: Rework TransactionID stuff --- apt-pkg/acquire-item.cc | 140 +++++++++++++++-------- apt-pkg/acquire-item.h | 47 ++++---- apt-pkg/acquire.cc | 3 + apt-pkg/deb/debmetaindex.cc | 13 ++- test/integration/test-apt-update-rollback | 180 ++++++++++++++++++++++++++++++ 5 files changed, 305 insertions(+), 78 deletions(-) create mode 100755 test/integration/test-apt-update-rollback diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 01e1841d6..47cd9eab2 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -64,10 +64,12 @@ static void printHashSumComparision(std::string const &URI, HashStringList const /*}}}*/ // Acquire::Item::Item - Constructor /*{{{*/ -pkgAcquire::Item::Item(pkgAcquire *Owner, HashStringList const &ExpectedHashes) : - Owner(Owner), FileSize(0), PartialSize(0), Mode(0), ID(0), Complete(false), - Local(false), QueueCounter(0), TransactionID(0), ExpectedAdditionalItems(0), - ExpectedHashes(ExpectedHashes) +pkgAcquire::Item::Item(pkgAcquire *Owner, + HashStringList const &ExpectedHashes, + unsigned long TransactionID) + : Owner(Owner), FileSize(0), PartialSize(0), Mode(0), ID(0), Complete(false), + Local(false), QueueCounter(0), TransactionID(TransactionID), + ExpectedAdditionalItems(0), ExpectedHashes(ExpectedHashes) { Owner->Add(this); Status = StatIdle; @@ -239,10 +241,12 @@ void pkgAcquire::Item::ReportMirrorFailure(string FailCode) // --------------------------------------------------------------------- /* Get a sub-index file based on checksums from a 'master' file and possibly query additional files */ -pkgAcqSubIndex::pkgAcqSubIndex(pkgAcquire *Owner, string const &URI, - string const &URIDesc, string const &ShortDesc, - HashStringList const &ExpectedHashes) - : Item(Owner, ExpectedHashes) +pkgAcqSubIndex::pkgAcqSubIndex(pkgAcquire *Owner, + unsigned long TransactionID, + string const &URI, + string const &URIDesc, string const &ShortDesc, + HashStringList const &ExpectedHashes) + : Item(Owner, ExpectedHashes, TransactionID) { /* XXX: Beware: Currently this class does nothing (of value) anymore ! */ Debug = _config->FindB("Debug::pkgAcquire::SubIndex",false); @@ -354,11 +358,12 @@ bool pkgAcqSubIndex::ParseIndex(string const &IndexFile) /*{{{*/ * patches. If anything goes wrong in that process, it will fall back to * the original packages file */ -pkgAcqDiffIndex::pkgAcqDiffIndex(pkgAcqMetaIndex *MetaOwner, +pkgAcqDiffIndex::pkgAcqDiffIndex(pkgAcquire *Owner, + unsigned long TransactionID, IndexTarget const * const Target, HashStringList const &ExpectedHashes, indexRecords *MetaIndexParser) - : pkgAcqBaseIndex(MetaOwner, Target, ExpectedHashes, + : pkgAcqBaseIndex(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser) { @@ -457,7 +462,8 @@ bool pkgAcqDiffIndex::ParseDiffIndex(string IndexDiffFile) /*{{{*/ std::clog << "Package file is up-to-date" << std::endl; // 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(MetaOwner, Target, ExpectedHashes, MetaIndexParser, + new pkgAcqIndexDiffs(Owner, TransactionID, Target, + ExpectedHashes, MetaIndexParser, ServerSha1, available_patches); return true; } @@ -544,14 +550,17 @@ bool pkgAcqDiffIndex::ParseDiffIndex(string IndexDiffFile) /*{{{*/ if (pdiff_merge == false) { - new pkgAcqIndexDiffs(MetaOwner, Target, ExpectedHashes, MetaIndexParser, + new pkgAcqIndexDiffs(Owner, TransactionID, Target, ExpectedHashes, + MetaIndexParser, ServerSha1, available_patches); } else { std::vector *diffs = new std::vector(available_patches.size()); for(size_t i = 0; i < available_patches.size(); ++i) - (*diffs)[i] = new pkgAcqIndexMergeDiffs(MetaOwner, Target, + (*diffs)[i] = new pkgAcqIndexMergeDiffs(Owner, + TransactionID, + Target, ExpectedHashes, MetaIndexParser, available_patches[i], @@ -579,7 +588,7 @@ void pkgAcqDiffIndex::Failed(string Message,pkgAcquire::MethodConfig * /*Cnf*/)/ std::clog << "pkgAcqDiffIndex failed: " << Desc.URI << " with " << Message << std::endl << "Falling back to normal index file acquire" << std::endl; - new pkgAcqIndex(MetaOwner, Target, ExpectedHashes, MetaIndexParser); + new pkgAcqIndex(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser); Complete = false; Status = StatDone; @@ -621,13 +630,14 @@ void pkgAcqDiffIndex::Done(string Message,unsigned long long Size,HashStringList /* The package diff is added to the queue. one object is constructed * for each diff and the index */ -pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcqMetaIndex *MetaOwner, +pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcquire *Owner, + unsigned long TransactionID, struct IndexTarget const * const Target, HashStringList const &ExpectedHashes, indexRecords *MetaIndexParser, string ServerSha1, vector diffs) - : pkgAcqBaseIndex(MetaOwner, Target, ExpectedHashes, MetaIndexParser), + : pkgAcqBaseIndex(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser), available_patches(diffs), ServerSha1(ServerSha1) { @@ -659,7 +669,7 @@ void pkgAcqIndexDiffs::Failed(string Message,pkgAcquire::MethodConfig * /*Cnf*/) if(Debug) std::clog << "pkgAcqIndexDiffs failed: " << Desc.URI << " with " << Message << std::endl << "Falling back to normal index file acquire" << std::endl; - new pkgAcqIndex(MetaOwner, Target, ExpectedHashes, MetaIndexParser); + new pkgAcqIndex(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser); Finish(); } /*}}}*/ @@ -799,7 +809,7 @@ void pkgAcqIndexDiffs::Done(string Message,unsigned long long Size, HashStringLi // see if there is more to download if(available_patches.empty() == false) { - new pkgAcqIndexDiffs(MetaOwner, Target, + new pkgAcqIndexDiffs(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser, ServerSha1, available_patches); return Finish(); @@ -809,13 +819,14 @@ void pkgAcqIndexDiffs::Done(string Message,unsigned long long Size, HashStringLi } /*}}}*/ // AcqIndexMergeDiffs::AcqIndexMergeDiffs - Constructor /*{{{*/ -pkgAcqIndexMergeDiffs::pkgAcqIndexMergeDiffs(pkgAcqMetaIndex *MetaOwner, +pkgAcqIndexMergeDiffs::pkgAcqIndexMergeDiffs(pkgAcquire *Owner, + unsigned long TransactionID, struct IndexTarget const * const Target, HashStringList const &ExpectedHashes, indexRecords *MetaIndexParser, DiffInfo const &patch, std::vector const * const allPatches) - : pkgAcqBaseIndex(MetaOwner, Target, ExpectedHashes, MetaIndexParser), + : pkgAcqBaseIndex(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser), patch(patch), allPatches(allPatches), State(StateFetchDiff) { @@ -858,7 +869,7 @@ void pkgAcqIndexMergeDiffs::Failed(string Message,pkgAcquire::MethodConfig * /*C // first failure means we should fallback State = StateErrorDiff; std::clog << "Falling back to normal index file acquire" << std::endl; - new pkgAcqIndex(MetaOwner, Target, ExpectedHashes, MetaIndexParser); + new pkgAcqIndex(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser); } /*}}}*/ void pkgAcqIndexMergeDiffs::Done(string Message,unsigned long long Size,HashStringList const &Hashes, /*{{{*/ @@ -941,7 +952,7 @@ void pkgAcqIndexMergeDiffs::Done(string Message,unsigned long long Size,HashStri pkgAcqIndex::pkgAcqIndex(pkgAcquire *Owner, string URI,string URIDesc,string ShortDesc, HashStringList const &ExpectedHash, string comprExt) - : pkgAcqBaseIndex(Owner, NULL, ExpectedHash, NULL), RealURI(URI) + : pkgAcqBaseIndex(Owner, 0, NULL, ExpectedHash, NULL), RealURI(URI) { if(comprExt.empty() == true) { @@ -969,18 +980,21 @@ pkgAcqIndex::pkgAcqIndex(pkgAcquire *Owner, IndexTarget const *Target, } #endif /*}}}*/ -pkgAcqIndex::pkgAcqIndex(pkgAcqMetaIndex *MetaOwner, +pkgAcqIndex::pkgAcqIndex(pkgAcquire *Owner, + unsigned long TransactionID, IndexTarget const *Target, HashStringList const &ExpectedHash, indexRecords *MetaIndexParser) - : pkgAcqBaseIndex(MetaOwner->GetOwner(), Target, ExpectedHash, + : pkgAcqBaseIndex(Owner, TransactionID, Target, ExpectedHash, MetaIndexParser), RealURI(Target->URI) { // autoselect the compression method AutoSelectCompression(); Init(Target->URI, Target->Description, Target->ShortDesc); - TransactionID = (unsigned long)MetaOwner; + if(_config->FindB("Debug::Acquire::Transaction", false) == true) + std::clog << "New pkgIndex with TransactionID " + << TransactionID << std::endl; } /*}}}*/ void pkgAcqIndex::AutoSelectCompression() @@ -1265,9 +1279,12 @@ pkgAcqIndexTrans::pkgAcqIndexTrans(pkgAcquire *Owner, { } /*}}}*/ -pkgAcqIndexTrans::pkgAcqIndexTrans(pkgAcqMetaIndex *MetaOwner, IndexTarget const * const Target, - HashStringList const &ExpectedHashes, indexRecords *MetaIndexParser) - : pkgAcqIndex(MetaOwner, Target, ExpectedHashes, MetaIndexParser) +pkgAcqIndexTrans::pkgAcqIndexTrans(pkgAcquire *Owner, + unsigned long TransactionID, + IndexTarget const * const Target, + HashStringList const &ExpectedHashes, + indexRecords *MetaIndexParser) + : pkgAcqIndex(Owner, TransactionID, Target, ExpectedHashes, MetaIndexParser) { // load the filesize indexRecords::checkSum *Record = MetaIndexParser->Lookup(string(Target->MetaKey)); @@ -1302,6 +1319,7 @@ void pkgAcqIndexTrans::Failed(string Message,pkgAcquire::MethodConfig *Cnf) return; } + // FIXME: this is used often (e.g. in pkgAcqIndexTrans) so refactor if (Cnf->LocalOnly == true || StringToBool(LookupTag(Message,"Transient-Failure"),false) == false) { @@ -1315,12 +1333,13 @@ void pkgAcqIndexTrans::Failed(string Message,pkgAcquire::MethodConfig *Cnf) Item::Failed(Message,Cnf); } /*}}}*/ -pkgAcqMetaSig::pkgAcqMetaSig(pkgAcqMetaIndex *MetaOwner, /*{{{*/ +pkgAcqMetaSig::pkgAcqMetaSig(pkgAcquire *Owner, /*{{{*/ + unsigned long TransactionID, string URI,string URIDesc,string ShortDesc, string MetaIndexFile, const vector* IndexTargets, indexRecords* MetaIndexParser) : - Item(MetaOwner->GetOwner(), HashStringList()), RealURI(URI), + Item(Owner, HashStringList(), TransactionID), RealURI(URI), MetaIndexParser(MetaIndexParser), MetaIndexFile(MetaIndexFile), IndexTargets(IndexTargets), AuthPass(false), IMSHit(false) { @@ -1333,7 +1352,9 @@ pkgAcqMetaSig::pkgAcqMetaSig(pkgAcqMetaIndex *MetaOwner, /*{{{*/ unlink(DestFile.c_str()); // set the TransactionID - TransactionID = (unsigned long)MetaOwner; + if(_config->FindB("Debug::Acquire::Transaction", false) == true) + std::clog << "New pkgAcqMetaSig with TransactionID " + << TransactionID << std::endl; // Create the item Desc.Description = URIDesc; @@ -1432,35 +1453,56 @@ void pkgAcqMetaSig::Failed(string Message,pkgAcquire::MethodConfig *Cnf)/*{{{*/ DestFile += URItoFileName(RealURI); PartialFile = ""; + // FIXME: this is used often (e.g. in pkgAcqIndexTrans) so refactor + if (Cnf->LocalOnly == true || + StringToBool(LookupTag(Message,"Transient-Failure"),false) == false) + { + // Ignore this + Status = StatDone; + Complete = false; + Dequeue(); + return; + } Item::Failed(Message,Cnf); } /*}}}*/ pkgAcqMetaIndex::pkgAcqMetaIndex(pkgAcquire *Owner, /*{{{*/ + unsigned long TransactionID, string URI,string URIDesc,string ShortDesc, string MetaIndexSigURI,string MetaIndexSigURIDesc, string MetaIndexSigShortDesc, const vector* IndexTargets, indexRecords* MetaIndexParser) : - Item(Owner, HashStringList()), RealURI(URI), IndexTargets(IndexTargets), + Item(Owner, HashStringList(), TransactionID), RealURI(URI), IndexTargets(IndexTargets), MetaIndexParser(MetaIndexParser), AuthPass(false), IMSHit(false), MetaIndexSigURI(MetaIndexSigURI), MetaIndexSigURIDesc(MetaIndexSigURIDesc), MetaIndexSigShortDesc(MetaIndexSigShortDesc) { - DestFile = _config->FindDir("Dir::State::lists") + "partial/"; - DestFile += URItoFileName(URI); + if(TransactionID == 0) + this->TransactionID = (unsigned long)this; + + if(_config->FindB("Debug::Acquire::Transaction", false) == true) + std::clog << "New pkgAcqMetaIndex with TransactionID " + << TransactionID << std::endl; - TransactionID = (unsigned long)this; + Init(URIDesc, ShortDesc); +} + /*}}}*/ +// pkgAcqMetaIndex::Init - Delayed constructor /*{{{*/ +void pkgAcqMetaIndex::Init(std::string URIDesc, std::string ShortDesc) +{ + DestFile = _config->FindDir("Dir::State::lists") + "partial/"; + DestFile += URItoFileName(RealURI); // Create the item Desc.Description = URIDesc; Desc.Owner = this; Desc.ShortDesc = ShortDesc; - Desc.URI = URI; + Desc.URI = RealURI; // we expect more item ExpectedAdditionalItems = IndexTargets->size(); QueueURI(Desc); } - /*}}}*/ // pkgAcqMetaIndex::Custom600Headers - Insert custom request headers /*{{{*/ // --------------------------------------------------------------------- /* The only header we use is the last-modified header. */ @@ -1585,7 +1627,8 @@ void pkgAcqMetaIndex::RetrievalDone(string Message) /*{{{*/ // queue a signature if(SigFile != DestFile) - new pkgAcqMetaSig(this, MetaIndexSigURI, MetaIndexSigURIDesc, + new pkgAcqMetaSig(Owner, TransactionID, + MetaIndexSigURI, MetaIndexSigURIDesc, MetaIndexSigShortDesc, DestFile, IndexTargets, MetaIndexParser); @@ -1695,15 +1738,16 @@ void pkgAcqMetaIndex::QueueIndexes(bool verify) /*{{{*/ if ((*Target)->IsOptional() == true) { if ((*Target)->IsSubIndex() == true) - new pkgAcqSubIndex(Owner, (*Target)->URI, (*Target)->Description, + new pkgAcqSubIndex(Owner, TransactionID, + (*Target)->URI, (*Target)->Description, (*Target)->ShortDesc, ExpectedIndexHashes); else if (transInRelease == false || Record != NULL || compressedAvailable == true) { if (_config->FindB("Acquire::PDiffs",true) == true && transInRelease == true && MetaIndexParser->Exists((*Target)->MetaKey + ".diff/Index") == true) - new pkgAcqDiffIndex(this, *Target, ExpectedIndexHashes, MetaIndexParser); + new pkgAcqDiffIndex(Owner, TransactionID, *Target, ExpectedIndexHashes, MetaIndexParser); else - new pkgAcqIndexTrans(this, *Target, ExpectedIndexHashes, MetaIndexParser); + new pkgAcqIndexTrans(Owner, TransactionID, *Target, ExpectedIndexHashes, MetaIndexParser); } continue; } @@ -1714,9 +1758,9 @@ void pkgAcqMetaIndex::QueueIndexes(bool verify) /*{{{*/ instead, but passing the required info to it is to much hassle */ if(_config->FindB("Acquire::PDiffs",true) == true && (verify == false || MetaIndexParser->Exists((*Target)->MetaKey + ".diff/Index") == true)) - new pkgAcqDiffIndex(this, *Target, ExpectedIndexHashes, MetaIndexParser); + new pkgAcqDiffIndex(Owner, TransactionID, *Target, ExpectedIndexHashes, MetaIndexParser); else - new pkgAcqIndex(this, *Target, ExpectedIndexHashes, MetaIndexParser); + new pkgAcqIndex(Owner, TransactionID, *Target, ExpectedIndexHashes, MetaIndexParser); } } /*}}}*/ @@ -1872,8 +1916,9 @@ void pkgAcqMetaIndex::Finished() { if(_config->FindB("Debug::Acquire::Transaction", false) == true) std::clog << "Finished: " << DestFile <TransactionHasError((unsigned long)this) == false) - Owner->CommitTransaction((unsigned long)this); + if(Owner->TransactionHasError(TransactionID) == false && + TransactionID > 0) + Owner->CommitTransaction(TransactionID); } @@ -1883,7 +1928,7 @@ pkgAcqMetaClearSig::pkgAcqMetaClearSig(pkgAcquire *Owner, /*{{{*/ string const &MetaSigURI, string const &MetaSigURIDesc, string const &MetaSigShortDesc, const vector* IndexTargets, indexRecords* MetaIndexParser) : - pkgAcqMetaIndex(Owner, URI, URIDesc, ShortDesc, MetaSigURI, MetaSigURIDesc,MetaSigShortDesc, IndexTargets, MetaIndexParser), + pkgAcqMetaIndex(Owner, (unsigned long)this, URI, URIDesc, ShortDesc, MetaSigURI, MetaSigURIDesc,MetaSigShortDesc, IndexTargets, MetaIndexParser), MetaIndexURI(MetaIndexURI), MetaIndexURIDesc(MetaIndexURIDesc), MetaIndexShortDesc(MetaIndexShortDesc), MetaSigURI(MetaSigURI), MetaSigURIDesc(MetaSigURIDesc), MetaSigShortDesc(MetaSigShortDesc) { @@ -1892,7 +1937,6 @@ pkgAcqMetaClearSig::pkgAcqMetaClearSig(pkgAcquire *Owner, /*{{{*/ // index targets + (worst case:) Release/Release.gpg ExpectedAdditionalItems = IndexTargets->size() + 2; - // keep the old InRelease around in case of transistent network errors string const Final = _config->FindDir("Dir::State::lists") + URItoFileName(RealURI); if (RealFileExists(Final) == true) @@ -1947,7 +1991,7 @@ void pkgAcqMetaClearSig::Failed(string Message,pkgAcquire::MethodConfig *Cnf) /* if (FileExists(FinalFile)) unlink(FinalFile.c_str()); - new pkgAcqMetaIndex(Owner, + new pkgAcqMetaIndex(Owner, TransactionID, MetaIndexURI, MetaIndexURIDesc, MetaIndexShortDesc, MetaSigURI, MetaSigURIDesc, MetaSigShortDesc, IndexTargets, MetaIndexParser); diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index 11a596ad5..3f7cca083 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -297,7 +297,8 @@ class pkgAcquire::Item : public WeakPointable * \param ExpectedHashes of the file represented by this item */ Item(pkgAcquire *Owner, - HashStringList const &ExpectedHashes=HashStringList()); + HashStringList const &ExpectedHashes=HashStringList(), + unsigned long TransactionID=0); /** \brief Remove this item from its owner's queue by invoking * pkgAcquire::Remove. @@ -370,7 +371,9 @@ class pkgAcqSubIndex : public pkgAcquire::Item * * \param ExpectedHashes The list file's hashsums which are expected. */ - pkgAcqSubIndex(pkgAcquire *Owner, std::string const &URI,std::string const &URIDesc, + pkgAcqSubIndex(pkgAcquire *Owner, + unsigned long TransactionID, + std::string const &URI,std::string const &URIDesc, std::string const &ShortDesc, HashStringList const &ExpectedHashes); }; /*}}}*/ @@ -459,6 +462,9 @@ class pkgAcqMetaIndex : public pkgAcquire::Item /** \brief A brief description of the meta-index file */ std::string MetaIndexSigShortDesc; + + /** \brief delayed constructor */ + void Init(std::string URIDesc, std::string ShortDesc); public: @@ -472,6 +478,7 @@ class pkgAcqMetaIndex : public pkgAcquire::Item /** \brief Create a new pkgAcqMetaIndex. */ pkgAcqMetaIndex(pkgAcquire *Owner, + unsigned long TransactionID, std::string URI,std::string URIDesc, std::string ShortDesc, std::string MetaIndexSigURI, std::string MetaIndexSigURIDesc, std::string MetaIndexSigShortDesc, const std::vector* IndexTargets, @@ -525,22 +532,14 @@ class pkgAcqBaseIndex : public pkgAcquire::Item */ const struct IndexTarget * Target; indexRecords *MetaIndexParser; - pkgAcqMetaIndex *MetaOwner; - - pkgAcqBaseIndex(pkgAcqMetaIndex *MetaOwner, - struct IndexTarget const * const Target, - HashStringList const &ExpectedHashes, - indexRecords *MetaIndexParser) - : Item(MetaOwner->GetOwner(), ExpectedHashes), Target(Target), - MetaIndexParser(MetaIndexParser), MetaOwner(MetaOwner) {}; pkgAcqBaseIndex(pkgAcquire *Owner, + unsigned long TransactionID, struct IndexTarget const * const Target, HashStringList const &ExpectedHashes, indexRecords *MetaIndexParser) : Item(Owner, ExpectedHashes), Target(Target), - MetaIndexParser(MetaIndexParser), MetaOwner(0) {}; - + MetaIndexParser(MetaIndexParser) {}; }; /*}}}*/ /** \brief An item that is responsible for fetching an index file of {{{ @@ -606,7 +605,8 @@ class pkgAcqDiffIndex : public pkgAcqBaseIndex * * \param ExpectedHashes The list file's hashsums which are expected. */ - pkgAcqDiffIndex(pkgAcqMetaIndex *MetaIndexOwner, + pkgAcqDiffIndex(pkgAcquire *Owner, + unsigned long TransactionID, struct IndexTarget const * const Target, HashStringList const &ExpectedHashes, indexRecords *MetaIndexParser); @@ -694,7 +694,8 @@ class pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex * \param allPatches contains all related items so that each item can * check if it was the last one to complete the download step */ - pkgAcqIndexMergeDiffs(pkgAcqMetaIndex *MetaIndexOwner, + pkgAcqIndexMergeDiffs(pkgAcquire *Owner, + unsigned long TransactionID, struct IndexTarget const * const Target, HashStringList const &ExpectedHash, indexRecords *MetaIndexParser, @@ -822,7 +823,8 @@ class pkgAcqIndexDiffs : public pkgAcqBaseIndex * should be ordered so that each diff appears before any diff * that depends on it. */ - pkgAcqIndexDiffs(pkgAcqMetaIndex *MetaIndexOwner, + pkgAcqIndexDiffs(pkgAcquire *Owner, + unsigned long TransactionID, struct IndexTarget const * const Target, HashStringList const &ExpectedHash, indexRecords *MetaIndexParser, @@ -899,13 +901,7 @@ class pkgAcqIndex : public pkgAcqBaseIndex pkgAcqIndex(pkgAcquire *Owner,std::string URI,std::string URIDesc, std::string ShortDesc, HashStringList const &ExpectedHashes, std::string compressExt=""); -#if 0 - pkgAcqIndex(pkgAcquire *Owner, - IndexTarget const * const Target, - HashStringList const &ExpectedHash, - indexRecords *MetaIndexParser); -#endif - pkgAcqIndex(pkgAcqMetaIndex *MetaIndexOwner, + pkgAcqIndex(pkgAcquire *Owner, unsigned long TransactionID, IndexTarget const * const Target, HashStringList const &ExpectedHash, indexRecords *MetaIndexParser); @@ -942,7 +938,9 @@ class pkgAcqIndexTrans : public pkgAcqIndex pkgAcqIndexTrans(pkgAcquire *Owner, std::string URI,std::string URIDesc, std::string ShortDesc); - pkgAcqIndexTrans(pkgAcqMetaIndex *Owner, IndexTarget const * const Target, + pkgAcqIndexTrans(pkgAcquire *Owner, + unsigned long TransactionID, + IndexTarget const * const Target, HashStringList const &ExpectedHashes, indexRecords *MetaIndexParser); }; @@ -1047,7 +1045,8 @@ class pkgAcqMetaSig : public pkgAcquire::Item virtual std::string DescURI() const {return RealURI; }; /** \brief Create a new pkgAcqMetaSig. */ - pkgAcqMetaSig(pkgAcqMetaIndex *MetaOwner, + pkgAcqMetaSig(pkgAcquire *Owner, + unsigned long TransactionID, std::string URI,std::string URIDesc, std::string ShortDesc, std::string MetaIndexFile, const std::vector* IndexTargets, diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc index b14a54f0f..33afd8f1f 100644 --- a/apt-pkg/acquire.cc +++ b/apt-pkg/acquire.cc @@ -198,6 +198,7 @@ bool pkgAcquire::TransactionHasError(unsigned long TransactionID) if((*I)->Status == pkgAcquire::Item::StatError || (*I)->Status == pkgAcquire::Item::StatAuthError) return true; + return false; } // Acquire::CommitTransaction - Commit a transaction /*{{{*/ @@ -230,6 +231,8 @@ void pkgAcquire::CommitTransaction(unsigned long TransactionID) << (*I)->DestFile << std::endl; unlink((*I)->DestFile.c_str()); } + // mark that this transaction is finished + (*I)->TransactionID = 0; } } /*}}}*/ diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc index 98f99e888..b1dc060fe 100644 --- a/apt-pkg/deb/debmetaindex.cc +++ b/apt-pkg/deb/debmetaindex.cc @@ -265,11 +265,12 @@ bool debReleaseIndex::GetIndexes(pkgAcquire *Owner, bool const &GetAll) const // this is normally created in pkgAcqMetaSig, but if we run // in --print-uris mode, we add it here if (tryInRelease == false) - new pkgAcqMetaIndex(Owner, MetaIndexURI("Release"), - MetaIndexInfo("Release"), "Release", - MetaIndexURI("Release.gpg"), MetaIndexInfo("Release.gpg"), "Release.gpg", - ComputeIndexTargets(), - new indexRecords (Dist)); + new pkgAcqMetaIndex(Owner, 0, + MetaIndexURI("Release"), + MetaIndexInfo("Release"), "Release", + MetaIndexURI("Release.gpg"), MetaIndexInfo("Release.gpg"), "Release.gpg", + ComputeIndexTargets(), + new indexRecords (Dist)); } if (tryInRelease == true) @@ -280,7 +281,7 @@ bool debReleaseIndex::GetIndexes(pkgAcquire *Owner, bool const &GetAll) const ComputeIndexTargets(), new indexRecords (Dist)); else - new pkgAcqMetaIndex(Owner, + new pkgAcqMetaIndex(Owner, 0, MetaIndexURI("Release"), MetaIndexInfo("Release"), "Release", MetaIndexURI("Release.gpg"), MetaIndexInfo("Release.gpg"), "Release.gpg", ComputeIndexTargets(), diff --git a/test/integration/test-apt-update-rollback b/test/integration/test-apt-update-rollback new file mode 100755 index 000000000..cd28f1f1f --- /dev/null +++ b/test/integration/test-apt-update-rollback @@ -0,0 +1,180 @@ +#!/bin/sh +# +# test that apt-get update is transactional +# +set -e + +avoid_ims_hit() { + touch -d '+1hour' aptarchive/dists/unstable/main/binary-i386/Packages* + touch -d '+1hour' aptarchive/dists/unstable/main/source/Sources* + touch -d '+1hour' aptarchive/dists/unstable/*Release* + + touch -d '-1hour' rootdir/var/lib/apt/lists/* +} + +create_fresh_archive() +{ + rm -rf aptarchive/* + rm -f rootdir/var/lib/apt/lists/_* rootdir/var/lib/apt/lists/partial/* + + insertpackage 'unstable' 'old' 'all' '1.0' + + setupaptarchive +} + +add_new_package() { + insertpackage "unstable" "new" "all" "1.0" + insertsource "unstable" "new" "all" "1.0" + + setupaptarchive --no-update + + avoid_ims_hit +} + +break_repository_sources_index() { + printf "xxx" > $APTARCHIVE/dists/unstable/main/source/Sources + gzip -c $APTARCHIVE/dists/unstable/main/source/Sources > \ + $APTARCHIVE/dists/unstable/main/source/Sources.gz + avoid_ims_hit +} + +test_inrelease_to_new_inrelease() { + msgmsg "Test InRelease to new InRelease works fine" + create_fresh_archive + testequal "old/unstable 1.0 all" apt list -q + + add_new_package + testsuccess aptget update + + testequal "new/unstable 1.0 all +old/unstable 1.0 all" apt list -q +} + +test_inrelease_to_broken_hash_reverts_all() { + msgmsg "Test InRelease to broken InRelease reverts everything" + create_fresh_archive + add_new_package + # break the Sources file + break_repository_sources_index + + # test the error condition + testequal "W: Failed to fetch file:${APTARCHIVE}/dists/unstable/InRelease + +W: Failed to fetch copy:${APTARCHIVE}/dists/unstable/main/source/Sources Hash Sum mismatch + +W: Failed to fetch copy:${APTARCHIVE}/dists/unstable/main/binary-i386/Packages + +E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update -qq + # ensure that the Packages file is also rolled back + testequal "E: Unable to locate package new" aptget install new -s -qq +} + +test_inreleae_to_valid_release() { + msgmsg "Test InRelease to valid Release" + create_fresh_archive + add_new_package + # switch to a unsinged repo now + rm $APTARCHIVE/dists/unstable/InRelease + rm $APTARCHIVE/dists/unstable/Release.gpg + avoid_ims_hit + + # update works + testsuccess aptget update -o Debug::Acquire::Transaction=1 + + # test that we can install the new packages but do no longer have a sig + testsuccess aptget install old -s + testsuccess aptget install new -s + testfailure ls $ROOTDIR/var/lib/apt/lists/*_InRelease + testfailure ls $ROOTDIR/var/lib/apt/lists/*_Release.gpg + testsuccess ls $ROOTDIR/var/lib/apt/lists/*_Release +} + +test_inreleae_to_release_reverts_all() { + msgmsg "Test InRelease to broken Release reverts everything" + create_fresh_archive + + # switch to a unsinged repo now + add_new_package + rm $APTARCHIVE/dists/unstable/InRelease + rm $APTARCHIVE/dists/unstable/Release.gpg + # break it + break_repository_sources_index + + # ensure error + testequal "W: Failed to fetch file:$APTARCHIVE/dists/unstable/InRelease + +W: Failed to fetch file:$APTARCHIVE/dists/unstable/Release + +W: Failed to fetch file:$APTARCHIVE/dists/unstable/Release.gpg + +W: Failed to fetch copy:$APTARCHIVE/dists/unstable/main/source/Sources Hash Sum mismatch + +W: Failed to fetch copy:$APTARCHIVE/dists/unstable/main/binary-i386/Packages + +E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update -qq # -o Debug::acquire::transaction=1 + + # ensure that the Packages file is also rolled back + testsuccess aptget install old -s + testfailure aptget install new -s + testsuccess ls $ROOTDIR/var/lib/apt/lists/*_InRelease + testfailure ls $ROOTDIR/var/lib/apt/lists/*_Release +} + +test_unauthenticated_to_invalid_inrelease() { + msgmsg "Test UnAuthenticated to invalid InRelease reverts everything" + create_fresh_archive + rm $APTARCHIVE/dists/unstable/InRelease + rm $APTARCHIVE/dists/unstable/Release.gpg + avoid_ims_hit + + testsuccess aptget update -qq + testequal "WARNING: The following packages cannot be authenticated! + old +E: There are problems and -y was used without --force-yes" aptget install -qq -y old + + # go to authenticated but not correct + add_new_package + break_repository_sources_index + + testequal "W: Hashsum mismatch $ROOTDIR/var/lib/apt/lists/${APTARCHIVE_LISTS}_dists_unstable_main_source_Sources +W: Failed to fetch file:$APTARCHIVE/dists/unstable/InRelease + +W: Failed to fetch copy:$APTARCHIVE/dists/unstable/main/source/Sources Hash Sum mismatch + +E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update -qq + + testfailure ls rootdir/var/lib/apt/lists/*_InRelease + testequal "WARNING: The following packages cannot be authenticated! + old +E: There are problems and -y was used without --force-yes" aptget install -qq -y old +} + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture "i386" + +# setup the archive and ensure we have a single package that installs fine +setupaptarchive +APTARCHIVE=$(readlink -f ./aptarchive) +ROOTDIR=${TMPWORKINGDIRECTORY}/rootdir +APTARCHIVE_LISTS="$(echo $APTARCHIVE | tr "/" "_" )" + +# test the following cases: +# - InRelease -> broken InRelease revert to previous state +# - empty lists dir and broken remote leaves nothing on the system +# - InRelease -> hashsum mismatch for one file reverts all files to previous state +# - Release/Release.gpg -> hashsum mismatch +# - InRelease -> Release with hashsum mismatch revert entire state and kills Release +# - Release -> InRelease with broken Sig/Hash removes InRelease +# going from Release/Release.gpg -> InRelease and vice versa +# - unauthenticated -> invalid InRelease + +test_inrelease_to_new_inrelease +test_inrelease_to_broken_hash_reverts_all + +test_inreleae_to_valid_release +test_inreleae_to_release_reverts_all + +#test_unauthenticated_to_invalid_inrelease -- cgit v1.2.3