classification
Title: distutils.archive_util should handle absence of zlib module
Type: crash Stage: commit review
Components: Distutils Versions: Python 3.3, Python 3.2, Python 3.1, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: alexis, eric.araujo, ezio.melotti, nessita, python-dev, tarek
Priority: normal Keywords: patch

Created on 2011-03-14 16:19 by nessita, last changed 2012-04-11 21:38 by eric.araujo.

Files
File name Uploaded Description Edit
pycon-issue11501.patch nessita, 2011-03-15 13:58 review
Messages (10)
msg130847 - (view) Author: Natalia B. Bidart (nessita) * Date: 2011-03-14 16:19
When creating a zipfile, the code:

zip = zipfile.ZipFile(zip_filename, "w",
                      compression=zipfile.ZIP_DEFLATED)

does not handle the potential RuntimeError casued by:

"If ZIP_DEFLATED is specified but the zlib module is not available, RuntimeError is also raised."
msg130894 - (view) Author: Natalia B. Bidart (nessita) * Date: 2011-03-14 20:46
Attaching patch that performs the following:

* skip all the distutils tests that need zlib in order to run properly.
* add a new test (test_make_zipfile_no_zlib) to ensure that make_zipfile uses the compression option from zipfile module that works without zlib.
* add a helper method to patch objects.

The fix itself was discussed with Tarek, and the patch helper implementation was discussed with Michael Foord.
msg130896 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-03-14 20:55
Thanks.  One question: why not using a clause like “if zlib is not None” instead of catching a RuntimeError?  The if test looks more specific to me, whereas the RuntimeError might come from something else, and thus shouldn’t always e ignored.

In the patch function, I would not use assert but hasattr and a real exception (so that the behavior does not change when run under python -O).
msg130898 - (view) Author: Natalia B. Bidart (nessita) * Date: 2011-03-14 21:08
Hi Éric,

Thanks for looking at the patch. I'm not using "if zlib is not None" since the archive_utils module never explicitly imports zlib. Only the zipfile module imports zlib and raises RuntimeError if not available (as per http://docs.python.org/library/zipfile.html#zipfile-objects).

I'm changing the patch function to raise an exception as requested.
msg130902 - (view) Author: Natalia B. Bidart (nessita) * Date: 2011-03-14 21:16
* Added change notice to Misc/NEWS.
* changed patch method implementation so AttributeError is raised when trying to patch a non-existent attribute for an object (see Éric's comment).
msg130962 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-03-15 11:15
> I'm not using "if zlib is not None" since the archive_utils module
> never explicitly imports zlib.

Well, you can import zlib in that module too to detect in advance whether zipfile will work.
msg130967 - (view) Author: Natalia B. Bidart (nessita) * Date: 2011-03-15 12:40
On Tue, Mar 15, 2011 at 7:15 AM, Éric Araujo <report@bugs.python.org> wrote:
>
> Éric Araujo <merwok@netwok.org> added the comment:
>
>> I'm not using "if zlib is not None" since the archive_utils module
>> never explicitly imports zlib.
>
> Well, you can import zlib in that module too to detect in advance whether zipfile will work.

Indeed, but from my POV that's less cleaner than the proposed
solution: importing a module that is not used (other than checking for
a condition) may generate confusion to a reader. IMHO, the proposed
solution may ease the readability of the code block by being
super-explicit about what condition is being handled when creating the
ZipFile.

Let me know if you still consider I should change that.
msg130973 - (view) Author: Natalia B. Bidart (nessita) * Date: 2011-03-15 13:58
Attaching a new patch with latest changes from trunk merged in (conflicts resolved).
msg131034 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-15 20:17
New changeset 6b33aa811821 by Antoine Pitrou in branch '3.1':
On behalf of Tarek: Issue #11501: disutils.archive_utils.make_zipfile no
http://hg.python.org/cpython/rev/6b33aa811821

New changeset 1bf4383f190a by Antoine Pitrou in branch '3.2':
Merge fix for issue #11501
http://hg.python.org/cpython/rev/1bf4383f190a

New changeset cccddd797d95 by Antoine Pitrou in branch 'default':
Merge fix for issue #11501
http://hg.python.org/cpython/rev/cccddd797d95
msg158079 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-04-11 21:38
Reopening for 2.7.4.
History
Date User Action Args
2012-04-11 21:38:51eric.araujosetstatus: closed -> open
versions: + Python 2.7
messages: + msg158079

assignee: tarek -> eric.araujo
resolution: fixed ->
stage: resolved -> commit review
2011-03-15 20:25:32pitrousetstatus: open -> closed
nosy: tarek, ezio.melotti, eric.araujo, nessita, alexis, python-dev
versions: + Python 3.1, Python 3.2
resolution: fixed
stage: resolved
2011-03-15 20:17:20python-devsetnosy: + python-dev
messages: + msg131034
2011-03-15 19:30:30tareksetfiles: - pycon-issue11501.patch
nosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
2011-03-15 19:30:20tareksetfiles: - pycon-issue11501.patch
nosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
2011-03-15 13:58:26nessitasetfiles: + pycon-issue11501.patch
nosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
messages: + msg130973
2011-03-15 12:40:33nessitasetnosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
messages: + msg130967
2011-03-15 11:15:10eric.araujosetnosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
messages: + msg130962
2011-03-14 21:16:50nessitasetfiles: + pycon-issue11501.patch
nosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
messages: + msg130902
2011-03-14 21:08:58nessitasetnosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
messages: + msg130898
versions: - Python 3.1, Python 2.7, Python 3.2
2011-03-14 20:55:51eric.araujosetnosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
messages: + msg130896
versions: + Python 3.1, Python 2.7, Python 3.2
2011-03-14 20:46:06nessitasetfiles: + pycon-issue11501.patch
nosy: tarek, ezio.melotti, eric.araujo, nessita, alexis
messages: + msg130894

components: + Distutils, - Distutils2
keywords: + patch
2011-03-14 16:21:24ezio.melottisetnosy: + ezio.melotti
2011-03-14 16:19:38nessitacreate