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

Fix editable install by unsetting build_ext.copy_extensions_to_source #1710

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

nicholasjng
Copy link
Contributor

This method was the culprit for the recent editable install breakage, since it just tries to copy the generated extension file without checking its existence.

Since the BazelExtension uses a non-standard location to store the build artifacts, calling the copy method fails the build since the extension is not found in the expected location.

But, since we already copy the file into the source tree as part of the BazelExtension.bazel_build method, it's fine - the extension appears in the right place, and the egg info is generated correctly as well.

This method also does not affect the general install, so it solves the editable problem without regressing the fixed install.

nicholasjng and others added 2 commits December 7, 2023 13:20
This method was the culprit for the recent editable install breakage,
since it just tries to copy the generated extension file without checking
its existence.

Since the `BazelExtension` uses a non-standard location to store the
build artifacts, calling the copy method fails the build since the
extension is not found in the expected location.

But, since we already copy the file into the source tree as part of the
`BazelExtension.bazel_build` method, it's fine - the extension appears
in the right place, and the egg info is generated correctly as well.

This method also does not affect the general install, so it solves the
editable problem without regressing the fixed install.
@nicholasjng
Copy link
Contributor Author

To check that this works, try to python -m pip install -e . from this branch.

What I would like to test before the merge (because I have run into this before) is whether the shutil.copyfile API actually overwrites the .so instead of changing its update time - if this is not the case, it leads to catastrophic failure on macOS due to its kernel protections.

@nicholasjng
Copy link
Contributor Author

Also, if you prefer moving the extension copy to the copy_extensions_to_source API overload, let me know (I think this is even more sensible).

@dmah42 dmah42 merged commit e2c13db into google:main Dec 7, 2023
80 checks passed
@nicholasjng nicholasjng deleted the editable-fix branch December 7, 2023 14:35
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

Successfully merging this pull request may close these issues.

2 participants