Issue 5411: Add xz support to shutil (original) (raw)
Created on 2009-03-03 11:56 by proyvind, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Messages (34)
Author: Per Øyvind Karlsen (proyvind)
Date: 2009-03-03 11:56
Here's a patch that adds support for xz compression: http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/python/current/SOURCES/Python-2.6.1-distutils-xz-support.patch?view=log
Author: Tarek Ziadé (tarek) * 
Date: 2009-03-07 00:56
Good idea !
are you able provide a unit test for the changes made into the two commands ?
Author: Per Øyvind Karlsen (proyvind)
Date: 2009-03-09 20:12
hmm, I'm unsure about how this should be done..
I guess such a test would belong in Lib/distutils/test_dist.py, but I'm uncertain about how it should be done, ie. should it be a test for doing 'bdist', 'bdist_rpm' and 'sdist' for each of the formats supported? I cannot seem to find any tests for the currently supported formats and such tests would introduce dependencies on the tools used to compress with these formats..
Author: Tarek Ziadé (tarek) *
Date: 2009-03-09 21:07
I guess such a test would belong in Lib/distutils/test_dist.py
no, rather in test_bdist_rpm, test_sdist and so on, but I can do it, it'll just take more time.
Author: Éric Araujo (eric.araujo) *
Date: 2010-07-21 12:06
distutils2 uses tarfile now instead of external programs. Adding dependency on another bug.
Author: Éric Araujo (eric.araujo) *
Date: 2010-09-07 15:38
FTR, the distutils2 repo is http://bitbucket.org/tarek/distutils2. distutils2.tests.support contains helper to create temp directories and run commands; see docstrings and example uses in the tests for more info.
Author: Éric Araujo (eric.araujo) *
Date: 2011-11-30 15:12
distutils2/packaging now just uses the shutil functions. I’ll make a patch for shutil after tarfile is updated.
Author: Éric Araujo (eric.araujo) *
Date: 2012-02-06 17:11
This not-so-bad patch adds lzma compression support to the shutil functions, under the 'xztar' name. Please review.
Author: Éric Araujo (eric.araujo) *
Date: 2012-02-06 17:16
I should add that a doc update will be part of the next patch.
In parallel, I’ve also started updating packaging to add lzma support to the bdist command, but the situation is very disappointing: tarfile knows what format it supports, shutil has its own list, and packaging has a few duplicates too! Ideally, tarfile should have an attribute to expose what compression formats are available, then shutil would reuse that, and packaging would just call shutil. (I’m not even talking about the duplication in the doc.) If you agree with the general idea, I’ll open reports to a) add the attribute to tarfile b) rework the (luckily internal) registries in shutil to stop being hard-coded c) remove packaging’s registries.
Author: Nadeem Vawda (nadeem.vawda) *
Date: 2012-02-11 18:28
This not-so-bad patch adds lzma compression support to the shutil functions, under the 'xztar' name. Please review.
Functionally, the patch looks good to me. There are some docstrings that should probably be updated, though:
- _make_tarball (the 'compress' argument)
- make_archive
- _unpack_tarfile
- unpack_archive
Ideally, tarfile should have an attribute to expose what compression formats are available, then shutil would reuse that, and packaging would just call shutil.
+1
Author: Éric Araujo (eric.araujo) *
Date: 2012-02-14 16:34
If Lars agrees to #14013, I will withdraw this patch in favor of another one that would make shutil automatically support all compressors that tarfile supports (my b) point).
Note that this is not going to be pain-free, as for example bzip2 compression is obtained with the strings “bz2”, “bzip2”, “bzip” or “bztar” depending on the modules; I think it’s worth it nonetheless, even if I have to add an ugly conversion in shutil for compat.
Author: Éric Araujo (eric.araujo) *
Date: 2012-02-21 01:24
I have a working updated shutil module, tests pass and the documentation is improved. I will make sure to make different commits for improving the tests, cleaning up some things, adding tarfile.compression_formats and removing duplication in shutil (see dep bugs), to be sure not to introduce regressions.
Before I finish and post the patch, I’d like feedback on a choice I made. I don’t think it’s possible to have shutil automatically support all the formats that tarfile does, because of the spelling issue I mentioned. Here’s what the code would look like (let me know if I should post it elsewhere or as a file to let you get syntax coloring):
_ARCHIVE_FORMATS = {} _UNPACK_FORMATS = {}
for fmt in tarfile.compression_formats: code = fmt + 'tar' ext = '.' + fmt desc = "%s'ed tar file" % fmt _ARCHIVE_FORMATS[code] = (_make_tarball, [('compress', fmt)], desc) _UNPACK_FORMATS[code] = ([ext], _unpack_tarfile, [], desc)
kludge to add alternate extension
if 'gztar' in _ARCHIVE_FORMATS: _UNPACK_FORMATS['gztar'][0].append('.tgz') # XXX desc should say "gzip'ed tar file", not "gz'ed"
rectify naming incompatibility
if 'bz2tar' in _ARCHIVE_FORMATS: # XXX alternative: make 'bztar' alias for 'bz2tar', but would complicate # manipulating the registries del _ARCHIVE_FORMATS['bz2tar'] del _UNPACK_FORMATS['bz2tar'] desc = "bzip2'ed tar file" _ARCHIVE_FORMATS['bztar'] = (_make_tarball, [('compress', 'bz2')], desc) _UNPACK_FORMATS['bztar'] = (['.bz2'], _unpack_tarfile, [], desc)
now add uncompressed tar and zip file
I really don’t like that code. Given that tarfile is not extensible at run time, it is not a big deal to have to update shutil whenever we add a compression format to tarfile. Therefore, I backtracked on my “automatic support” idea but kept a lot of cleanup in did in the code. Here’s what the code looks like:
_ARCHIVE_FORMATS = {} _UNPACK_FORMATS = {}
if 'xz' in tarfile.compression_formats: desc = "xz'ed tar file" # XXX '.xz' is not great, '.tar.xz' would be better _ARCHIVE_FORMATS['xztar'] = (_make_tarball, [('compress', 'xz')], desc) _UNPACK_FORMATS['xztar'] = (['.xz'], _unpack_tarfile, [], desc)
if 'bz2' in tarfile.compression_formats: desc = "bzip2'ed tar file" _ARCHIVE_FORMATS['bztar'] = (_make_tarball, [('compress', 'bz2')], desc) _UNPACK_FORMATS['bztar'] = (['.bz2'], _unpack_tarfile, [], desc)
if 'gz' in tarfile.compression_formats: desc = "gzip'ed tar file" _ARCHIVE_FORMATS['gztar'] = (_make_tarball, [('compress', 'gz')], desc) _UNPACK_FORMATS['gztar'] = (['.gz', '.tgz'], _unpack_tarfile, [], desc)
So, do you agree that “not automated but not ugly” is better than “automated with ugly klutches”?
Author: Éric Araujo (eric.araujo) *
Date: 2012-02-21 01:26
s/cleanup in did in the code/cleanup I did in the code/
Author: Éric Araujo (eric.araujo) *
Date: 2012-02-21 02:27
Note that there is a way to get fully automated support for tar formats: tarfile could expose, in addition to the list compression_formats, another structure with the descriptions (e.g. “gzip’ed tar file”) and file extensions (e.g. ['.gz', '.tgz'] —no, it’s not '.tar.gz', which is unfortunate, and could cause Lars to reject that idea). I’m just putting this here for reference, but my preference is still for the second idea I talk about in my precedent message.
Author: Nadeem Vawda (nadeem.vawda) *
Date: 2012-02-22 09:04
So, do you agree that “not automated but not ugly” is better than “automated with ugly klutches”?
Definitely. If we have to add special cases that are almost as long as the original code, the "automation" seems pointless.
Note that there is a way to get fully automated support for tar formats: tarfile could expose, in addition to the list compression_formats, another structure with the descriptions (e.g. “gzip’ed tar file”) and file extensions (e.g. ['.gz', '.tgz'] —no, it’s not '.tar.gz', which is unfortunate, and could cause Lars to reject that idea). I’m just putting this here for reference, but my preference is still for the second idea I talk about in my precedent message.
As much as it would be nice to have all of the information centralized in one place, I don't think this is practical - some of the data that would be stored in this structure really is specific to shutil, so I agree that your second option is better.
I think we can restructure the code a bit to reduce the work involved in adding a new format, though. Maybe something like this:
_ARCHIVE_FORMATS = {}
_UNPACK_FORMATS = {}
for name, tarname, progname, extensions, desc in [
("xz", "xztar", "xz", [".xz"], "xz'ed tar file"),
("bz2", "bztar", "bzip2", [".bz2"], "bzip2'ed tar file"),
("gz", "gztar", "gzip", [".gz", ".tgz"], "gzip'ed tar file")]:
if name in tarfile.compression_formats:
_ARCHIVE_FORMATS[tarname] = _make_tarball, [("compress", progname)], desc
_UNPACK_FORMATS[tarname] = extensions, _unpack_tarfile, [], descBy the way, is there any reason why you've used ".gz" instead of ".tar.gz" as the extension for "gztar" in your proposed change? The current version of shutil.py uses ".tar.gz", but has ".bz2" for "bztar".
Also, I notice that _make_tarball() will need to be updated to add the info for xz to the tar_compression and compress_ext dictionaries. It seems a bit ugly to duplicate this information when it's already present in the format dicts, so maybe it would be better to pass the file extension in directly instead? I assume that _make_tarball() is not part of shutil's public API.
If we did this, there would be no need for a separate "progname" field in the initialization code I suggested above:
for name, tarname, extensions, desc in [
("xz", "xztar", [".xz"], "xz'ed tar file"),
("bz2", "bztar", [".bz2"], "bzip2'ed tar file"),
("gz", "gztar", [".gz", ".tgz"], "gzip'ed tar file")]:
if name in tarfile.compression_formats:
_ARCHIVE_FORMATS[tarname] = _make_tarball, [("compress", name)], desc
_UNPACK_FORMATS[tarname] = extensions, _unpack_tarfile, [], descAuthor: Nadeem Vawda (nadeem.vawda) *
Date: 2012-02-22 09:10
For the "xztar" format, you should also perhaps recognize the ".txz" extension - I've seen this used by FreeBSD.
Author: Éric Araujo (eric.araujo) *
Date: 2012-02-23 01:22
I think we can restructure the code a bit to reduce the work involved in adding a new format, though. Maybe something like this: [snip] Thanks, I’ve applied your obvious refactor in my clone :) For clarity, I will split my patch into many patches for different reports (bug fixes and doc improvements for 3.2, then code cleanup in 3.3, then adding xz).
By the way, is there any reason why you've used ".gz" instead of ".tar.gz" as the extension for "gztar" in your proposed change? The current version of shutil.py uses ".tar.gz", but has ".bz2" for "bztar". Yes, I confused the two formats. I reported the '.bz2' bug (should be '.tar.bz2') and will fix it.
Also, I notice that _make_tarball() will need to be updated to add the info for xz to the tar_compression and compress_ext dictionaries. I ripped them off. I could have pasted my full patch to save you time, sorry!
For the "xztar" format, you should also perhaps recognize the ".txz" extension Done. Google finds a number of matches. I won’t add '.tbz' though, as I don’t know if it’s bzip or bzip2, and as it does not seem used much.
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2012-10-26 10:26
I have not seen this issue and was created a new with almost same patch as Eric's one (only I have changed the documentation too). Here's the patch. I wonder that it was not committed yet to 3.3. I think it would be better first to add xz support and the code cleanup to do then (if it takes so much time).
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2012-10-26 10:45
Oh, I see the '.bz2' bug. Patch updated.
Author: Hynek Schlawack (hynek) *
Date: 2012-11-02 06:55
Éric, what’s your take on this approach (not code)? We have time enough till 3.4 but it seems this doesn't really move forward. Any thoughts how to get this moving? Unfortunately I'm not invested enough in this to make a educated decision.
Author: Éric Araujo (eric.araujo) *
Date: 2012-11-03 21:54
I will upload my patch and compare it with Serhiy’s. Now that 3.3.0 is released, there is no hurry.
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2013-10-15 06:23
3.4 beta 1 will be soon.
Author: Antoine Pitrou (pitrou) *
Date: 2013-10-15 10:22
Serhiy's patch needs a "versionchanged" or "versionadded" tag in the Docs.
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2013-10-18 06:41
Serhiy's patch needs a "versionchanged" or "versionadded" tag in the Docs.
Done.
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2014-08-01 07:37
Now 3.4 is released, but shutil still doesn't support the xz compression. I think we should hurry on to be in time for 3.5.
Author: Antoine Pitrou (pitrou) *
Date: 2014-08-04 14:09
I think we should hurry on to be in time for 3.5.
Your patch looks ok to me. Just update the version number in the docs ;)
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2014-08-04 20:39
Éric?
Author: Éric Araujo (eric.araujo) *
Date: 2014-08-04 22:36
I’m afraid I changed computers once or twice since I worked on that, so I can’t readily find my clone and get the latest patch. I know where the hard drives are but I can’t say when I will have time to search them.
Author: Roundup Robot (python-dev) 
Date: 2014-08-06 15:51
New changeset e57db221b6c4 by Serhiy Storchaka in branch 'default': Issue #5411: Added support for the "xztar" format in the shutil module. http://hg.python.org/cpython/rev/e57db221b6c4
Author: Antoine Pitrou (pitrou) *
Date: 2014-08-06 15:53
Thank you Serhiy! You need to fix the version number in the "versionchanged".
Author: Roundup Robot (python-dev)
Date: 2014-08-06 15:56
New changeset a47996c10579 by Serhiy Storchaka in branch 'default': Issue #5411: Fixed version number. http://hg.python.org/cpython/rev/a47996c10579
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2014-08-06 15:58
Oh, my fault. Thank you Antoine!
Author: Akira Li (akira) *
Date: 2014-08-11 10:48
sphinx generates warning for the current docs introduced by this issue:
WARNING: Explicit markup ends without a blank line; unexpected unindent.
I've uploaded a documentation patch that fixes it.
Author: Berker Peksag (berker.peksag) *
Date: 2014-08-11 15:11
Fixed. Thanks for the patch, Akira.
http://hg.python.org/cpython/rev/7fcfb634ccca
History
Date
User
Action
Args
2022-04-11 14:56:46
admin
set
github: 49661
2014-08-11 15:11:21
berker.peksag
set
nosy: + berker.peksag
messages: +
2014-08-11 10:48:27
akira
set
files: + docs-fix-sphinx-warning.patch
nosy: + akira
messages: +
2014-08-06 15:58:54
serhiy.storchaka
set
status: open -> closed
resolution: fixed
messages: +
stage: patch review -> resolved
2014-08-06 15:56:24
python-dev
set
messages: +
2014-08-06 15:53:11
pitrou
set
messages: +
2014-08-06 15:51:10
python-dev
set
nosy: + python-dev
messages: +
2014-08-06 15:44:46
serhiy.storchaka
set
assignee: serhiy.storchaka
dependencies: - Misc fixes and cleanups in archiving code in shutil and test_shutil
2014-08-04 22:36:41
eric.araujo
set
assignee: eric.araujo -> (no value)
messages: +
2014-08-04 20:39:15
serhiy.storchaka
set
messages: +
2014-08-04 14:09:13
pitrou
set
messages: +
2014-08-01 07:37:57
serhiy.storchaka
set
messages: +
versions: + Python 3.5, - Python 3.4
2013-10-18 20:39:33
Arfrever
set
nosy: + Arfrever
2013-10-18 06:41:32
serhiy.storchaka
set
files: + shutil-lzma_3.patch
messages: +
2013-10-18 06:40:18
serhiy.storchaka
set
files: - shutil-lzma_2.patch
2013-10-15 10:22:00
pitrou
set
nosy: + pitrou
messages: +
2013-10-15 06:23:38
serhiy.storchaka
set
messages: +
2012-11-03 21:54:56
eric.araujo
set
messages: +
2012-11-02 06:55:57
hynek
set
messages: +
2012-10-26 10:45:23
serhiy.storchaka
set
files: + shutil-lzma_2.patch
messages: +
2012-10-26 10:44:09
serhiy.storchaka
set
files: - shutil-lzma.patch
2012-10-26 10:27:01
serhiy.storchaka
set
stage: needs patch -> patch review
2012-10-26 10:26:39
serhiy.storchaka
set
files: + shutil-lzma.patch
versions: + Python 3.4, - Python 3.3
nosy: + serhiy.storchaka
messages: +
2012-10-26 10:15:26
serhiy.storchaka
link
2012-02-23 01:22:05
eric.araujo
set
dependencies: - tarfile should expose supported formats
messages: +
2012-02-22 09:10:13
nadeem.vawda
set
messages: +
2012-02-22 09:04:13
nadeem.vawda
set
messages: +
2012-02-22 02:14:03
pitrou
set
nosy: + hynek
2012-02-21 02:27:39
eric.araujo
set
messages: +
2012-02-21 01:26:18
eric.araujo
set
messages: +
2012-02-21 01:24:35
eric.araujo
set
messages: +
title: shutil should support all formats supported by tarfile automatically -> Add xz support to shutil
2012-02-20 02:35:56
eric.araujo
set
dependencies: + Misc fixes and cleanups in archiving code in shutil and test_shutil
2012-02-20 02:33:56
eric.araujo
set
files: - shutil-xz.diff
2012-02-20 02:33:48
eric.araujo
set
title: add xz compression support to shutil -> shutil should support all formats supported by tarfile automatically
2012-02-14 16:34:22
eric.araujo
set
dependencies: + tarfile should expose supported formats
messages: +
2012-02-11 18:28:32
nadeem.vawda
set
messages: +
2012-02-06 17:16:11
eric.araujo
set
messages: +
2012-02-06 17:11:38
eric.araujo
set
files: + shutil-xz.diff
messages: +
2011-11-30 15:19:07
nadeem.vawda
set
nosy: + nadeem.vawda
2011-11-30 15:12:08
eric.araujo
set
versions: + Python 3.3, - 3rd party
title: add xz compression support to distutils -> add xz compression support to shutil
messages: +
assignee: tarek -> eric.araujo
components: + Library (Lib), - Distutils2
2010-09-29 23:58:50
eric.araujo
set
versions: + 3rd party, - Python 2.6, Python 2.5, Python 3.1, Python 2.7, Python 3.2
2010-09-07 15:38:35
eric.araujo
set
messages: +
2010-07-21 12:06:34
eric.araujo
set
dependencies: + Support xz compression in tarfile module
messages: +
stage: test needed -> needs patch
versions: + Python 2.6, Python 2.5, Python 3.2
2010-04-09 07:44:48
tarek
set
nosy:tarek, eric.araujo, proyvind
components: + Distutils2, - Distutils
2010-04-09 01:39:23
eric.araujo
set
nosy: + eric.araujo
2009-03-09 21:07:06
tarek
set
messages: +
2009-03-09 20:12:58
proyvind
set
messages: +
2009-03-07 00:56:06
tarek
set
priority: normal
keywords: + patch
versions: + Python 3.1, Python 2.7, - Python 2.6
messages: +
stage: test needed
2009-03-03 11:56:24
proyvind
create