[Python-bugs-list] [ python-Bugs-639118 ] archiver should use zipfile before zip

noreply@sourceforge.net noreply@sourceforge.net
Thu, 21 Nov 2002 07:07:24 -0800


Bugs item #639118, was opened at 2002-11-15 15:33
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=639118&group_id=5470

Category: Distutils
Group: Python 2.2.1
Status: Open
Resolution: Accepted
Priority: 5
Submitted By: ollie oldham (ooldham)
Assigned to: A.M. Kuchling (akuchling)
Summary: archiver should use zipfile before zip

Initial Comment:
The distutils archiver should attempt zipfile.py usage 
before attempting a spawn of an external 'zip'.

The current code in archive_util.py attempts to spawn an 
external 'zip' program for zip action, if this fails, an 
attempt to import zipfile.py is made...

This bites folks who have 'old' or non-conforming zip 
programs on windows platforms...

Have had a conversation about this with thellar, and he 
suggested  A: a bug report, B: changing code to attempt 
import first, then take spawn action if that fails.

Since this is my first bug report, I am attaching a 
archive_util.py that I have modified to do this... I also 
modified this particular file to not use the '-q' option when 
in verbose mode, for the zip spawn. Also modified so 
that if in verbose mode, what gets added to zip file 
during zipfile usage gets printed to stdio.

I tested the attached file for with and w/o verbose, and 
test with and w/o zipfile.py to force the different code 
path executions. Also forced error of having no zipfile.py 
and no external zip program.


----------------------------------------------------------------------

>Comment By: ollie oldham (ooldham)
Date: 2002-11-21 09:07

Message:
Logged In: YES 
user_id=649833

A.M.  Yes. I had considered this type of change as well.
Looks good, however, I have a few comments :)

1) I LIKE the log.info modification you made...

nit pick 1) Consider renaming new variable 'zipfile' to 
something like 'haveZipModule'. Just for clarity...

nit pick 2) You left the comment lines below: 'except 
DistutilsExecError' intact, only the indentation changed... I 
removed this comment, as in my mind it no longer made 
sense, since the function now attempts zipfile module first... 
If you are going to leave it, at least re-word it...

nit pick 3) I re-worded the raise DistutilsExecError message... 
to show the true order of what it could not do - as did the 
original message.


----------------------------------------------------------------------

Comment By: A.M. Kuchling (akuchling)
Date: 2002-11-20 17:15

Message:
Logged In: YES 
user_id=11375

Good idea!

Here's an updated patch against the CVS trunk.  I've rearranged the code a bit to reduce the depth of indentation, but it still seems to work.

Ollie, can you please scan the patch and see if I introduced any problems with the rearrangement?  If you don't spot anything wrong, I'll check it in.




----------------------------------------------------------------------

Comment By: ollie oldham (ooldham)
Date: 2002-11-18 09:00

Message:
Logged In: YES 
user_id=649833

attaching diff between archive_util.orig.py and modified 
archive_util.py

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-11-16 03:44

Message:
Logged In: YES 
user_id=21627

I have a meta comment: Ollie, thanks for your report. It would 
be even more useful if you had attached a context or unified 
diff (diff -c/-u) of the modified file, since that would simplify 
merging you changes with other changes that the file has 
seen (or will see until the patch is accepted).

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2002-11-15 17:11

Message:
Logged In: YES 
user_id=33168

amk, you've been doing a lot of distutils stuff recently. 
Do you have any comments?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=639118&group_id=5470