Bug#649451: hard-coded gzip-only support in apt-cdrom
On Wed, Nov 23, 2011 at 17:44, Steve McIntyre <steve@einval.com> wrote:
> The main change is that I've factored out the decompressor code in
> IndexCopy::CopyPackages() and TranslationsCopy::CopyTranslations()
> into a single common function, rather than the current repeated
> code. Then I've added support for bzip2 and xz there, by forking and
> execing the right decompression tool. I'm not a C++ developer by any
> means, as you'll probably tell. :-) You'll definitely want to move
> DecompressFile() somewhere else...
Thanks! The patch is far from being bad, so don't worry.
(beside that it doesn't follow the insane indent style - but is that bad?)
I played with it a bit more, but i am running out of time for today:
Attached patch should apply on experimental, for sid you need to
apply Julians first. It should work (tm), but is completely untested,
so it properly doesn't. Interesting is that DecompressFile is only a
slightly modified copy of MultiCompress::OpenCompress in ftparchive,
so we really want to move it somewhere accessible and your
FileFd suggestion seems like an ideal fit, i will try to work on that further.
> A *much* better solution IMHO would be to extend class FileFd (which
> already has ReadOnlyGzip as an option) for either specific support for
> other compression methods, or to replace it with a generic option
> ReadOnlyCompressed that can do the right thing. *BUT* that will then
> mean linking directly with libbz2, liblzma etc. which we may not
> want due to extra library dependencies.
dpkg does or properly will soon depend on all of them given that
data.tar-members could be compressed with those algorithms,
so it should be save to depend on them, mhhhh…
Best regards
David Kalnischkies
=== modified file 'apt-pkg/cdrom.cc'
--- apt-pkg/cdrom.cc 2011-11-22 17:59:28 +0000
+++ apt-pkg/cdrom.cc 2011-11-23 22:55:56 +0000
@@ -82,66 +82,68 @@
/* Aha! We found some package files. We assume that everything under
this dir is controlled by those package files so we don't look down
anymore */
- std::vector<std::string> types = APT::Configuration::getCompressionTypes();
- types.push_back("");
- for (std::vector<std::string>::const_iterator t = types.begin();
- t != types.end(); ++t)
- {
- std::string filename = std::string("Packages");
- if ((*t).size() > 0)
- filename.append("."+*t);
- if (stat(filename.c_str(), &Buf) == 0)
- {
- List.push_back(CD);
-
- // Continue down if thorough is given
- if (_config->FindB("APT::CDROM::Thorough",false) == false)
- return true;
- break;
- }
- }
- for (std::vector<std::string>::const_iterator t = types.begin();
- t != types.end(); ++t)
- {
- std::string filename = std::string("Sources");
- if ((*t).size() > 0)
- filename.append("."+*t);
- {
- SList.push_back(CD);
-
- // Continue down if thorough is given
- if (_config->FindB("APT::CDROM::Thorough",false) == false)
- return true;
- break;
- }
- }
-
- // see if we find translatin indexes
- if (stat("i18n",&Buf) == 0)
+ std::vector<APT::Configuration::Compressor> const compressor = APT::Configuration::getCompressors();
+ for (std::vector<APT::Configuration::Compressor>::const_iterator c = compressor.begin();
+ c != compressor.end(); ++c)
+ {
+ if (stat(std::string("Packages").append(c->Extension).c_str(), &Buf) != 0)
+ continue;
+
+ if (_config->FindB("Debug::aptcdrom",false) == true)
+ std::clog << "Found Packages in " << CD << std::endl;
+ List.push_back(CD);
+
+ // Continue down if thorough is given
+ if (_config->FindB("APT::CDROM::Thorough",false) == false)
+ return true;
+ break;
+ }
+ for (std::vector<APT::Configuration::Compressor>::const_iterator c = compressor.begin();
+ c != compressor.end(); ++c)
+ {
+ if (stat(std::string("Sources").append(c->Extension).c_str(), &Buf) != 0)
+ continue;
+
+ if (_config->FindB("Debug::aptcdrom",false) == true)
+ std::clog << "Found Sources in " << CD << std::endl;
+ SList.push_back(CD);
+
+ // Continue down if thorough is given
+ if (_config->FindB("APT::CDROM::Thorough",false) == false)
+ return true;
+ break;
+ }
+
+ // see if we find translation indices
+ if (DirectoryExists("i18n") == true)
{
D = opendir("i18n");
for (struct dirent *Dir = readdir(D); Dir != 0; Dir = readdir(D))
{
- if(strstr(Dir->d_name,"Translation") != NULL)
+ if(strncmp(Dir->d_name, "Translation-", strlen("Translation-")) != 0)
+ continue;
+ string file = Dir->d_name;
+ for (std::vector<APT::Configuration::Compressor>::const_iterator c = compressor.begin();
+ c != compressor.end(); ++c)
{
- if (_config->FindB("Debug::aptcdrom",false) == true)
- std::clog << "found translations: " << Dir->d_name << "\n";
- string file = Dir->d_name;
- for (std::vector<std::string>::const_iterator t = types.begin();
- t != types.end(); ++t)
- {
- std::string needle = "." + *t;
- if(file.substr(file.size()-needle.size()) == needle)
- file = file.substr(0, file.size()-needle.size());
- TransList.push_back(CD+"i18n/"+ file);
- break;
- }
+ string fileext = flExtension(file);
+ if (file == fileext)
+ fileext.clear();
+ else if (fileext.empty() == false)
+ fileext = "." + fileext;
+
+ if (c->Extension == fileext)
+ {
+ if (_config->FindB("Debug::aptcdrom",false) == true)
+ std::clog << "Found translation " << Dir->d_name << " in " << CD << "i18n/" << std::endl;
+ TransList.push_back(CD + "i18n/" + file);
+ break;
+ }
}
}
closedir(D);
}
-
D = opendir(".");
if (D == 0)
return _error->Errno("opendir","Unable to read %s",CD.c_str());
@@ -278,24 +280,27 @@
{
// Get a list of all the inodes
ino_t *Inodes = new ino_t[List.size()];
- for (unsigned int I = 0; I != List.size(); I++)
+ for (unsigned int I = 0; I != List.size(); ++I)
{
struct stat Buf;
- std::vector<std::string> types = APT::Configuration::getCompressionTypes();
- types.push_back("");
- for (std::vector<std::string>::const_iterator t = types.begin();
- t != types.end(); ++t)
+ bool found = false;
+
+ std::vector<APT::Configuration::Compressor> const compressor = APT::Configuration::getCompressors();
+ for (std::vector<APT::Configuration::Compressor>::const_iterator c = compressor.begin();
+ c != compressor.end(); ++c)
{
- std::string filename = List[I] + Name;
- if ((*t).size() > 0)
- filename.append("." + *t);
+ std::string filename = std::string(List[I]).append(Name).append(c->Extension);
if (stat(filename.c_str(), &Buf) != 0)
- _error->Errno("stat","Failed to stat %s%s",List[I].c_str(),
- Name);
- Inodes[I] = Buf.st_ino;
+ continue;
+ Inodes[I] = Buf.st_ino;
+ found = true;
+ break;
}
+
+ if (found == false)
+ _error->Errno("stat","Failed to stat %s%s",List[I].c_str(), Name);
}
-
+
if (_error->PendingError() == true) {
delete[] Inodes;
return false;
=== modified file 'apt-pkg/indexcopy.cc'
--- apt-pkg/indexcopy.cc 2011-09-13 15:52:22 +0000
+++ apt-pkg/indexcopy.cc 2011-11-23 23:48:48 +0000
@@ -16,6 +16,7 @@
#include <apt-pkg/progress.h>
#include <apt-pkg/strutl.h>
#include <apt-pkg/fileutl.h>
+#include <apt-pkg/aptconfiguration.h>
#include <apt-pkg/configuration.h>
#include <apt-pkg/tagfile.h>
#include <apt-pkg/indexrecords.h>
@@ -37,8 +38,65 @@
using namespace std;
-
-
+// DecompressFile - wrapper for decompressing compressed files /*{{{*/
+// ---------------------------------------------------------------------
+/* */
+bool DecompressFile(string Filename, int *fd, off_t *FileSize)
+{
+ struct stat Buf;
+ *fd = -1;
+
+ std::vector<APT::Configuration::Compressor> const compressor = APT::Configuration::getCompressors();
+ std::vector<APT::Configuration::Compressor>::const_iterator UnCompress;
+ std::string file = std::string(Filename).append(UnCompress->Extension);
+ for (UnCompress = compressor.begin(); UnCompress != compressor.end(); ++UnCompress)
+ {
+ if (stat(file.c_str(), &Buf) == 0)
+ break;
+ }
+
+ if (UnCompress == compressor.end())
+ return _error->Errno("decompressor", "Unable to parse file");
+
+ *FileSize = Buf.st_size;
+
+ // Create a data pipe
+ int Pipe[2] = {-1,-1};
+ if (pipe(Pipe) != 0)
+ return _error->Errno("pipe",_("Failed to create subprocess IPC"));
+ for (int J = 0; J != 2; J++)
+ SetCloseExec(Pipe[J],true);
+
+ *fd = Pipe[1];
+
+ // The child..
+ pid_t Pid = ExecFork();
+ if (Pid == 0)
+ {
+ dup2(Pipe[1],STDOUT_FILENO);
+ SetCloseExec(STDOUT_FILENO, false);
+
+ std::vector<char const*> Args;
+ Args.push_back(UnCompress->Binary.c_str());
+ for (std::vector<std::string>::const_iterator a = UnCompress->UncompressArgs.begin();
+ a != UnCompress->UncompressArgs.end(); ++a)
+ Args.push_back(a->c_str());
+ Args.push_back("--stdout");
+ Args.push_back(file.c_str());
+ Args.push_back(NULL);
+
+ execvp(Args[0],(char **)&Args[0]);
+ cerr << _("Failed to exec compressor ") << Args[0] << endl;
+ _exit(100);
+ }
+
+ // Wait for decompress to finish
+ if (ExecWait(Pid, UnCompress->Binary.c_str(), false) == false)
+ return false;
+
+ return true;
+}
+ /*}}}*/
// IndexCopy::CopyPackages - Copy the package files from the CD /*{{{*/
// ---------------------------------------------------------------------
/* */
@@ -57,15 +115,25 @@
// Prepare the progress indicator
off_t TotalSize = 0;
+ std::vector<APT::Configuration::Compressor> const compressor = APT::Configuration::getCompressors();
for (vector<string>::iterator I = List.begin(); I != List.end(); ++I)
{
struct stat Buf;
- if (stat(string(*I + GetFileName()).c_str(),&Buf) != 0 &&
- stat(string(*I + GetFileName() + ".gz").c_str(),&Buf) != 0)
- return _error->Errno("stat","Stat failed for %s",
- string(*I + GetFileName()).c_str());
+ bool found = false;
+ std::string file = std::string(*I).append(GetFileName());
+ for (std::vector<APT::Configuration::Compressor>::const_iterator c = compressor.begin();
+ c != compressor.end(); ++c)
+ {
+ if (stat(std::string(file + c->Extension).c_str(), &Buf) != 0)
+ continue;
+ found = true;
+ break;
+ }
+
+ if (found == false)
+ return _error->Errno("stat", "Stat failed for %s", file.c_str());
TotalSize += Buf.st_size;
- }
+ }
off_t CurrentSize = 0;
unsigned int NotFound = 0;
@@ -85,46 +153,14 @@
}
else
{
- FileFd From(*I + GetFileName() + ".gz",FileFd::ReadOnly);
- if (_error->PendingError() == true)
- return false;
- FileSize = From.Size();
-
- // Get a temp file
- FILE *tmp = tmpfile();
- if (tmp == 0)
- return _error->Errno("tmpfile","Unable to create a tmp file");
- Pkg.Fd(dup(fileno(tmp)));
- fclose(tmp);
-
- // Fork gzip
- pid_t Process = fork();
- if (Process < 0)
- return _error->Errno("fork","Couldn't fork gzip");
-
- // The child
- if (Process == 0)
- {
- dup2(From.Fd(),STDIN_FILENO);
- dup2(Pkg.Fd(),STDOUT_FILENO);
- SetCloseExec(STDIN_FILENO,false);
- SetCloseExec(STDOUT_FILENO,false);
-
- const char *Args[3];
- string Tmp = _config->Find("Dir::bin::gzip","gzip");
- Args[0] = Tmp.c_str();
- Args[1] = "-d";
- Args[2] = 0;
- execvp(Args[0],(char **)Args);
- exit(100);
- }
-
- // Wait for gzip to finish
- if (ExecWait(Process,_config->Find("Dir::bin::gzip","gzip").c_str(),false) == false)
- return _error->Error("gzip failed, perhaps the disk is full.");
-
+ int fd;
+ if (!DecompressFile(string(*I + GetFileName()), &fd, &FileSize))
+ return _error->Errno("decompress","Decompress failed for %s",
+ string(*I + GetFileName()).c_str());
+ Pkg.Fd(dup(fd));
Pkg.Seek(0);
}
+
pkgTagFile Parser(&Pkg);
if (_error->PendingError() == true)
return false;
@@ -789,15 +825,25 @@
// Prepare the progress indicator
off_t TotalSize = 0;
+ std::vector<APT::Configuration::Compressor> const compressor = APT::Configuration::getCompressors();
for (vector<string>::iterator I = List.begin(); I != List.end(); ++I)
{
struct stat Buf;
- if (stat(string(*I).c_str(),&Buf) != 0 &&
- stat(string(*I + ".gz").c_str(),&Buf) != 0)
- return _error->Errno("stat","Stat failed for %s",
- string(*I).c_str());
+ bool found = false;
+ std::string file = *I;
+ for (std::vector<APT::Configuration::Compressor>::const_iterator c = compressor.begin();
+ c != compressor.end(); ++c)
+ {
+ if (stat(std::string(file + c->Extension).c_str(), &Buf) != 0)
+ continue;
+ found = true;
+ break;
+ }
+
+ if (found == false)
+ return _error->Errno("stat", "Stat failed for %s", file.c_str());
TotalSize += Buf.st_size;
- }
+ }
off_t CurrentSize = 0;
unsigned int NotFound = 0;
@@ -817,44 +863,10 @@
}
else
{
- FileFd From(*I + ".gz",FileFd::ReadOnly);
- if (_error->PendingError() == true)
- return false;
- FileSize = From.Size();
-
- // Get a temp file
- FILE *tmp = tmpfile();
- if (tmp == 0)
- return _error->Errno("tmpfile","Unable to create a tmp file");
- Pkg.Fd(dup(fileno(tmp)));
- fclose(tmp);
-
- // Fork gzip
- pid_t Process = fork();
- if (Process < 0)
- return _error->Errno("fork","Couldn't fork gzip");
-
- // The child
- if (Process == 0)
- {
- dup2(From.Fd(),STDIN_FILENO);
- dup2(Pkg.Fd(),STDOUT_FILENO);
- SetCloseExec(STDIN_FILENO,false);
- SetCloseExec(STDOUT_FILENO,false);
-
- const char *Args[3];
- string Tmp = _config->Find("Dir::bin::gzip","gzip");
- Args[0] = Tmp.c_str();
- Args[1] = "-d";
- Args[2] = 0;
- execvp(Args[0],(char **)Args);
- exit(100);
- }
-
- // Wait for gzip to finish
- if (ExecWait(Process,_config->Find("Dir::bin::gzip","gzip").c_str(),false) == false)
- return _error->Error("gzip failed, perhaps the disk is full.");
-
+ int fd;
+ if (!DecompressFile(*I, &fd, &FileSize))
+ return _error->Errno("decompress","Decompress failed for %s", (*I).c_str());
+ Pkg.Fd(dup(fd));
Pkg.Seek(0);
}
pkgTagFile Parser(&Pkg);
Reply to: