Use literal '/'s to build internal zip file paths #51
Conversation
|
Can one of the admins verify this patch? |
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
|
CLAs look good, thanks! |
|
Can one of the admins verify this patch? |
rules_python/whl.py
Outdated
| @@ -60,12 +61,12 @@ def metadata(self): | ||
| with zipfile.ZipFile(self.path(), 'r') as whl: | ||
| # first check for metadata.json | ||
| try: | ||
| with whl.open(os.path.join(self._dist_info(), 'metadata.json')) as f: | ||
| with whl.open(posixpath.join(self._dist_info(), 'metadata.json')) as f: |
There was a problem hiding this comment.
Is posixpath always available on Windows and other non-Posix platforms? From the zip specification and the code for the zipfile module, it appears that literal forward slash '/' is the only thing that can be used here, so we could safely simplify this if we wanted to:
with whl.open(self._dist_info() + '/metadata.json') as f
|
Jenkins: test this please |
|
I ran into this on Windows, and tested the fix there. :-) I don’t know about other non-Posix platforms, but other than Windows, I don’t know any that are supported by Bazel. Using a literal slash should also work fine; want me to make the change? |
|
Yes, please go ahead and change to literal slashes. |
|
Done |
|
FYI -- without write access to the repository, I can't actually merge the pull request myself; someone else will have to do that. :-) |
|
Jenkins: test this please |
|
@mattmoor I think the CI is wedged, and I don't have permission to force a merge. |
|
Test this please |
|
Oh, it got renamed. @davidstanke that's annoying :) |
|
This is the protected branch feature, you should be able to mess with it under |
Necessary in order to work on Windows