duplicate lines in zipfile.py

[amauryfa]

BPO	3317
Nosy	@loewis, @amauryfa
Files	zipfile-removedups.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/amauryfa'
closed_at = <Date 2008-07-11.21:28:38.566>
created_at = <Date 2008-07-07.20:57:49.177>
labels = ['type-bug', 'library']
title = 'duplicate lines in zipfile.py'
updated_at = <Date 2008-07-12.21:03:54.715>
user = 'https://github.com/amauryfa'

bugs.python.org fields:

activity = <Date 2008-07-12.21:03:54.715>
actor = 'alanmcintyre'
assignee = 'amaury.forgeotdarc'
closed = True
closed_date = <Date 2008-07-11.21:28:38.566>
closer = 'amaury.forgeotdarc'
components = ['Library (Lib)']
creation = <Date 2008-07-07.20:57:49.177>
creator = 'amaury.forgeotdarc'
dependencies = []
files = ['10852']
hgrepos = []
issue_num = 3317
keywords = ['patch']
message_count = 9.0
messages = ['69398', '69400', '69409', '69415', '69416', '69419', '69538', '69563', '69600']
nosy_count = 3.0
nosy_names = ['loewis', 'amaury.forgeotdarc', 'alanmcintyre']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue3317'
versions = ['Python 2.6']

[amauryfa]

Since r64688, zipfile.py contains duplicated definitions. The attached
patch removes them.

Also, Twisted uses a zipfile item that have been renamed by this change:
zipfile.stringFileHeader (now magicFileHeader).
This makes some tests fail on the community buildbots. See the rightmost
column on
http://www.python.org/dev/buildbot/community/trunk/

I suggest to rename all magicXXX variables back to stringXXX.

[loewis]

Alan, what do you think?

[alanmcintyre]

I don't see a patch attached, but the duplicated code does need
removing. If you can attach a patch I'll try it out.

As much as I dislike the "string" names (magicXXX seemed much more
descriptive), I suppose they're publicly available and shouldn't be
renamed without a good reason. (Unless not being in __all__ counts as
being something one shouldn't depend on as part of the API ;)

[amauryfa]

Sorry, here is the patch

[amauryfa]

Since the use of "from X import *" is discouraged (and serious projects
try to avoid it), the __all__ list is less and less meaningful.

[alanmcintyre]

The patch seems to work just fine for me, all tests pass (including
test_zipfile64) on an Intel Mac. I'd vote to go ahead and revert the
magicXXX variables back to their original names--I'm sure Twisted isn't
the only project out there that made use of them.

If the magicXXX name change gets checked in along with the current patch
I'll run the zip64 tests on Mac and Linux afterward, since they don't
get run in the standard test run on the buildbots. I don't currently
have a Windows machine to test with.

[loewis]

Amaury, as Alan is fine with these changes, feel free to apply them.

[amauryfa]

Committed as 64880.

[alanmcintyre]

Thanks for fixing this, Amaury. I ran the test_zipfile64 and
test_zipfile tests on Linux and OS X, and they pass.