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

Change jaxlib build rules to build a wheel package. #4982

Merged
merged 1 commit into from
Nov 21, 2020

Conversation

hawkinsp
Copy link
Collaborator

@hawkinsp hawkinsp commented Nov 20, 2020

Before this PR, the jaxlib build script (build/build.py) writes its output to build/ subdirectory of the jax source tree. This is a bit ugly since it intermingles the jax source tree with the artifacts built from it (an unpacked wheel).

This PR changes the build scripts to build a .whl package as the output of running build.py, which is by default written to the dist/ subdirectory of the current directory.

Also remove mentions of cython as a build dependency. We don't need the user to provide cython, bazel downloads it. six also seems unused.

@google-cla google-cla bot added the cla: yes label Nov 20, 2020
@hawkinsp hawkinsp requested a review from skye November 20, 2020 15:47
@hawkinsp hawkinsp added the pull ready Ready for copybara import and testing label Nov 20, 2020
@hawkinsp
Copy link
Collaborator Author

@cloudhan

@cloudhan
Copy link
Contributor

Hmm, it take me time to figure out why from wheel import pep425tags fails, turns out it has been removed in 0.35 of wheel
See pypa/wheel#370 and pypa/wheel@a519770

@hawkinsp
Copy link
Collaborator Author

Hmm, it take me time to figure out why from wheel import pep425tags fails, turns out it has been removed in 0.35 of wheel
See pypa/wheel#370 and pypa/wheel@a519770

I just noticed that too. I already pushed a new version that fixes this.

@cloudhan
Copy link
Contributor

tmpdir.cleanup() is causing the same problem.

@hawkinsp
Copy link
Collaborator Author

tmpdir.cleanup() is causing the same problem.

Can you verify you are using commit c06ead6 ?

In particular I was seeing that until I added an extra os.chdir to make sure the current directory wasn't inside the temp dir when it was deleted.

@cloudhan
Copy link
Contributor

I think I'll wait you merge this PR and fix that later.

@cloudhan
Copy link
Contributor

I am on that commit. But cleanup is calling into rmtree and rmtree will refuse to delete readonly pyd file. 😂

@copybara-service copybara-service bot merged commit 83e7e39 into jax-ml:master Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants