-
Notifications
You must be signed in to change notification settings - Fork 21
Sets PIP_USE_FEATURE="build-constraint" in rapids-init-pip #237
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
Sets PIP_USE_FEATURE="build-constraint" in rapids-init-pip #237
Conversation
for more information, see https://pre-commit.ci
jameslamb
left a comment
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.
Thanks, this is exactly what I was expected and should be enough.
Before we merge this, could you try testing it in some project's wheel CI? ucxx is probably a good candidate, I see this warning in the distributed-ucxx wheel building jobs (build link)
Since this doesn't call any other gha-tools, I'd probably just stick the content of this file in a file like ci/rapids-init-pip and then change all the rapids init pip calls in ucxx CI scripts to source ci/rapids-init-pip.
We'd want to see builds and tests run successfully without this warning.
And I'd test that the constraint's being respected by adding a couple garbage characters in the middle of the filepath of lines like https://github.com/rapidsai/ucxx/blob/19c4b58ba42eff314083a9a11c79ece9f36fbd51/ci/build_wheel_ucxx.sh#L20 to see if pip complains.
Approving so you can merge whenever this has been tested and you're feeling confident in it.
jameslamb
left a comment
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.
A few suggestions based on the testing.
Co-authored-by: James Lamb <jaylamb20@gmail.com>
|
@jameslamb - Done |
jameslamb
left a comment
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.
Great, let's do it!
Once we merge this and it's built into new CI images, I'm not expecting it to break anything, based on our testing in rapidsai/ucxx#567
And I think doing it in this script is preferable to adding this in the global pip.conf in CI images (https://github.com/rapidsai/ci-imgs/blob/main/context/pip.conf) for the benefit of reproducing CI locally outside of those images.
…apidsai#237)" This reverts commit ceb80cf.
…#2212) Contributes to rapidsai/build-planning#242 Modifying `ci/build_wheel_python.sh` to avoid passing `--build-constraint` and `--no-build-isolation` together which results in an error from `pip`, however we want to keep environment variable `PIP_CONSTRAINT` set unconditionally. Can be merged after rapidsai/gha-tools#237 Authors: - Mike McCarty (https://github.com/mmccarty) Approvers: - James Lamb (https://github.com/jameslamb) URL: #2212
…rapidsai#2212) Contributes to rapidsai/build-planning#242 Modifying `ci/build_wheel_python.sh` to avoid passing `--build-constraint` and `--no-build-isolation` together which results in an error from `pip`, however we want to keep environment variable `PIP_CONSTRAINT` set unconditionally. Can be merged after rapidsai/gha-tools#237 Authors: - Mike McCarty (https://github.com/mmccarty) Approvers: - James Lamb (https://github.com/jameslamb) URL: rapidsai#2212
…#21048) Contributes to rapidsai/build-planning#242 Modifying `ci/build_wheel.sh` to avoid passing `--build-constraint` and `--no-build-isolation` together which results in an error from `pip`, however we want to keep environment variable `PIP_CONSTRAINT` set unconditionally. Can be merged after rapidsai/gha-tools#237 Authors: - Mike McCarty (https://github.com/mmccarty) Approvers: - James Lamb (https://github.com/jameslamb) URL: #21048
Contributes to rapidsai/build-planning#242
In rapids-init-pip (code link):