classification
Title: Import from Zip Archive
Type: Stage:
Components: Interpreter Core Versions: Python 2.3
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: nnorwitz Nosy List: ahlstromjc, gvanrossum, loewis, nnorwitz, paul.moore
Priority: normal Keywords: patch

Created on 2002-11-29 11:37 by paul.moore, last changed 2002-12-30 21:56 by gvanrossum. This issue is now closed.

Files
File name Uploaded Description Edit
python.diff paul.moore, 2002-11-29 11:37 Patch against current CVS - includes documentation & test changes
foo.zip paul.moore, 2002-11-29 11:40 Test file needed for the new zip import test
zip-patch.zip nnorwitz, 2002-11-29 18:47 neal's diff
Newpatch-Dec01.zip paul.moore, 2002-12-02 16:43
Newpatch-Dec06.zip loewis, 2002-12-06 13:40
Messages (16)
msg41840 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-11-29 11:37
This patch supersedes patch 492105.

This is an updated version of the patch for imports from 
a zip file. This version is against current CVS, and 
includes documentation and tests.

The patch builds on Windows, and runs OK. I haven't 
(yet) run any comprehensive tests, as this is the first 
time I've set up a build environment, and I'm not sure 
how to run the tests yet :-(
msg41841 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-11-29 11:40
Logged In: YES 
user_id=113328

Uploaded file foo.zip, needed (in lib\test directory) for the 
changes to test_import.py
msg41842 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-11-29 11:57
Logged In: YES 
user_id=113328

Bad news. I've just hit problems with the patched Python - 
when I start python.exe, it reports "cannot import site" and 
seems unable to locate the standard library. (This when I run 
python from the PCBuild directory).

I built a clean CVS version, and don't see this problem.

Can anyone else reproduce this, or help me diagnose it? I 
don't understand Jim's changes well enough to see what 
might be the problem (I updated the patch purely 
mechanically).
msg41843 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2002-11-29 18:47
Logged In: YES 
user_id=33168

Paul, thanks for taking this up.  I made very little
progress on this  I have attached my version of the diffs.

I had some problems with the original patch which I was
never able to correct.  For example, I think it was not
possible to run python from a local build env't.  It
appeared that python had to be installed for everything to
work.  See the problem in bug 586680 (also in site.py). 
That should probably be addressed and was part of my problem.

I tried to simplify the code some, so you should see some
differences between the patch I attached and the original. 
These are not as important as making the patch work
properly, both when installed and running from a build env't.

I will try to take a look at your problem later, but it may
take a while.  Let me know if you make progress.
msg41844 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-11-29 21:11
Logged In: YES 
user_id=113328

My problem turns out to be an interaction with the 
new "universal newline support". In find_module, within the 
switch(path_type), case PY_IMP_LISTDIR, the section of 
code marked /* Search the list of suffixes: .pyc, .pyd, ... */ 
tries to do fopen(buf, fdp->mode). But .py files are now 
mode 'U' (universal newline) which is not a valid mode for fopen
(). I've made a fix, but I'm not sure how reliable it is.

On a related note, I'm not 100% sure how robust the code is 
in the presence of case-insensitive filesystems. I'm way out of 
my depth in this code, so I don't know if I'll be able to fix it, 
but I'll have a look (and I'll look at your version of the patch, as 
well).
msg41845 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-12-01 22:34
Logged In: YES 
user_id=113328

I've now taken Neal's patch and integrated my changes. I'll 
check it through again, and run a few more tests, and then 
upload the result (I've had problems with cvs update, so I'm 
downloading a new copy of CVS python first).

Most things look OK. There are a couple of issues with case-
insensitive filesystems.

The first is that the cache of os.listdir results is (case-
sensitively) keyed on directory name. I don't see this as an 
issue as (a) I'd expect the case used to remain constant 
(who's going to change a directory in sys.path at runtime to 
the same name with a different case???) and (b) it's only an 
efficiency hit, not an error, in any case.

The second issue is more problematic. Zip files on sys.path 
are recognised (see add_zip_names()) by the extension ".zip" 
(case sensitively). It's arguable that on case-insensitive 
filesystems, this check should also be case insensitive. But 
finding that a file is on a case-insensitive filesystem is, I 
believe, not possible. And using a case-insensitive test all the 
time isn't right either. I'd say that we should just *define* the 
algorithm as only treating files ending in ".zip" (case 
sensitive) as zip files. It doesn't seem to be a problem in 
practice.

