classification
Title: Add xz support to shutil
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: 5689 Superseder:
Assigned To: serhiy.storchaka Nosy List: Arfrever, akira, berker.peksag, eric.araujo, hynek, nadeem.vawda, pitrou, proyvind, python-dev, serhiy.storchaka, tarek
Priority: normal Keywords: patch

Created on 2009-03-03 11:56 by proyvind, last changed 2014-08-11 15:11 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
shutil-lzma_3.patch serhiy.storchaka, 2013-10-18 06:41 review
docs-fix-sphinx-warning.patch akira, 2014-08-11 10:48 fix sphinx's whitespace/indentation warning review
Messages (34)
msg83072 - (view) 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
msg83263 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-03-07 00:56
Good idea !

are you able provide a unit test for the changes made into the two
commands ?
msg83401 - (view) 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..
msg83404 - (view) Author: Tarek Ziadé (tarek) * (Python committer) 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.
msg111038 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-07-21 12:06
distutils2 uses tarfile now instead of external programs. Adding dependency on another bug.
msg115780 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg148660 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg152748 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg152749 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg153131 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) 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
msg153351 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-14 16:34
a) is #14013; b) is #14011.

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.
msg153831 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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”?
msg153832 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-21 01:26
s/cleanup in did in the code/cleanup I did in the code/
msg153834 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg153937 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) 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, [], desc

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".


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, [], desc
msg153938 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) 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.
msg154029 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg173831 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-26 10:26
I have not seen this issue and was created a new issue16313 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).
msg173833 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-26 10:45
Oh, I see the '.bz2' bug. Patch updated.
msg174491 - (view) Author: Hynek Schlawack (hynek) * (Python committer) 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.
msg174707 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg199977 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-10-15 06:23
3.4 beta 1 will be soon.
msg199987 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-10-15 10:22
Serhiy's patch needs a "versionchanged" or "versionadded" tag in the Docs.
msg200183 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-10-18 06:41
> Serhiy's patch needs a "versionchanged" or "versionadded" tag in the Docs.

Done.
msg224471 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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.
msg224722 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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 ;)
msg224771 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-04 20:39
Éric?
msg224780 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) 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.
msg224943 - (view) 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
msg224944 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-08-06 15:53
Thank you Serhiy! You need to fix the version number in the "versionchanged".
msg224945 - (view) 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
msg224946 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-06 15:58
Oh, my fault. Thank you Antoine!
msg225181 - (view) 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.
msg225193 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-08-11 15:11
Fixed. Thanks for the patch, Akira.

http://hg.python.org/cpython/rev/7fcfb634ccca
History
Date User Action Args
2014-08-11 15:11:21berker.peksagsetnosy: + berker.peksag
messages: + msg225193
2014-08-11 10:48:27akirasetfiles: + docs-fix-sphinx-warning.patch
nosy: + akira
messages: + msg225181

2014-08-06 15:58:54serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg224946

stage: patch review -> resolved
2014-08-06 15:56:24python-devsetmessages: + msg224945
2014-08-06 15:53:11pitrousetmessages: + msg224944
2014-08-06 15:51:10python-devsetnosy: + python-dev
messages: + msg224943
2014-08-06 15:44:46serhiy.storchakasetassignee: serhiy.storchaka
dependencies: - Misc fixes and cleanups in archiving code in shutil and test_shutil
2014-08-04 22:36:41eric.araujosetassignee: eric.araujo -> (no value)
messages: + msg224780
2014-08-04 20:39:15serhiy.storchakasetmessages: + msg224771
2014-08-04 14:09:13pitrousetmessages: + msg224722
2014-08-01 07:37:57serhiy.storchakasetmessages: + msg224471
versions: + Python 3.5, - Python 3.4
2013-10-18 20:39:33Arfreversetnosy: + Arfrever
2013-10-18 06:41:32serhiy.storchakasetfiles: + shutil-lzma_3.patch

messages: + msg200183
2013-10-18 06:40:18serhiy.storchakasetfiles: - shutil-lzma_2.patch
2013-10-15 10:22:00pitrousetnosy: + pitrou
messages: + msg199987
2013-10-15 06:23:38serhiy.storchakasetmessages: + msg199977
2012-11-03 21:54:56eric.araujosetmessages: + msg174707
2012-11-02 06:55:57hyneksetmessages: + msg174491
2012-10-26 10:45:23serhiy.storchakasetfiles: + shutil-lzma_2.patch

messages: + msg173833
2012-10-26 10:44:09serhiy.storchakasetfiles: - shutil-lzma.patch
2012-10-26 10:27:01serhiy.storchakasetstage: needs patch -> patch review
2012-10-26 10:26:39serhiy.storchakasetfiles: + shutil-lzma.patch
versions: + Python 3.4, - Python 3.3
nosy: + serhiy.storchaka

messages: + msg173831
2012-10-26 10:15:26serhiy.storchakalinkissue16313 superseder
2012-02-23 01:22:05eric.araujosetdependencies: - tarfile should expose supported formats
messages: + msg154029
2012-02-22 09:10:13nadeem.vawdasetmessages: + msg153938
2012-02-22 09:04:13nadeem.vawdasetmessages: + msg153937
2012-02-22 02:14:03pitrousetnosy: + hynek
2012-02-21 02:27:39eric.araujosetmessages: + msg153834
2012-02-21 01:26:18eric.araujosetmessages: + msg153832
2012-02-21 01:24:35eric.araujosetmessages: + msg153831
title: shutil should support all formats supported by tarfile automatically -> Add xz support to shutil
2012-02-20 02:35:56eric.araujosetdependencies: + Misc fixes and cleanups in archiving code in shutil and test_shutil
2012-02-20 02:33:56eric.araujosetfiles: - shutil-xz.diff
2012-02-20 02:33:48eric.araujosettitle: add xz compression support to shutil -> shutil should support all formats supported by tarfile automatically
2012-02-14 16:34:22eric.araujosetdependencies: + tarfile should expose supported formats
messages: + msg153351
2012-02-11 18:28:32nadeem.vawdasetmessages: + msg153131
2012-02-06 17:16:11eric.araujosetmessages: + msg152749
2012-02-06 17:11:38eric.araujosetfiles: + shutil-xz.diff

messages: + msg152748
2011-11-30 15:19:07nadeem.vawdasetnosy: + nadeem.vawda
2011-11-30 15:12:08eric.araujosetversions: + Python 3.3, - 3rd party
title: add xz compression support to distutils -> add xz compression support to shutil
messages: + msg148660

assignee: tarek -> eric.araujo
components: + Library (Lib), - Distutils2
2010-09-29 23:58:50eric.araujosetversions: + 3rd party, - Python 2.6, Python 2.5, Python 3.1, Python 2.7, Python 3.2
2010-09-07 15:38:35eric.araujosetmessages: + msg115780
2010-07-21 12:06:34eric.araujosetdependencies: + Support xz compression in tarfile module
messages: + msg111038
stage: test needed -> needs patch
versions: + Python 2.6, Python 2.5, Python 3.2
2010-04-09 07:44:48tareksetnosy: tarek, eric.araujo, proyvind
components: + Distutils2, - Distutils
2010-04-09 01:39:23eric.araujosetnosy: + eric.araujo
2009-03-09 21:07:06tareksetmessages: + msg83404
2009-03-09 20:12:58proyvindsetmessages: + msg83401
2009-03-07 00:56:06tareksetpriority: normal
keywords: + patch
versions: + Python 3.1, Python 2.7, - Python 2.6
messages: + msg83263
stage: test needed
2009-03-03 11:56:24proyvindcreate