| msg13298 - (view) |
Author: ollie oldham (ooldham) |
Date: 2002-11-15 21:33 |
| 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. |
|
|
| msg13299 - (view) |
Author: Neal Norwitz (nnorwitz) *  |
Date: 2002-11-15 23:11 |
| Logged In: YES user_id=33168 amk, you've been doing a lot of distutils stuff recently. Do you have any comments? |
|
|
| msg13300 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2002-11-16 09:44 |
| 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). |
|
|
| msg13301 - (view) |
Author: ollie oldham (ooldham) |
Date: 2002-11-18 15:00 |
| Logged In: YES user_id=649833 attaching diff between archive_util.orig.py and modified archive_util.py |
|
|
| msg13302 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2002-11-20 23:15 |
| 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. |
|
|
| msg13303 - (view) |
Author: ollie oldham (ooldham) |
Date: 2002-11-21 15:07 |
| 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. |
|
|
| msg13304 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2002-11-21 18:19 |
| Logged In: YES user_id=11375 On 1), zipfile is either the module object or None; it's not a Boolean, so I'd like to keep it named 'zipfile'. On 2), good point; edited down to # XXX really should distinguish between "couldn't find # external 'zip' command" and "zip failed". On 3), another good point; fixed. |
|
|
| msg13305 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2002-11-21 18:54 |
| Logged In: YES user_id=11375 Checked in to CVS as rev. 1.15 of archive_util.py; thanks for your contribution! |
|
|