Make spread_purelib_into_root behave like the wheel was installed by pip #581
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pip. Merge purelib dir into toplevel even if purelib and toplevel have subdirs with the same name. See tensorflow-io for an example of a package which was not installed correctly by rules_python before this change.
7e083a7 to
a5381b7
Compare
alexeagle
approved these changes
Dec 29, 2021
| for child in purelib_dir.iterdir(): | ||
| if child.is_dir(): | ||
| backport_copytree(src=child, dst=pathlib.Path(root_dir, child.name)) | ||
| elif not pathlib.Path(root_dir, grandchild.name).exists(): |
There was a problem hiding this comment.
I think grandchild -> child is a needed patch, to avoid NameError?
There was a problem hiding this comment.
Yes, thanks for catching that. Will provide a patch.
hrfuller
added a commit
that referenced
this pull request
Jan 14, 2022
Follow up for #581 fixing undefined name.
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merge purelib dir into toplevel even if purelib and toplevel have
subdirs with the same name. See tensorflow-io for an example of
a package which was not installed correctly by rules_python before this
change.
PR Checklist
Please check if your PR fulfills the following requirements:
.parfiles. See CONTRIBUTING.md for infoPR Type
What kind of change does this PR introduce?
What is the current behavior?
In pip_install and pip_parse rule if you try to install a wheel which has Root-Is-Purelib: false (tensorflow-io is an example), python sources in the .data/purelib dir will not be merged into the root because we skip children of the purelib which have the same basename as a directory in the top level of the wheel.
What rules_python does currently: https://gist.github.com/hrfuller/ccc0420cd5b72b38f6151fa1b5c811e0
After running the example open up the created repo in the bazel output base, and notice that purelib directory is not merged.
This leads to issues where tensorflow_io python sources cannot be imported.
Issue Number: N/A
What is the new behavior?
Descendants of the purelib directory will be merged with existing directories in the top level, but will not overwrite any files in the toplevel. This is the same thing that pip does when you install a wheel.
To verify this run
pip install --no-deps --only-binary :all: --target tfio tensorflow-io==0.19.0Compare the shape of the tfio directory with the external repo bazel generated using the gist above.
Does this PR introduce a breaking change?
Other information