wheel builds: react to changes in pip's handling of build constraints

[mmccarty]

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

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[rockhowse]

ahh see this is DO NOT MERGE and @mmccarty is the author... thought it was a bot for second 😅 ignoring until I get pinged if this needs to be merged in the future.

[mmccarty]

Yeah, this is just a test for rapidsai/build-planning#242

@jameslamb - Is this what you had in mind?

[mmccarty]

/ok to test

[jameslamb]

Mike and I are pairing on this @rockhowse, and it's just a test that'll never be merged. You can unsubscribe.

For this test, I'd like to see the proposed new rapids-init-pip used everywhere in this repo. We should be testing the effect of these changes on both builds AND tests.

[mmccarty]

/ok to test

[mmccarty]

/ok to test

[copy-pr-bot]

/ok to test

@mmccarty, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

[mmccarty]

/ok to test

[copy-pr-bot]

/ok to test

@mmccarty, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

[mmccarty]

/ok to test c8f7ef1

[mmccarty]

/ok to test 66e6398

[mmccarty]

Interesting error in wheel-build-libucxx @jameslamb

ERROR: --build-constraint cannot be used with --no-build-isolation.

[mmccarty]

/ok to test 8143b9d

[jameslamb]

Interesting error in wheel-build-libucxx @jameslamb

ERROR: --build-constraint cannot be used with --no-build-isolation.

Ugh that is going to make this a bit more complicated. I was hoping we could just set these environment variables and be done with it, but that error message means we'll need to go case-by-case and do different things based on whether or not build isolation is used.

We really do want --no-build-isolation for anything that ends up doing a lot of compiling, because without it we get frequent cache misses from sccache (rapidsai/build-planning#108).

So maybe the right mix here is like this?

• leave rapids-init-pip unchanged
• add command-line flag --build-constraint "${PIP_CONSTRAINT}" to every invocation of pip wheel in CI scripts

Kind of unfortunate that it'll require touching every repo, but that shouldn't be too bad and should allow the process of cross-PR testing to still work exactly as it did in https://docs.rapids.ai/resources/github-actions/#using-wheel-ci-artifacts-in-other-prs.

Do you agree? If so could you try something like that here?

[mmccarty]

Agreed. I'll work on that. Thanks!

[mmccarty]

/ok to test 2818508

[mmccarty]

/ok to test 342efd1

[mmccarty]

/ok to test dd142bd

[mmccarty]

/ok to test 1cef895

[mmccarty]

This latest commit conditionally passes the build constraint flag if build isolation is enabled. Note, that we still get the deprecation warning when build isolation is disable (example here).

[jameslamb]

_This latest commit conditionally passes the build constraint flag if build isolation is enabled. _

Ok good, I think that's what we want to do.

Note, that we still get the deprecation warning when build isolation is disable (example here).

I think this is because you've fully reverted out rapids-init-pip. I was wrong to say "leave rapids-init-pip unchanged", I think we still need to set PIP_USE_FEATURE="build-constraint" in the environment unconditionally to be sure we're opting in to the new behavior.

As the warning says:

To disable this warning without any build constraints set --use-feature=build-constraint or PIP_USE_FEATURE="build-constraint".

[jameslamb]

This is getting closer, left a few comments for your consideration.

[mmccarty]

/ok to test 6c3b7ce

[mmccarty]

/ok to test 52d60cb

[mmccarty]

/ok to test 1b670c5

[mmccarty]

/ok to test a3e83fd

[jameslamb]

Re-approving, looks great, thanks!

[jameslamb]

/merge

[jameslamb]

Ah right, can't /merge now that release/0.48 is cut. I'll manually merge this.