Bug#649451: hard-coded gzip-only support in apt-cdrom
On Wed, Nov 23, 2011 at 05:20:36PM +0100, Michael Vogt wrote:
>On Wed, Nov 23, 2011 at 04:09:23PM +0000, Steve McIntyre wrote:
>> On Wed, Nov 23, 2011 at 05:01:13PM +0100, Michael Vogt wrote:
>> >On Mon, Nov 21, 2011 at 06:57:41PM +0100, Julian Andres Klode wrote:
>> >> On Mon, Nov 21, 2011 at 01:03:08PM +0100, David Kalnischkies wrote:
>> >> > On Mon, Nov 21, 2011 at 01:26, Steve McIntyre <steve@einval.com> wrote:
>> >> > > I've just added support for translated description files into
>[..]
>> >> It would be great if someone who actually needs it takes a look
>> >> at it. That said, I have pushed out the following patch to
>> >> the debian-experimental2 branch, please test it (I didn't
>> >> test it, but it compiles and looks logically right).
>> >[..]
>> >
>> >Thanks! This looks fine, I still changed it a bit to make use of the
>> >APT::Configuration::getCompressionTypes() code so that we have only a
>> >single place to add new compression types. Needs some serious testing
>> >still.
>>
>> My own testing suggests it's necessary but not sufficient. The code in
>> indexcopy.cc needs updating too to support things other than
>> uncompressed and gzipped files. I've got a grotty patch that works for
>> me but you *will* want to refactor. :-)
>
>Oh, indeed, that code needs some serious love in general. But please
>send us the patch :)
I'm cleaning it up slightly to apply against current debian-sid right
now, but here is the diff against 0.8.10.
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...
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.
--
Steve McIntyre, Cambridge, UK. steve@einval.com
< sladen> I actually stayed in a hotel and arrived to find a post-it
note stuck to the mini-bar saying "Paul: This fridge and
fittings are the correct way around and do not need altering"
diff -uNrBb apt-0.8.10.old/apt-pkg//cdrom.cc apt-0.8.10/apt-pkg//cdrom.cc
--- apt-0.8.10.old/apt-pkg//cdrom.cc 2010-08-24 16:58:13.000000000 +0100
+++ apt-0.8.10/apt-pkg//cdrom.cc 2011-11-23 02:09:11.055019767 +0000
@@ -78,7 +78,8 @@
/* 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 */
- if (stat("Packages",&Buf) == 0 || stat("Packages.gz",&Buf) == 0)
+ if (stat("Packages",&Buf) == 0 || stat("Packages.gz",&Buf) == 0 ||
+ stat("Packages.bz2",&Buf) == 0 || stat("Packages.xz",&Buf) == 0)
{
List.push_back(CD);
@@ -86,7 +87,8 @@
if (_config->FindB("APT::CDROM::Thorough",false) == false)
return true;
}
- if (stat("Sources.gz",&Buf) == 0 || stat("Sources",&Buf) == 0)
+ if (stat("Packages",&Buf) == 0 || stat("Packages.gz",&Buf) == 0 ||
+ stat("Packages.bz2",&Buf) == 0 || stat("Packages.xz",&Buf) == 0)
{
SList.push_back(CD);
@@ -95,7 +97,7 @@
return true;
}
- // see if we find translatin indexes
+ // see if we find translatin indices
if (stat("i18n",&Buf) == 0)
{
D = opendir("i18n");
@@ -106,8 +108,11 @@
if (_config->FindB("Debug::aptcdrom",false) == true)
std::clog << "found translations: " << Dir->d_name << "\n";
string file = Dir->d_name;
- if(file.substr(file.size()-3,file.size()) == ".gz")
+ if(file.substr(file.size()-3,file.size()) == ".gz" ||
+ file.substr(file.size()-3,file.size()) == ".xz")
file = file.substr(0,file.size()-3);
+ if(file.substr(file.size()-4,file.size()) == ".bz2")
+ file = file.substr(0,file.size()-4);
TransList.push_back(CD+"i18n/"+ file);
}
}
@@ -251,7 +256,9 @@
{
struct stat Buf;
if (stat((List[I] + Name).c_str(),&Buf) != 0 &&
- stat((List[I] + Name + ".gz").c_str(),&Buf) != 0)
+ stat((List[I] + Name + ".gz").c_str(),&Buf) != 0 &&
+ stat((List[I] + Name + ".bz2").c_str(),&Buf) != 0 &&
+ stat((List[I] + Name + ".xz").c_str(),&Buf) != 0)
_error->Errno("stat","Failed to stat %s%s",List[I].c_str(),
Name);
Inodes[I] = Buf.st_ino;
diff -uNrBb apt-0.8.10.old/apt-pkg//indexcopy.cc apt-0.8.10/apt-pkg//indexcopy.cc
--- apt-0.8.10.old/apt-pkg//indexcopy.cc 2010-09-07 14:19:57.000000000 +0100
+++ apt-0.8.10/apt-pkg//indexcopy.cc 2011-11-23 04:01:56.038989999 +0000
@@ -34,7 +34,83 @@
using namespace std;
+ /*}}}*/
+// DecompressFile - wrapper for decompressing gzip/bzip2/xz compressed files /*{{{*/
+// ---------------------------------------------------------------------
+/* */
+bool DecompressFile(string Filename, int *fd, unsigned long *FileSize)
+{
+ string CompressProg;
+ string CompressProgFind;
+ FileFd From;
+ struct stat Buf;
+ *fd = -1;
+
+ if (stat((Filename + ".gz").c_str(), &Buf) == 0)
+ {
+ CompressProg = "gzip";
+ CompressProgFind = "Dir::bin::gzip";
+ From.Open(Filename + ".gz",FileFd::ReadOnly);
+ }
+ else if (stat((Filename + ".bz2").c_str(), &Buf) == 0)
+ {
+ CompressProg = "bzip2";
+ CompressProgFind = "Dir::bin::bzip2";
+ From.Open(Filename + ".bz2",FileFd::ReadOnly);
+ }
+ else if (stat((Filename + ".xz").c_str(), &Buf) == 0)
+ {
+ CompressProg = "xz";
+ CompressProgFind = "Dir::bin::xz";
+ From.Open(Filename + ".xz",FileFd::ReadOnly);
+ }
+ else
+ {
+ return _error->Errno("decompressor", "Unable to parse file");
+ }
+
+ if (_error->PendingError() == true)
+ return -1;
+
+ *FileSize = Buf.st_size;
+
+ // Get a temp file
+ FILE *tmp = tmpfile();
+ if (tmp == 0)
+ return _error->Errno("tmpfile","Unable to create a tmp file");
+ *fd = dup(fileno(tmp));
+ fclose(tmp);
+
+ // Fork decompressor
+ pid_t Process = fork();
+ if (Process < 0)
+ return _error->Errno("fork","Couldn't fork to run decompressor");
+
+ // The child
+ if (Process == 0)
+ {
+ dup2(From.Fd(),STDIN_FILENO);
+ dup2(*fd,STDOUT_FILENO);
+ SetCloseExec(STDIN_FILENO,false);
+ SetCloseExec(STDOUT_FILENO,false);
+
+ const char *Args[3];
+ string Tmp = _config->Find(CompressProgFind, CompressProg);
+ Args[0] = Tmp.c_str();
+ Args[1] = "-d";
+ Args[2] = 0;
+ if(execvp(Args[0],(char **)Args))
+ return(_error->Errno("decompressor","decompress failed"));
+ /* Should never get here */
+ exit(100);
+ }
+ // Wait for decompress to finish
+ if (ExecWait(Process,CompressProg.c_str(),false) == false)
+ return false;
+
+ return true;
+}
// IndexCopy::CopyPackages - Copy the package files from the CD /*{{{*/
// ---------------------------------------------------------------------
@@ -58,7 +134,9 @@
{
struct stat Buf;
if (stat(string(*I + GetFileName()).c_str(),&Buf) != 0 &&
- stat(string(*I + GetFileName() + ".gz").c_str(),&Buf) != 0)
+ stat(string(*I + GetFileName() + ".gz").c_str(),&Buf) != 0 &&
+ stat(string(*I + GetFileName() + ".xz").c_str(),&Buf) != 0 &&
+ stat(string(*I + GetFileName() + ".bz2").c_str(),&Buf) != 0)
return _error->Errno("stat","Stat failed for %s",
string(*I + GetFileName()).c_str());
TotalSize += Buf.st_size;
@@ -82,46 +160,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;
@@ -769,8 +816,11 @@
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)
+ stat(string(*I + ".gz").c_str(),&Buf) != 0 &&
+ stat(string(*I + ".bz2").c_str(),&Buf) != 0 &&
+ stat(string(*I + ".xz").c_str(),&Buf) != 0)
return _error->Errno("stat","Stat failed for %s",
string(*I).c_str());
TotalSize += Buf.st_size;
@@ -794,44 +844,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: