GNOME Bugzilla – Bug 499497
Replace the tar/bzip2/gzip/unzip calls for native python code
Last modified: 2007-12-15 20:56:05 UTC
Jhbuild uses shell commands to unpack tarballs and zipfiles and python includes a tarfile and a zipfile module for extraction already. Using the python modules would make the code more portable and less dependant of other tools.
Created attachment 99607 [details] [review] Replaces bunzip2/gunzip/tar/unzip calls for native python code. This patch fixes the issue adding a utils/unpack.py module. It also uses tarfile and zipfile helpers to discover the filetype instead of checking the file name suffix. Note that the symlink creation is not implemented for the unpack_zip_file method yet, but it's not very common for the kind of packages that we're using so I don't think it's critical.
I am not totally opposed to this but what were *in practice* the problems with the current code ? There is actually a ton of code to deal with broken archives in the different tools, I am not sure the Python modules are on par with this. There is also the matter of how a user could follow the shell commands printed by jhbuild and reproduce them out later. If there are real problems regarding portability of the current method, I'd like them to be exposed (notably since JHBuild calls out to shell commands a lot).
I've been thinking on this for a while, and I don't see that much risk, actually, the code is more simple, and the tarball module of python is pretty mature. I can see some risks on the zipfile support, but we don't use zipfiles that often on modulesets so the risk is very low there. Plus, setting up an msys environment on windows with bzip2, and all the stuff is not that pleasant, so it's not that standard, and I'm trying to make the windows development experience a little bit better for gtk and stuff like that. If anything we could move the bash calls, to unpack.py, and add an environment variable check, such as 'USE_PYTHON_IMPL', so the developer can choose. I have plans to keep adding support for python implementations to replace the rest of the shell calls, and I would really like to put this effort into mainstream.
Adding a jhbuild user configurable option like USE_PYTHON_IMPL allows trying to reduce dependencies on win32 while not changing the current code paths. Testing in unpack.py whether the option is set (or defined) to use a pure python implementation or the current one would not make the current code more difficult to read or maintain, it even abstracts it a bit.
Abstracting things will not automatically yield more maintainable code... Anyway, even though I am not in favour of this change (and even less so in the idea of replacing more shell calls (as I said I find it useful to see commands that are executed, to replay them by hand later)), let's go with this patch. Two comments: - move import statements to the top; - create a single function, unpack_file, so the tarball.py are really simplified.
Created attachment 100733 [details] [review] Updated patch to reflect changes Works fine with my testing. Also works on Win32
2007-12-15 Frederic Peters <fpeters@0d.be> * jhbuild/modtypes/tarball.py, jhbuild/utils/unpack.py, jhbuild/versioncontrol/tarball.py: use native Python modules to unpack archives when standard tools (tar, gunzip, bunzip2, unzip) are not available; patch by Alberto Ruiz, updated by John Stowers and further edited by myself. (closes: #499497)