diff options
author | Julian Andres Klode <jak@debian.org> | 2024-01-08 09:15:34 +0000 |
---|---|---|
committer | Julian Andres Klode <jak@debian.org> | 2024-01-08 09:15:34 +0000 |
commit | 6e43eef9ca8250eb561f2c9af2f4890d674f3911 (patch) | |
tree | 94e31c8f341cc886f55f87b7a6e8048187994934 | |
parent | 7a9a7c1025a5a9dcb27b0e8b5a2d40327fd3b911 (diff) | |
parent | afcdbcf895284efd76903b2b3ba5cc849059ce50 (diff) |
Merge branch 'fix/dontstorediffindex' into 'main'
Do not store .diff_Index files in update
See merge request apt-team/apt!316
-rw-r--r-- | apt-pkg/acquire-item.cc | 46 | ||||
-rw-r--r-- | apt-pkg/acquire.cc | 24 | ||||
-rw-r--r-- | apt-pkg/acquire.h | 17 | ||||
-rw-r--r-- | apt-private/private-cmndline.cc | 5 | ||||
-rw-r--r-- | apt-private/private-download.cc | 40 | ||||
-rw-r--r-- | apt-private/private-download.h | 1 | ||||
-rw-r--r-- | cmdline/apt-get.cc | 2 | ||||
-rwxr-xr-x | test/integration/test-apt-get-clean | 40 | ||||
-rwxr-xr-x | test/integration/test-pdiff-usage | 23 |
9 files changed, 106 insertions, 92 deletions
diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 7df6483ba..400838dcb 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -276,6 +276,15 @@ static HashStringList GetExpectedHashesFromFor(metaIndex * const Parser, std::st return R->Hashes; } /*}}}*/ +static void RemoveOldLeftoverDiffIndex(IndexTarget const &Target) /*{{{*/ +{ + std::string const FinalFile = GetFinalFileNameFromURI(GetDiffIndexURI(Target)); + RemoveFile("TransactionCommit", FinalFile); + for (auto const &ext: APT::Configuration::getCompressorExtensions()) + if (not ext.empty() && ext != ".") + RemoveFile("TransactionCommit", FinalFile + ext); +} + /*}}}*/ class pkgAcquire::Item::Private /*{{{*/ { @@ -414,12 +423,16 @@ bool pkgAcqTransactionItem::QueueURI(pkgAcquire::ItemDesc &Item) std::clog << "Skip " << Target.URI << " as transaction was already dealt with!" << std::endl; return false; } - std::string const FinalFile = GetFinalFilename(); - if (TransactionManager->IMSHit == true && FileExists(FinalFile) == true) + if (TransactionManager->IMSHit) { - PartialFile = DestFile = FinalFile; - Status = StatDone; - return false; + std::string const FinalFile = GetFinalFilename(); + if (FinalFile.empty() || FileExists(FinalFile)) + { + if (not FinalFile.empty()) + PartialFile = DestFile = FinalFile; + Status = StatDone; + return false; + } } // this ensures we rewrite only once and only the first step auto const OldBaseURI = Target.Option(IndexTarget::BASE_URI); @@ -508,11 +521,7 @@ std::string pkgAcquire::Item::GetFinalFilename() const } std::string pkgAcqDiffIndex::GetFinalFilename() const { - std::string const FinalFile = GetFinalFileNameFromURI(GetDiffIndexURI(Target)); - // we don't want recompress, so lets keep whatever we got - if (CurrentCompressionExtension == "uncompressed") - return FinalFile; - return FinalFile + "." + CurrentCompressionExtension; + return {}; } std::string pkgAcqIndex::GetFinalFilename() const { @@ -675,12 +684,11 @@ bool pkgAcqDiffIndex::TransactionState(TransactionStates const state) switch (state) { - case TransactionStarted: _error->Fatal("Item %s changed to invalid transaction start state!", Target.URI.c_str()); break; + case TransactionStarted: _error->Fatal("Item %s changed to invalid transaction start state!", GetDiffIndexURI(Target).c_str()); break; case TransactionCommit: + RemoveOldLeftoverDiffIndex(Target); break; case TransactionAbort: - std::string const Partial = GetPartialFileNameFromURI(Target.URI); - RemoveFile("TransactionAbort", Partial); break; } @@ -761,6 +769,7 @@ class APT_HIDDEN CleanupItem : public pkgAcqTransactionItem /*{{{*/ std::clog << "rm " << DestFile << " # " << DescURI() << std::endl; if (RemoveFile("TransItem::TransactionCommit", DestFile) == false) return false; + RemoveOldLeftoverDiffIndex(Target); break; } return true; @@ -1704,9 +1713,6 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify) /*{{{*/ if (filename.empty() == false) { new NoActionItem(Owner, Target, filename); - std::string const idxfilename = GetFinalFileNameFromURI(GetDiffIndexURI(Target)); - if (FileExists(idxfilename)) - new NoActionItem(Owner, Target, idxfilename); targetsSeen.emplace(Target.Option(IndexTarget::CREATED_BY)); continue; } @@ -2703,12 +2709,8 @@ void pkgAcqDiffIndex::Failed(string const &Message,pkgAcquire::MethodConfig cons new pkgAcqIndex(Owner, TransactionManager, Target); } /*}}}*/ -bool pkgAcqDiffIndex::VerifyDone(std::string const &Message, pkgAcquire::MethodConfig const * const)/*{{{*/ +bool pkgAcqDiffIndex::VerifyDone(std::string const &/*Message*/, pkgAcquire::MethodConfig const * const)/*{{{*/ { - string const FinalFile = GetFinalFilename(); - if(StringToBool(LookupTag(Message,"IMS-Hit"),false)) - DestFile = FinalFile; - if (ParseDiffIndex(DestFile)) return true; @@ -2748,7 +2750,7 @@ void pkgAcqDiffIndex::Done(string const &Message,HashStringList const &Hashes, / } } - TransactionManager->TransactionStageCopy(this, DestFile, GetFinalFilename()); + TransactionManager->TransactionStageRemoval(this, DestFile); Complete = true; Status = StatDone; diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc index 727880e71..1cb55bf14 100644 --- a/apt-pkg/acquire.cc +++ b/apt-pkg/acquire.cc @@ -826,11 +826,11 @@ pkgAcquire::Worker *pkgAcquire::WorkerStep(Worker *I) return I->NextAcquire; } /*}}}*/ -// Acquire::CleanDir - Cleans a directory /*{{{*/ +// CleanDir - Cleans a directory /*{{{*/ // --------------------------------------------------------------------- /* This is a bit simplistic, it looks at every file in the dir and sees if it matches the predicate or not. */ -bool pkgAcquire::CleanDir(string Dir, std::function<bool(char const*)> const &Keep, char const * const Caller) +static bool CleanDir(std::string const &Dir, std::function<bool(std::string_view)> const &Keep, char const * const Caller) { // non-existing directories are by definition clean… if (DirectoryExists(Dir) == false) @@ -855,7 +855,7 @@ bool pkgAcquire::CleanDir(string Dir, std::function<bool(char const*)> const &Ke strcmp(E->d_name, "lost+found") == 0 || strcmp(E->d_name, ".") == 0 || strcmp(E->d_name, "..") == 0 || - Keep(E->d_name) == true) + Keep(E->d_name)) continue; RemoveFileAt(Caller, dirfd, E->d_name); } @@ -867,12 +867,12 @@ bool pkgAcquire::CleanDir(string Dir, std::function<bool(char const*)> const &Ke // --------------------------------------------------------------------- /* This is a bit simplistic, it looks at every file in the dir and sees if it is part of the download set. */ -bool pkgAcquire::Clean(string Dir) +bool pkgAcquire::Clean(std::string Dir) { return CleanDir( Dir, // Look in the get list and if found then keep - [this](char const *FName) { + [this](std::string_view const FName) { return std::any_of(Items.cbegin(), Items.cend(), [FName](pkgAcquire::Item const * const I) { return flNotDir(I->DestFile) == FName; @@ -882,21 +882,19 @@ bool pkgAcquire::Clean(string Dir) ); } /*}}}*/ -// Acquire::CleanLists - Cleans a directory of list files /*{{{*/ -// --------------------------------------------------------------------- -/* */ -bool pkgAcquire::CleanLists(string Dir) +// Acquire::CleanLists - Cleans a directory of list files /*{{{*/ +bool pkgAcquire::CleanLists(std::string const &Dir) { + std::regex const KeepPattern(".*_(Release|Release\\.gpg|InRelease)"); return CleanDir( Dir, - [](char const *FName) noexcept { - static const std::regex KeepPattern(".*_(Release|Release.gpg|InRelease)"); - return std::regex_match(FName, KeepPattern); + [&KeepPattern](std::string_view const FName) noexcept { + return std::regex_match(FName.begin(), FName.end(), KeepPattern); }, "pkgAcquire::CleanLists" ); } - /*}}}*/ + /*}}}*/ // Acquire::TotalNeeded - Number of bytes to fetch /*{{{*/ // --------------------------------------------------------------------- /* This is the total number of bytes needed */ diff --git a/apt-pkg/acquire.h b/apt-pkg/acquire.h index a5f7a40c8..f7c40aa5f 100644 --- a/apt-pkg/acquire.h +++ b/apt-pkg/acquire.h @@ -70,7 +70,6 @@ #include <apt-pkg/weakptr.h> #include <chrono> -#include <functional> #include <string> #include <vector> @@ -338,7 +337,7 @@ class APT_PUBLIC pkgAcquire * * \return \b true if the directory exists and is readable. */ - bool CleanLists(std::string Dir); + bool CleanLists(std::string const &Dir); /** \return the total size in bytes of all the items included in * this download. @@ -381,20 +380,6 @@ class APT_PUBLIC pkgAcquire private: APT_HIDDEN void Initialize(); - - /** Delete each entry in the given directory whose name does \em not match - * a criterion. - * - * \param Dir The directory to be cleaned. - * - * \param KeepP A predicate telling to keep the named file; if it is - * non-empty and returns true, the file is kept. - * - * \param Caller Log name of the caller. - * - * \return \b true if the directory exists and is readable. - */ - APT_HIDDEN static bool CleanDir(std::string Dir, std::function<bool(char const*)> const &KeepP, char const * const Caller); }; /** \brief Represents a single download source from which an item diff --git a/apt-private/private-cmndline.cc b/apt-private/private-cmndline.cc index 980d00530..9e3938407 100644 --- a/apt-private/private-cmndline.cc +++ b/apt-private/private-cmndline.cc @@ -242,14 +242,15 @@ static bool addArgumentsAPTGet(std::vector<CommandLine::Args> &Args, char const addArg(0,"format","APT::Get::IndexTargets::Format", CommandLine::HasArg); addArg(0,"release-info","APT::Get::IndexTargets::ReleaseInfo", 0); } - else if (CmdMatches("clean", "autoclean", "auto-clean", "check", "download", "changelog") || + else if (CmdMatches("clean", "autoclean", "auto-clean", "distclean", "dist-clean", "check", "download", "changelog") || CmdMatches("markauto", "unmarkauto")) // deprecated commands ; else if (CmdMatches("moo")) addArg(0, "color", "APT::Moo::Color", 0); if (CmdMatches("install", "reinstall", "remove", "purge", "upgrade", "dist-upgrade", - "dselect-upgrade", "autoremove", "auto-remove", "autopurge", "clean", "autoclean", "auto-clean", "check", + "dselect-upgrade", "autoremove", "auto-remove", "autopurge", "check", + "clean", "autoclean", "auto-clean", "distclean", "dist-clean", "build-dep", "satisfy", "full-upgrade", "source")) { addArg('s', "simulate", "APT::Get::Simulate", 0); diff --git a/apt-private/private-download.cc b/apt-private/private-download.cc index b271ec03f..84a088f3d 100644 --- a/apt-private/private-download.cc +++ b/apt-private/private-download.cc @@ -303,20 +303,8 @@ bool DoChangelog(CommandLine &CmdL) return true; } /*}}}*/ - -// DoClean - Remove download archives /*{{{*/ -bool DoClean(CommandLine &) -{ - return DoDistClean(false); -} - /*}}}*/ -// DoDistClean - Remove download archives and lists /*{{{*/ -bool DoDistClean(CommandLine &) -{ - return DoDistClean(true); -} -// DoDistClean - the worker -bool DoDistClean(bool ListsToo) +// DoClean & DoDistClean - Remove download archives and/or lists /*{{{*/ +static bool CleanDownloadDirectories(bool const ListsToo) { std::string const archivedir = _config->FindDir("Dir::Cache::archives"); std::string const listsdir = _config->FindDir("Dir::state::lists"); @@ -325,22 +313,24 @@ bool DoDistClean(bool ListsToo) { std::string const pkgcache = _config->FindFile("Dir::cache::pkgcache"); std::string const srcpkgcache = _config->FindFile("Dir::cache::srcpkgcache"); - std::cout << "Del " << archivedir << "* " << archivedir << "partial/*"<< std::endl - << "Del " << listsdir << "partial/*" << (ListsToo ? " *_{Packages,Sources}{,.diff_Index}" : "") << std::endl - << "Del " << pkgcache << " " << srcpkgcache << std::endl; + std::cout << "Del " << archivedir << "* " << archivedir << "partial/*" << std::endl + << "Del " << listsdir << "partial/*" << std::endl; + if (ListsToo) + std::cout << "Del " << listsdir << "*_{Packages,Sources,Translation-*}" << std::endl; + std::cout << "Del " << pkgcache << " " << srcpkgcache << std::endl; return true; } pkgAcquire Fetcher; - if (archivedir.empty() == false && FileExists(archivedir) == true && - Fetcher.GetLock(archivedir) == true) + if (not archivedir.empty() && FileExists(archivedir) && + Fetcher.GetLock(archivedir)) { Fetcher.Clean(archivedir); Fetcher.Clean(archivedir + "partial/"); } - if (listsdir.empty() == false && FileExists(listsdir) == true && - Fetcher.GetLock(listsdir) == true) + if (not listsdir.empty() && FileExists(listsdir) && + Fetcher.GetLock(listsdir)) { Fetcher.Clean(listsdir + "partial/"); if (ListsToo) @@ -351,6 +341,14 @@ bool DoDistClean(bool ListsToo) return true; } +bool DoClean(CommandLine &) +{ + return CleanDownloadDirectories(false); +} +bool DoDistClean(CommandLine &) +{ + return CleanDownloadDirectories(true); +} /*}}}*/ // DoAutoClean - Smartly remove downloaded archives /*{{{*/ // --------------------------------------------------------------------- diff --git a/apt-private/private-download.h b/apt-private/private-download.h index 918828241..e13ade1c6 100644 --- a/apt-private/private-download.h +++ b/apt-private/private-download.h @@ -34,7 +34,6 @@ APT_PUBLIC bool DoChangelog(CommandLine &CmdL); APT_PUBLIC bool DoClean(CommandLine &CmdL); APT_PUBLIC bool DoDistClean(CommandLine &CmdL); -bool DoDistClean(bool ListsToo); APT_PUBLIC bool DoAutoClean(CommandLine &CmdL); #endif diff --git a/cmdline/apt-get.cc b/cmdline/apt-get.cc index 95c4a85ab..d41e0f243 100644 --- a/cmdline/apt-get.cc +++ b/cmdline/apt-get.cc @@ -424,6 +424,8 @@ static std::vector<aptDispatchWithHelp> GetCommands() /*{{{*/ {"clean", &DoClean, _("Erase downloaded archive files")}, {"autoclean", &DoAutoClean, _("Erase old downloaded archive files")}, {"auto-clean", &DoAutoClean, nullptr}, + {"distclean", &DoDistClean, nullptr}, + {"dist-clean", &DoDistClean, nullptr}, {"check", &DoCheck, _("Verify that there are no broken dependencies")}, {"source", &DoSource, _("Download source archives")}, {"download", &DoDownload, _("Download the binary package into the current directory")}, diff --git a/test/integration/test-apt-get-clean b/test/integration/test-apt-get-clean index d05073218..32dd3e852 100755 --- a/test/integration/test-apt-get-clean +++ b/test/integration/test-apt-get-clean @@ -13,13 +13,27 @@ insertpackage 'experimental' 'foo' 'all' '1:1' insertinstalledpackage 'foo' 'all' '3' setupaptarchive --no-update +mkdir -p rootdir/var/lib/apt/lists/partial +testsuccess test -d rootdir/var/lib/apt/lists/partial + +# nothing to do always works (part 1) +for cmd in aptget apt; do + for sub in distclean dist-clean; do + testsuccess $cmd $sub + testsuccess $cmd $sub -s + done +done +testsuccess test -d rootdir/var/lib/apt/lists/partial -mkdir -p rootdir/var/lib/apt/lists/lost+found testsuccess apt update -# nothing to do always works -testsuccess aptget clean -testsuccess aptget clean -s +# nothing to do always works (part 2) +for cmd in aptget apt; do + for sub in clean autoclean auto-clean; do + testsuccess $cmd $sub + testsuccess $cmd $sub -s + done +done # generate some dirt and clean it up generatedirt() { @@ -63,6 +77,23 @@ if [ "$(id -u)" != '0' ]; then chmod 644 rootdir/var/cache/apt/archives/lock fi +msgmsg 'distclean' +listcurrentlistsdirectory > aptdistclean.before +testsuccessequal '3' grep -c -e 'Release$' aptdistclean.before +testsuccess grep -e 'Packages$' -e 'Sources$' -e 'Translation-*$' aptdistclean.before +testsuccess apt distclean +listcurrentlistsdirectory > aptdistclean.after +testfailure cmp aptdistclean.before aptdistclean.after +testsuccessequal '3' grep -c -e 'Release$' aptdistclean.after +testfailureequal '0' grep -c -e 'Packages$' -e 'Sources$' -e 'Translation-*$' aptdistclean.after +for dirt in Packages.gz Sources.xz Release2gpg Release_Packages; do + touch "rootdir/var/lib/apt/lists/_foo_bar_$dirt" +done +listcurrentlistsdirectory > aptdistclean.before +testsuccess apt distclean +listcurrentlistsdirectory > aptdistclean.cleaned +testsuccess cmp aptdistclean.after aptdistclean.cleaned + directorygone() { rm -rf "$1" testsuccess apt autoclean @@ -86,3 +117,4 @@ directorygone 'rootdir/var/lib/apt/lists' msgmsg 'apt directory missing' directorygone 'rootdir/var/cache/apt' directorygone 'rootdir/var/lib/apt' + diff --git a/test/integration/test-pdiff-usage b/test/integration/test-pdiff-usage index 38e455b00..cf3a2f2d9 100755 --- a/test/integration/test-pdiff-usage +++ b/test/integration/test-pdiff-usage @@ -38,14 +38,7 @@ wasmergeused() { #apt update "$@" 2>&1 | tee rootdir/tmp/testsuccess.output msgtest 'No intermediate patch files' 'still exist' - local EDS="$(find rootdir/var/lib/apt/lists -name '*.ed' -o -name '*.ed.*')" - if [ -z "$EDS" ]; then - msgpass - else - echo - echo "$EDS" - msgfail - fi + testempty --nomsg find rootdir/var/lib/apt/lists -name '*.ed' -o -name '*.ed.*' -o -name '*.diff_Index' -o -name '*.diff_Index.xz' if echo "$*" | grep -q -- '-o test::cannot-use-pdiff=1'; then msgtest 'Check if pdiff was' 'not used' @@ -389,21 +382,25 @@ testcase() { testrun -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=0 "$@" } generatepartialleftovers() { + local PREFIX="$1" + shift for f in "$@"; do - cat "${PKGFILE}" "${PKGFILE}" > "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${f}" - printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${f}" + cat "${PKGFILE}" "${PKGFILE}" > "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${PREFIX}${f}" + printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${PREFIX}${f}" done + printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/localhost:${APTHTTPPORT}_${PREFIX}Packages.diff_Index" + printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/localhost:${APTHTTPPORT}_${PREFIX}Packages.diff_Index.xz" } -partialleftovers() { generatepartialleftovers 'Packages' 'Packages-patched'; } +partialleftovers() { generatepartialleftovers '' 'Packages' 'Packages-patched'; } aptautotest_apt_update() { aptautotest_aptget_update "$@"; testsuccess test -e "rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_Packages"; } testcase -o Acquire::IndexTargets::deb::Packages::KeepCompressed=false -partialleftovers() { generatepartialleftovers "Packages.${LOWCOSTEXT}" "Packages-patched.${LOWCOSTEXT}"; } +partialleftovers() { generatepartialleftovers '' "Packages.${LOWCOSTEXT}" "Packages-patched.${LOWCOSTEXT}"; } aptautotest_apt_update() { aptautotest_aptget_update "$@"; testsuccess test -e "rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_Packages.$LOWCOSTEXT"; } testcase -o Acquire::IndexTargets::deb::Packages::KeepCompressed=true -partialleftovers() { generatepartialleftovers "redirectme_Packages.${LOWCOSTEXT}" "redirectme_Packages-patched.${LOWCOSTEXT}"; } +partialleftovers() { generatepartialleftovers 'redirectme_' "Packages.${LOWCOSTEXT}" "Packages-patched.${LOWCOSTEXT}"; } # redirect the InRelease file only – the other files are auto-redirected by apt webserverconfig 'aptwebserver::redirect::replace::/redirectme/I' "http://0.0.0.0:${APTHTTPPORT}/I" rewritesourceslist "http://localhost:${APTHTTPPORT}/redirectme" |