Import from Zip archive

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

BPO	492105
Nosy	@gvanrossum, @loewis, @pfmoore
Files	dashc.diff: diff -c version of the patch (Mar15, 2002) dashc-2.diff: Partially fixed diff import.c.diff: diff -c import.c versus Python-2.2.1 import.c.diff: diff -c import.c versus CVS 8-15-2002

^{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: ```python assignee = None closed_at = created_at = labels = ['interpreter-core'] title = 'Import from Zip archive' updated_at = user = 'https://bugs.python.org/ahlstromjc' ``` bugs.python.org fields: ```python activity = actor = 'paul.moore' assignee = 'nnorwitz' closed = True closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'ahlstromjc' dependencies = [] files = ['3836', '3837', '3838', '3839'] hgrepos = [] issue_num = 492105 keywords = ['patch'] message_count = 26.0 messages = ['38433', '38434', '38435', '38436', '38437', '38438', '38439', '38440', '38441', '38442', '38443', '38444', '38445', '38446', '38447', '38448', '38449', '38450', '38451', '38452', '38453', '38454', '38455', '38456', '38457', '38458'] nosy_count = 6.0 nosy_names = ['gvanrossum', 'loewis', 'jhylton', 'nnorwitz', 'paul.moore', 'ahlstromjc'] pr_nums = [] priority = 'normal' resolution = 'out of date' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue492105' versions = ['Python 2.3'] ```

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

This is the "final" patch to support imports from zip archives, and directory caching using os.listdir(). It replaces patch 483466 and 476047. It is a separate patch since I can't delete file attachments. It adds support for importing from "" and from relative paths.

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

Logged In: YES user_id=64929

I still can't delete files, but I added a new file which contains all diffs as a single file, and is made from the current CVS tree (Mar 15, 2002).

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

Logged In: YES user_id=64929

I added a diff -c version of the patch.

[03bde425-37ce-4291-88bd-d6cecc46a30e]

Logged In: YES user_id=31392

Deleteing the old diffs that Jim couldn't delete.

[61337411-43fc-4a9c-b8d5-4060aede66d0]

Logged In: YES user_id=21627

Is this patch ready to be applied?

[gvanrossum]

Logged In: YES user_id=6380

Sigh. We waited too long for this one, and now the patch is hopelessly out of date. I managed to fix most of the failing hunks, but the remaining hunk that fails (#11 in import.c) is a whopper: the saved import.c.rej is 270 lines long.

I'm going to sleep on that, but I could use some help.

In the mean time, I'm uploading an edited version of dashc.diff to help the next person.

[d21744ff-f396-4c71-955e-7dbd2e886779]

Logged In: YES user_id=33168

I'm reviewed most of the updated version. Quick summary:

• strncpy() doesn't null terminate strings, this needs to be added
• seems to be many leaked references

Here are more details:

• strncpy(dest, str, size) doesn't guarantee the result to be null terminated, need dest[size] = '\0'; after all strncpy()s (I see a bunch in getpath.c)
• getpath.c:get_sys_path_0(), I think some compilers warn when you do char[i] = 0; (use '\0' instead) (there may be other places of this)
• import.c:PyImport_InitZip(), have_zlib is an alias to zlib and isn't really helpful/necessary
• import.c:get_path_type() can leak a reference to pyobj if it's an int
• import.c:add_directory_names() pylist reference is leaked
• import.c:add_zip_names(), memcmp(path+i, ".zip", 4) is clearer to me than path[i] == '.' ....
• import.c:add_zip_names, there's a lot of magic #s in this function
• " " : leak refs when doing PyTuple_SetItem

I think there were other leaked references. I'll try to spend some more time later.

[gvanrossum]

Logged In: YES user_id=6380

Whoa... That's a lot. Neal, do you think you could come up with a reworked version of this? That would be great!

[d21744ff-f396-4c71-955e-7dbd2e886779]

Logged In: YES user_id=33168

Reassigning to me. I'll give it a shot. It will take a while. Do I understand you correctly that your updated patch (dashc-2) has all the necessary pieces, except for the import hunk 11 that was rejected? And I need to get that failed hunk from the original patch?

[gvanrossum]

Logged In: YES user_id=6380

No, the failing hunk is included in dashc-2. I actually *edited* the patch file until all but that one hunk succeeded.

Thanks for looking into this! If you need help don't fail to ask on python-dev, I read it daily. :-)

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

Logged In: YES user_id=64929

This patch is old. I can provide a new patch against Python 2.2.1 if that would help. Or a new patch just for import.c against 2.2.1.

[d21744ff-f396-4c71-955e-7dbd2e886779]

Logged In: YES user_id=33168

James, could you look at what Guido reworked? If that is fine, I can push it forward. Otherwise, feel free to update the patch. If I do any work on it, I'll make comments here so we don't duplicate work. Thanks.

[gvanrossum]

Logged In: YES user_id=6380

I think a new patch just for import.c would be helpful.

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

Logged In: YES user_id=64929

Here is the import.c diff -c against Python-2.2.1.

[gvanrossum]

Logged In: YES user_id=6380

Alas, the 2.2.1 diff doesn't help much. Current CVS is what we need. :-(

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

Logged In: YES user_id=64929

I just grabbed the CVS import.c (only). I will edit this to add my changes and submit it as a new import.c patch. This should help, although I can't test it unless I download the whole tree.

[a91270f9-3c6f-42ef-a627-c523b1e6bf04]

Logged In: YES user_id=64929

Here is a diff -c against today's import.c. It is untested because I didn't get the whole tree, but it is way closer than the old patch. I moved the find_module() loop over file suffixes into search_using_fopen(), a trivial change which produces a large diff. To make the change obvious, I didn't correct the indenting, so please do that after looking at the patch.

Next I will download the whole tree and check the other changes once I get a little time.

[pfmoore]

Logged In: YES user_id=113328

Sorry to poke my nose in on this, but I note that Guido is hoping for a 2.3a1 release by Christmas, and this patch has been making very slow progress.

I have an interest in seeing it go in - is there anything I can do to help?

[61337411-43fc-4a9c-b8d5-4060aede66d0]

Logged In: YES user_id=21627

Try it out, report whether it works; if it doesn't work, fix it. Verify that documentation and test cases are up-to-date.

[pfmoore]

Logged In: YES user_id=113328

I've uploaded a revised patch (zip-cvs.diff). This is against current Python CVS. It's basically just the existing import.c patch, plus the rest of the dashc-2 patch. Everything applied OK (with fuzz and offsets, but otherwise OK).

The resulting code builds, but when I try importing from a zip file, I get a crash. However, this happens when I use the zlib module directly, so appears to be because I haven't built zlib correctly :-(

I will investigate further, but thought I'd upload the new patch in case anyone else wants to take a look.

[pfmoore]

Logged In: YES user_id=113328

How the *&^!%%$ do I upload files to SF?

For now, the patch is at http://www.morpheus.demon.co.uk/zip-cvs.diff.

[pfmoore]

Logged In: YES user_id=113328

Sigh. If I'd read the README file properly, I wouldn't have screwed up the zlib build.

I am now happy to report that the patch seems to work fine. I've only done some simple tests, but have no problems.

[61337411-43fc-4a9c-b8d5-4060aede66d0]

Logged In: YES user_id=21627

Paul, Can you propose documentation changes for this patch? I'd expect to see modifications to NEWS, whatsnew23.tex, api, and ref, perhaps other places as well. Feel free to use material from PEP-273 as appropriate.

Also, having test cases for the feature would be good.

As you cannot attach to this report (you are not the submitter), feel free to create a new patch. To create a diff for a cvs tree, using the "cvs diff" command is best - no need to have two copies of the tree.

[pfmoore]

Logged In: YES user_id=113328

I'll see what I can do. I've never looked at documentation or test patches before, so I'll have to get my brain around things first...

I'll also have a look at the issues Neal raised.

Is the diff format I used a problem? I have dial-up internet access, so having 2 copies of the tree is faster for me than doing cvs diff, as well as being possible while offline :-(

[61337411-43fc-4a9c-b8d5-4060aede66d0]

Logged In: YES user_id=21627

The diff format is fine; I was just not certain you know about cvs diff.

[pfmoore]

Logged In: YES user_id=113328

Opened a new patch (645650) with an updated diff.