-
Notifications
You must be signed in to change notification settings - Fork 4
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
[DO NOT MERGE] document why and when --no-build-isolation is used in wheel-building CI #112
base: main
Are you sure you want to change the base?
Conversation
By default, this will create an isolated build environment... a temporary directory containing all of the build-time dependencies of the package. | ||
That's convenient because it means that you don't have to manually install the build dependencies ahead of time or deal with conflicts | ||
between those dependencies and other things in your development environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In keeping with the detail provided elsewhere in this section, we should mention rapids-build-backend here. Conversely, I don't think that we need to explain build isolation since it's a Python default. The old version of this file wasn't explaining what build isolation was, but rather why we couldn't use it. I think it's fair to expect that knowledge, or if not, to link out to pip/build docs rather than explaining it here ourselves.
By default, `pip wheel` or `python -m build` will install such build-time dependencies into a temporary directory (see ["Python Packages"](build.md#python-packages) for details). | ||
The path to that directory is randomly chosen, and so the path that those build dependencies are installed to changes on every build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, I would be comfortable rephrasing this assuming the reader knows what build isolation is and simply providing the informational link for them if they need it.
This pattern is followed to balance these two competing concerns: | ||
|
||
* improved cache hit rates for `sccache`, and therefore faster and less resource-intensive builds | ||
* testing that `rapids-build-backend` correctly generates package metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* testing that `rapids-build-backend` correctly generates package metadata | |
* testing that `rapids-build-backend` correctly generates package metadata | |
* ensuring that the default approach for local builds from source (with isolation) will work out of the box for the non-C++ packages |
Contributes to #108
--no-build-isolation
is used in wheel-building CIrapids-build-backend-now
😀 )pre-commit
config, to fix file endings and trailing whitespaceNotes for Reviewers
I've marked this
DO NOT MERGE
because it's describing as current state changes that haven't actually been made yet... it shouldn't be merged until most of #108 is done.But it's ready for review.