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

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.

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

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.

* 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).

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

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.

Attaching a new patch with latest changes from trunk merged in (conflicts resolved).

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

Reopening for 2.7.4.

If this is still wanted for python2, someone should create a 2.7 specific patch.

Tests are already skipped if zlib is not available in 2.7. To backport test_make_zipfile_no_zlib we also need the 'patch' helper so I'm not sure it's worth it at this point.