I'll try to find some way to fit that into the documentation...

Apart from this issue, I've seen no other problems with the 
patch.
msg41846 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-12-01 23:00
Logged In: YES 
user_id=21627

I certainly agree with only treating .zip as indication for
zipfiles. On a case insensitive file system, you can put
.zip into sys.path and it will still find .ZIP.
msg41847 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-12-02 16:43
Logged In: YES 
user_id=113328

Aargh. It looks like Neal's patch is wrong - it didn't include the 
changes to the files in the PC directory. Also it has a lot of 
strange changes, like replacing PyEval_CallObject with 
PyObject_Call - I don't see how this relates to zipfiles.

I wasted most of this morning trying to juggle too many 
patches, to no avail. I think I'm going to have to revert to my 
patch, get it applied to the CVS version, and upload that. 
Neal, I agree with a lot of the changes you made, but they'll 
need redoing - can you apply my patch to a clean checkout 
and redo your tidying up?

The patch I'm uploading with this comment builds cleanly for 
me on Windows 2000, and passes the supplied tests. You 
need to apply the patch, then put the 2 included zipfiles in the 
lib/test directory.

I'd be very grateful if someone could test this on Unix and/or 
Mac, and update it where necessary. I've not tested these 
platforms, but I'm assuming that Jim's original code worked 
there. There are also other areas which seem strange (the big 
block of deleted code in sysmodule.c worries me - but I see 
no failures because of it, so I've left it unchanged) and 
comments would be very helpful, but in the absence of 
anything better I'm sticking with "it worked when I tried it"...
msg41848 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2002-12-02 16:55
Logged In: YES 
user_id=33168

Hmmm, I thought I used to have changes to PC.  Not sure,
though.  I'll try to take a look at the diffs and test on
unix later this evening.  I can also add back in any of my
changes after you get everything working.
msg41849 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-12-03 09:31
Logged In: YES 
user_id=21627

Paul, there is sentiment on python-dev that this patch is 
unacceptable, because it is incomprehensible. I can 
sympathize with these feelings, so we need to take the 
possibility into account that this won't go into Python 2.3.
msg41850 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-12-03 14:06
Logged In: YES 
user_id=113328

Understood (I've been watching python-dev). I'm happy to 
keep working on this while a consensus is reached. Actually, 
I don't think there's much more I can do, I'm hoping that users 
of other OS's can test this now....
msg41851 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2002-12-03 14:08
Logged In: YES 
user_id=113328

Understood (I've been watching python-dev). I'm happy to 
keep working on this while a consensus is reached. Actually, 
I don't think there's much more I can do, I'm hoping that users 
of other OS's can test this now....
msg41852 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2002-12-03 14:31
Logged In: YES 
user_id=33168

Paul, I didn't get a chance to review/test the patch.  But
as I just said in python-dev, I think it's worthwhile. 
Guido seems to agree.  I'll try to test on Linux soon.
msg41853 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-12-06 13:40
Logged In: YES 
user_id=21627

I've now tested this patch on Unix. Support for compressed
files didn't work from the build directory, since the zlib
import happens before site.py is run. To work around this
issue, I import zlib in site.py if we run from the build
directory. I wonder whether the zlib import could be delayed
until a compressed zipfile is encountered.

Regenerated patch, attached as Newpatch-Dec06.zip (there,
zipped-uncompressed.zip actually does contain uncompressed
files).
msg41854 - (view) Author: James C. Ahlstrom (ahlstromjc) Date: 2002-12-10 18:31
Logged In: YES 
user_id=64929

> Support for compressed
> files didn't work from the build directory, since the zlib
> import happens before site.py is run.

I imported zlib before site.py is imported because site.py
may itself be in a compressed zip archive.  It looks like
your trick of importing zlib in site.py works, and there is
no need to limit it to running from the build directory
since "import zlib" is cheap.  An alternative is to try a
zlib import after site.py is imported.
 
As noted, this patch may not be accepted anyway, so I will
stand by on this and any other problems.
msg41855 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-12-30 21:56
Logged In: YES 
user_id=6380

I'm going with Just's version. However, some of the code
here may still make it into 2.3a2 (the bootstrap related
patches).
History
Date User Action Args
2002-11-29 11:37:24paul.moorecreate