From 9bd2313a5c7523501bcec398877489c5a1fc1415 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 3 Jan 2016 19:23:30 +0100 Subject: use one 'store' method to rule all (de)compressors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding a new compressor method meant adding a new method as well – even if that boilt down to just linking to our generalized decompressor with a new name. That is unneeded busywork if we can instead just call the generalized decompressor and let it figure out which compressor to use based on the filenames rather than by program name. For compatibility we ship still 'gzip', 'bzip2' and co, but they are just links to our "new" 'store' method. --- apt-pkg/acquire-item.cc | 37 +++-- methods/gzip.cc | 151 ------------------- methods/makefile | 14 +- methods/store.cc | 159 +++++++++++++++++++++ .../test-bug-595691-empty-and-broken-archive-files | 4 +- 5 files changed, 190 insertions(+), 175 deletions(-) delete mode 100644 methods/gzip.cc create mode 100644 methods/store.cc diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index ffe5bd1c4..6b0debb44 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -422,7 +422,7 @@ bool pkgAcqIndex::TransactionState(TransactionStates const state) { // keep the compressed file, but drop the decompressed EraseFileName.clear(); - if (PartialFile.empty() == false && flExtension(PartialFile) == "decomp") + if (PartialFile.empty() == false && flExtension(PartialFile) != CurrentCompressionExtension) RemoveFile("TransactionAbort", PartialFile); } break; @@ -2681,7 +2681,8 @@ void pkgAcqIndex::StageDownloadDone(string const &Message, HashStringList const { Stage = STAGE_DECOMPRESS_AND_VERIFY; Local = true; - DestFile += ".decomp"; + if (CurrentCompressionExtension != "uncompressed") + DestFile.erase(DestFile.length() - (CurrentCompressionExtension.length() + 1)); Desc.URI = "copy:" + FileName; QueueURI(Desc); SetActiveSubprocess("copy"); @@ -2703,6 +2704,18 @@ void pkgAcqIndex::StageDownloadDone(string const &Message, HashStringList const SetActiveSubprocess("copy"); return; } + else + { + // symlinking ensures that the filename can be used for compression detection + // that is e.g. needed for by-hash over file + if (symlink(FileName.c_str(),DestFile.c_str()) != 0) + _error->WarningE("pkgAcqIndex::StageDownloadDone", "Symlinking file %s to %s failed", FileName.c_str(), DestFile.c_str()); + else + { + EraseFileName = DestFile; + FileName = DestFile; + } + } } else EraseFileName = FileName; @@ -2717,25 +2730,19 @@ void pkgAcqIndex::StageDownloadDone(string const &Message, HashStringList const return; } - // get the binary name for your used compression type - string decompProg; - if(CurrentCompressionExtension == "uncompressed") - decompProg = "copy"; - else - decompProg = _config->Find(string("Acquire::CompressionTypes::").append(CurrentCompressionExtension),""); - if(decompProg.empty() == true) - { - _error->Error("Unsupported extension: %s", CurrentCompressionExtension.c_str()); - return; - } - + string decompProg = "store"; if (Target.KeepCompressed == true) { DestFile = "/dev/null"; EraseFileName.clear(); } else - DestFile += ".decomp"; + { + if (CurrentCompressionExtension == "uncompressed") + decompProg = "copy"; + else + DestFile.erase(DestFile.length() - (CurrentCompressionExtension.length() + 1)); + } // queue uri for the next stage Stage = STAGE_DECOMPRESS_AND_VERIFY; diff --git a/methods/gzip.cc b/methods/gzip.cc deleted file mode 100644 index c470807ac..000000000 --- a/methods/gzip.cc +++ /dev/null @@ -1,151 +0,0 @@ -// -*- mode: cpp; mode: fold -*- -// Description /*{{{*/ -// $Id: gzip.cc,v 1.17.2.1 2004/01/16 18:58:50 mdz Exp $ -/* ###################################################################### - - GZip method - Take a file URI in and decompress it into the target - file. - - ##################################################################### */ - /*}}}*/ -// Include Files /*{{{*/ -#include - -#include -#include -#include -#include -#include -#include -#include -#include "aptmethod.h" - -#include -#include -#include -#include -#include - -#include - /*}}}*/ - -class GzipMethod : public aptMethod -{ - std::string const Prog; - virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; - - public: - - explicit GzipMethod(std::string const &pProg) : aptMethod(pProg.c_str(),"1.1",SingleInstance | SendConfig), Prog(pProg) {}; -}; - -// GzipMethod::Fetch - Decompress the passed URI /*{{{*/ -// --------------------------------------------------------------------- -/* */ -bool GzipMethod::Fetch(FetchItem *Itm) -{ - URI Get = Itm->Uri; - std::string Path = Get.Host + Get.Path; // To account for relative paths - - FetchResult Res; - Res.Filename = Itm->DestFile; - URIStart(Res); - - std::vector const compressors = APT::Configuration::getCompressors(); - std::vector::const_iterator compressor = compressors.begin(); - for (; compressor != compressors.end(); ++compressor) - if (compressor->Name == Prog) - break; - if (compressor == compressors.end()) - return _error->Error("Extraction of file %s requires unknown compressor %s", Path.c_str(), Prog.c_str()); - - // Open the source and destination files - FileFd From; - if (_config->FindB("Method::Compress", false) == false) - { - From.Open(Path, FileFd::ReadOnly, *compressor); - if(From.FileSize() == 0) - return _error->Error(_("Empty files can't be valid archives")); - } - else - From.Open(Path, FileFd::ReadOnly); - if (From.IsOpen() == false || From.Failed() == true) - return false; - - FileFd To; - if (Itm->DestFile != "/dev/null") - { - if (_config->FindB("Method::Compress", false) == false) - To.Open(Itm->DestFile, FileFd::WriteAtomic); - else - To.Open(Itm->DestFile, FileFd::WriteOnly | FileFd::Create | FileFd::Empty, *compressor); - - if (To.IsOpen() == false || To.Failed() == true) - return false; - To.EraseOnFailure(); - } - - - // Read data from source, generate checksums and write - Hashes Hash(Itm->ExpectedHashes); - bool Failed = false; - Res.Size = 0; - while (1) - { - unsigned char Buffer[4*1024]; - unsigned long long Count = 0; - - if (!From.Read(Buffer,sizeof(Buffer),&Count)) - { - if (To.IsOpen()) - To.OpFail(); - return false; - } - if (Count == 0) - break; - Res.Size += Count; - - Hash.Add(Buffer,Count); - if (To.IsOpen() && To.Write(Buffer,Count) == false) - { - Failed = true; - break; - } - } - - From.Close(); - To.Close(); - - if (Failed == true) - return false; - - // Transfer the modification times - if (Itm->DestFile != "/dev/null") - { - struct stat Buf; - if (stat(Path.c_str(),&Buf) != 0) - return _error->Errno("stat",_("Failed to stat")); - - struct timeval times[2]; - times[0].tv_sec = Buf.st_atime; - Res.LastModified = times[1].tv_sec = Buf.st_mtime; - times[0].tv_usec = times[1].tv_usec = 0; - if (utimes(Itm->DestFile.c_str(), times) != 0) - return _error->Errno("utimes",_("Failed to set modification time")); - } - - // Return a Done response - Res.TakeHashes(Hash); - - URIDone(Res); - return true; -} - /*}}}*/ - -int main(int, char *argv[]) -{ - setlocale(LC_ALL, ""); - - GzipMethod Mth(flNotDir(argv[0])); - return Mth.Run(); -} diff --git a/methods/makefile b/methods/makefile index 868c52a40..3274e9279 100644 --- a/methods/makefile +++ b/methods/makefile @@ -23,11 +23,11 @@ LIB_MAKES = apt-pkg/makefile SOURCE = copy.cc include $(PROGRAM_H) -# The gzip method -PROGRAM=gzip +# The store method +PROGRAM=store SLIBS = -lapt-pkg $(INTLLIBS) LIB_MAKES = apt-pkg/makefile -SOURCE = gzip.cc +SOURCE = store.cc include $(PROGRAM_H) # The gpgv method @@ -96,15 +96,15 @@ $(BIN)/ssh: clean-$(BIN)/ssh: -rm $(BIN)/ssh -# create links for all other compressors -COMPRESSORS=bzip2 lzma xz +# create compat links for all compressors +COMPRESSORS=gzip bzip2 lzma xz binary: $(addprefix $(BIN)/,$(COMPRESSORS)) veryclean: $(addprefix clean-$(BIN)/,$(COMPRESSORS)) -$(addprefix $(BIN)/,$(COMPRESSORS)): $(BIN)/gzip +$(addprefix $(BIN)/,$(COMPRESSORS)): $(BIN)/store echo "Installing $(notdir $@) method link" - ln -fs gzip $@ + ln -fs store $@ $(addprefix clean-$(BIN)/,$(COMPRESSORS)): -rm $(BIN)/$(notdir $@) diff --git a/methods/store.cc b/methods/store.cc new file mode 100644 index 000000000..29cf9e947 --- /dev/null +++ b/methods/store.cc @@ -0,0 +1,159 @@ +// -*- mode: cpp; mode: fold -*- +// Description /*{{{*/ +/* ###################################################################### + + Store method - Takes a file URI and stores its content (for which it will + calculate the hashes) in the given destination. The input file will be + extracted based on its file extension (or with the given compressor if + called with one of the compatible symlinks) and potentially recompressed + based on the file extension of the destination filename. + + ##################################################################### */ + /*}}}*/ +// Include Files /*{{{*/ +#include + +#include +#include +#include +#include +#include +#include +#include +#include "aptmethod.h" + +#include +#include +#include +#include +#include + +#include + /*}}}*/ + +class StoreMethod : public aptMethod +{ + std::string const Prog; + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; + + public: + + explicit StoreMethod(std::string const &pProg) : aptMethod(pProg.c_str(),"1.2",SingleInstance | SendConfig), Prog(pProg) {}; +}; + +static bool OpenFileWithCompressorByName(FileFd &fileFd, std::string const &Filename, unsigned int const Mode, std::string const &Name) +{ + if (Name == "store") + return fileFd.Open(Filename, Mode, FileFd::Extension); + + std::vector const compressors = APT::Configuration::getCompressors(); + std::vector::const_iterator compressor = compressors.begin(); + for (; compressor != compressors.end(); ++compressor) + if (compressor->Name == Name) + break; + if (compressor == compressors.end()) + return _error->Error("Extraction of file %s requires unknown compressor %s", Filename.c_str(), Name.c_str()); + return fileFd.Open(Filename, Mode, *compressor); +} + + + /*}}}*/ +bool StoreMethod::Fetch(FetchItem *Itm) /*{{{*/ +{ + URI Get = Itm->Uri; + std::string Path = Get.Host + Get.Path; // To account for relative paths + + FetchResult Res; + Res.Filename = Itm->DestFile; + URIStart(Res); + + // Open the source and destination files + FileFd From; + if (_config->FindB("Method::Compress", false) == false) + { + if (OpenFileWithCompressorByName(From, Path, FileFd::ReadOnly, Prog) == false) + return false; + if(From.FileSize() == 0) + return _error->Error(_("Empty files can't be valid archives")); + } + else + From.Open(Path, FileFd::ReadOnly, FileFd::Extension); + if (From.IsOpen() == false || From.Failed() == true) + return false; + + FileFd To; + if (Itm->DestFile != "/dev/null" && Itm->DestFile != Path) + { + if (_config->FindB("Method::Compress", false) == false) + To.Open(Itm->DestFile, FileFd::WriteOnly | FileFd::Create | FileFd::Atomic, FileFd::Extension); + else if (OpenFileWithCompressorByName(To, Itm->DestFile, FileFd::WriteOnly | FileFd::Create | FileFd::Empty, Prog) == false) + return false; + + if (To.IsOpen() == false || To.Failed() == true) + return false; + To.EraseOnFailure(); + } + + // Read data from source, generate checksums and write + Hashes Hash(Itm->ExpectedHashes); + bool Failed = false; + Res.Size = 0; + while (1) + { + unsigned char Buffer[4*1024]; + unsigned long long Count = 0; + + if (!From.Read(Buffer,sizeof(Buffer),&Count)) + { + if (To.IsOpen()) + To.OpFail(); + return false; + } + if (Count == 0) + break; + Res.Size += Count; + + Hash.Add(Buffer,Count); + if (To.IsOpen() && To.Write(Buffer,Count) == false) + { + Failed = true; + break; + } + } + + From.Close(); + To.Close(); + + if (Failed == true) + return false; + + // Transfer the modification times + if (Itm->DestFile != "/dev/null") + { + struct stat Buf; + if (stat(Path.c_str(),&Buf) != 0) + return _error->Errno("stat",_("Failed to stat")); + + struct timeval times[2]; + times[0].tv_sec = Buf.st_atime; + Res.LastModified = times[1].tv_sec = Buf.st_mtime; + times[0].tv_usec = times[1].tv_usec = 0; + if (utimes(Itm->DestFile.c_str(), times) != 0) + return _error->Errno("utimes",_("Failed to set modification time")); + } + + // Return a Done response + Res.TakeHashes(Hash); + + URIDone(Res); + return true; +} + /*}}}*/ + +int main(int, char *argv[]) +{ + setlocale(LC_ALL, ""); + + StoreMethod Mth(flNotDir(argv[0])); + return Mth.Run(); +} diff --git a/test/integration/test-bug-595691-empty-and-broken-archive-files b/test/integration/test-bug-595691-empty-and-broken-archive-files index e24ee6b44..c216918c5 100755 --- a/test/integration/test-bug-595691-empty-and-broken-archive-files +++ b/test/integration/test-bug-595691-empty-and-broken-archive-files @@ -55,7 +55,7 @@ Reading package lists..." "empty archive Packages.$COMPRESS over file" Err:2 file:$APTARCHIVE Packages Empty files can't be valid archives Reading package lists... -W: Failed to fetch ${COMPRESSOR}:${APTARCHIVE}/Packages.$COMPRESS Empty files can't be valid archives +W: Failed to fetch store:$(readlink -f rootdir/var/lib/apt/lists/partial/$(echo "$APTARCHIVE" | sed -e 's#/#_#g')_Packages.${COMPRESS}) Empty files can't be valid archives E: Some index files failed to download. They have been ignored, or old ones used instead." "empty file Packages.$COMPRESS over file" } @@ -72,7 +72,7 @@ Reading package lists..." "empty archive Packages.$COMPRESS over http" Err:2 http://localhost:${APTHTTPPORT} Packages Empty files can't be valid archives Reading package lists... -W: Failed to fetch ${COMPRESSOR}:$(readlink -f rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_Packages.${COMPRESS}) Empty files can't be valid archives +W: Failed to fetch store:$(readlink -f rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_Packages.${COMPRESS}) Empty files can't be valid archives E: Some index files failed to download. They have been ignored, or old ones used instead." "empty file Packages.$COMPRESS over http" } -- cgit v1.2.3