Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wheels should include entries for directories #287

Closed
jaraco opened this issue Apr 26, 2019 · 9 comments
Closed

Wheels should include entries for directories #287

jaraco opened this issue Apr 26, 2019 · 9 comments

Comments

@jaraco
Copy link
Member

jaraco commented Apr 26, 2019

As discovered here, the wheel library, when producing a wheel, omits entries in the zip file for the directories present in the tree. This behavior has the unfortunate consequence of making PEP 420 namespace packages, which would otherwise be available through the normal importlib machinery, to be unreachable.

My recommendation, therefore, despite the fact that there is no supported container format and wheels are explicitly discouraged for this use, is that wheel should include directory entries in the zip files it creates. Doing so would shorten the divergence from eggs, align the format with the consensus convention, and enable certain features to just work.

@jaraco
Copy link
Member Author

jaraco commented Apr 26, 2019

I haven't tested it yet, but I think something like this might work:

wheel master $ git diff                                                                                                                                                        
diff --git a/wheel/wheelfile.py b/wheel/wheelfile.py
index 9a1c8d2..5b9c387 100644
--- a/wheel/wheelfile.py
+++ b/wheel/wheelfile.py
@@ -111,6 +111,10 @@ class WheelFile(ZipFile):
             # Sort the directory names so that `os.walk` will walk them in a
             # defined order on the next iteration.
             dirnames.sort()
+            for name in dirnames:
+                path = os.path.normpath(os.path.join(root, name))
+                arcname = os.path.relpath(path, base_dir)
+                self.mkdir(path, arcname)
             for name in sorted(filenames):
                 path = os.path.normpath(os.path.join(root, name))
                 if os.path.isfile(path):
@@ -136,6 +140,13 @@ class WheelFile(ZipFile):
         zinfo.compress_type = ZIP_DEFLATED
         self.writestr(zinfo, data, compress_type)
 
+    def mkdir(self, filename, arcname):
+        st = os.stat(filename)
+        zinfo = ZipInfo(arcname + '/', date_time=get_zipinfo_datetime(st.st_mtime))
+        zinfo.external_attr = st.st_mode << 16
+        zinfo.compress_type = ZIP_DEFLATED
+        self.writestr(zinfo, b'')
+
     def writestr(self, zinfo_or_arcname, bytes, compress_type=None):
         ZipFile.writestr(self, zinfo_or_arcname, bytes, compress_type)
         fname = (zinfo_or_arcname.filename if isinstance(zinfo_or_arcname, ZipInfo)

@jaraco
Copy link
Member Author

jaraco commented Apr 26, 2019

The mentioned commit includes the patch above. It still needs (a) a test to cover the desired behavior and (b) fix the existing tests that fail due to the new assumption.

I think I might also recommend a refactor of write_files that separates the two concerns of (a) ordering the outputs of a walk and (b) rendering those outputs to a zipfile.

@cas--
Copy link

cas-- commented May 8, 2019

Could there be a release with this fix included?

@agronholm
Copy link
Contributor

I just tagged the 0.33.2 release. It should appear on PyPI once the Travis pipeline has finished.

@jwodder
Copy link
Contributor

jwodder commented May 8, 2019

@agronholm
In case the pipeline hasn't already told you, you forgot to increment the version number in wheel/__init__.py, so the deployment to PyPI failed because it thought it was uploading a version that already existed.

@agronholm
Copy link
Contributor

Yeah, I noticed this when I got back home. It should be fine now. All my other projects use setuptools_scm for version numbering so this happens from time to time with wheel :)

@agronholm
Copy link
Contributor

agronholm commented May 11, 2019

@jaraco Seeing as this change is causing major problems for others, can you elaborate on the part about "consensus convention"? Is there another tool building wheels in this manner? And what features are you talking about that this enables?

@agronholm
Copy link
Contributor

No matter how hard I tried, I was unable to create a wheel with an empty namespace package in it, even with your patch.

@jaraco
Copy link
Member Author

jaraco commented May 11, 2019

Consensus convention

The consensus convention is illustrated in the first example of this comment, where running the zip command over a directory produces a zipfile with a example_pkg/ entry for the directory.

Similarly, using Python's zipfile, command-line or API, to create a zipfile from a directory tree, will create directory entries:

draft $ mkdir foo                                                                                                                                                              
draft $ python -m zipfile -c out.zip foo                                                                                                                                       
draft $ rmdir foo                                                                                                                                                              
draft $ unzip out.zip                                                                                                                                                          
Archive:  out.zip
   creating: foo/
draft $ python -m zipfile -l out.zip                                                                                                                                           
File Name                                             Modified             Size
foo/                                           2019-05-11 10:22:40            0

Additionally, this comment indicates that even Windows Explorer will create zipfiles with directory entries indicated by a trailing forward slash.

Is there another tool for building wheels in this manner?

As far as I know, there's no other tool for building wheels than this project. However, there are many tools for building zipfiles, a more general concept than wheels... so one way you could build a wheel in this manner would be to (a) download an older wheel, (b) extract it, (c) recreate the wheel from the extracted directory using Info-ZIP or python -m zipfile.

What features are you talking about that this enables?

See bpo-36740 for an illustration of the import behavior in Python that is meant to be addressed by this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants