-
Notifications
You must be signed in to change notification settings - Fork 112
wheel builds: react to changes in pip's handling of build constraints #785
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
Conversation
📝 WalkthroughWalkthroughThe changes refactor wheel-building scripts to centralize and conditionally manage pip wheel invocation arguments. A new RAPIDS_PIP_WHEEL_ARGS array is introduced in build_wheel.sh with conditional constraint handling logic, and documentation comments are clarified to explain PIP_CONSTRAINT scope across different pip command types. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ci/build_wheel.sh`:
- Around line 32-39: Update the incorrect GitHub issue reference in the comment
that explains PIP_NO_BUILD_ISOLATION behavior: replace the URL and issue number
referencing "#573" with the correct issue "#5735" (the comment near the
PIP_NO_BUILD_ISOLATION check that mentions PIP_CONSTRAINT,
RAPIDS_PIP_WHEEL_ARGS, and --build-constraint). Ensure the comment text still
accurately describes that PIP_NO_BUILD_ISOLATION=0 means "add
--no-build-isolation" and that the URL now points to the correct GitHub issue
discussing that behavior.
| # Only use --build-constraint when build isolation is enabled. | ||
| # | ||
| # Passing '--build-constraint' and '--no-build-isolation` together results in an error from 'pip', | ||
| # but we want to keep environment variable PIP_CONSTRAINT set unconditionally. | ||
| # PIP_NO_BUILD_ISOLATION=0 means "add --no-build-isolation" (ref: https://github.com/pypa/pip/issues/573 | ||
| if [[ "${PIP_NO_BUILD_ISOLATION:-}" != "0" ]]; then | ||
| RAPIDS_PIP_WHEEL_ARGS+=(--build-constraint="${PIP_CONSTRAINT}") | ||
| fi |
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.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && cat ci/build_wheel.sh | sed -n '32,39p'Repository: NVIDIA/cuopt
Length of output: 105
🌐 Web query:
PIP_NO_BUILD_ISOLATION pip issue 5735
💡 Result:
Short summary — Issue #5735 (pypa/pip): users reported that the environment variables with names like PIP_NO_BUILD_ISOLATION behave counter‑intuitively — their truthiness is inverted compared with what the names suggest (i.e., the NO vars are treated oddly so setting them to "off"/"false"/"0" can enable the feature and "on"/"true"/"1" can disable it), which confused many users. The issue requests clarifying/fixing that behavior. [1]
Related/follow‑up: other pip issues show practical problems around build isolation (e.g., when disabling build isolation pip may ignore or change how it handles build-system requirements), so the recommended reliable approach is to use the explicit CLI flag (--no-build-isolation) or install build dependencies manually when turning off isolation. [2][3]
Sources:
- GitHub: "
PIP_NO_BUILD_ISOLATIONbehaves opposite to how it reads" (Issue#5735). [1] - GitHub: "pip is completely ignoring build-system requirements when build isolation is disabled" (Issue
#9314). [2] - Stack Overflow explanation of --no-build-isolation and recommended workflow. [3]
🌐 Web query:
pip PIP_NO_BUILD_ISOLATION environment variable issue
💡 Result:
Summary and fixes
-
What it does: PIP_NO_BUILD_ISOLATION corresponds to the pip CLI option --no-build-isolation (i.e., disable PEP 517/518 build isolation so pip will build using the current environment). Use of this requires you have the build dependencies installed yourself. (pip.pypa.io)
-
Common problems and why they happen:
- Confusing boolean semantics / historical behavior: some users reported that NO environment variables behaved counter‑intuitively in older pip versions (causing confusion about which value enables/disables the feature). See the longstanding GitHub discussion. (github.com)
- Some pip versions / workflows ignore build-system requirements when build isolation is disabled (so disabling isolation can cause builds to fail because pip won’t auto-install build deps). This is a reported/closed issue affecting some versions. (github.com)
- Wrapper tools (e.g., uv, custom build scripts) may not pass environment variables through or may not map the variable to the pip flag, so the env var appears to “not work.” Confirm the wrapper supports the variable. (github.com)
-
How to troubleshoot / fix
- Prefer the explicit CLI flag to avoid ambiguity: pip install --no-build-isolation . (pip.pypa.io)
- If you need the env var, set it truthy (e.g., on Unix: export PIP_NO_BUILD_ISOLATION=1; on Windows: setx PIP_NO_BUILD_ISOLATION 1) and confirm pip version supports it. Then run pip in the same shell. (pip.pypa.io)
- Ensure build dependencies named in pyproject.toml [build-system] (e.g., setuptools, wheel, build-backend requirements) are already installed in your environment when you disable isolation. Otherwise the build will fail. (pip-test.readthedocs.io)
- If a wrapper/tool ignores the env var, use the CLI flag or update/configure the wrapper to pass the flag/variable (or file an issue with that tool). (github.com)
- If builds still fail, check your pip version and search pip’s issue tracker for matching bugs (some historical bugs relate to ignoring build-system requirements with no-build-isolation). Upgrading/downgrading pip may help. (github.com)
If you want, tell me: your pip version and the exact command/output and I’ll give targeted steps.
Citations:
- 1: https://pip.pypa.io/en/stable/cli/pip_install.html?utm_source=openai
- 2:
PIP_NO_BUILD_ISOLATIONbehaves opposite to how it reads pypa/pip#5735 - 3: pip is completely ignoring build-system requirements when build isolation is disabled pypa/pip#9314
- 4: Add environment variable for
--no-build-isolationastral-sh/uv#3309 - 5: https://pip.pypa.io/en/stable/cli/pip_install.html?utm_source=openai
- 6: https://pip.pypa.io/en/stable/cli/pip_install.html?utm_source=openai
- 7: https://pip-test.readthedocs.io/en/latest/reference/pip/?utm_source=openai
- 8: Add environment variable for
--no-build-isolationastral-sh/uv#3309 - 9: pip is completely ignoring build-system requirements when build isolation is disabled pypa/pip#9314
🏁 Script executed:
find . -name "build_wheel.sh" -type fRepository: NVIDIA/cuopt
Length of output: 75
🌐 Web query:
site:github.com/pypa/pip/issues/573
💡 Result:
I found the GitHub issue: "don't --user install in --system_site_packages virtualenvs when a conflict is present in the virtualenv site." (pypa/pip #573), opened Jun 10, 2012 — it explains that in virtualenvs created with --system_site_packages, sys.path order makes a --user install unusable if a conflicting package already exists in the virtualenv site; the issue was closed and is auto-locked. [1]
Source: GitHub — pypa/pip issue #573. [1]
🏁 Script executed:
sed -n '32,39p' ./ci/build_wheel.shRepository: NVIDIA/cuopt
Length of output: 526
Fix incorrect GitHub issue reference in comment.
The URL on line 36 references issue #573, which is unrelated to PIP_NO_BUILD_ISOLATION behavior. The correct issue is #5735 ("PIP_NO_BUILD_ISOLATION behaves opposite to how it reads"), which directly addresses the semantics discussed in the comment.
Proposed fix
-# PIP_NO_BUILD_ISOLATION=0 means "add --no-build-isolation" (ref: https://github.com/pypa/pip/issues/573
+# PIP_NO_BUILD_ISOLATION=0 means "add --no-build-isolation" (ref: https://github.com/pypa/pip/issues/5735)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Only use --build-constraint when build isolation is enabled. | |
| # | |
| # Passing '--build-constraint' and '--no-build-isolation` together results in an error from 'pip', | |
| # but we want to keep environment variable PIP_CONSTRAINT set unconditionally. | |
| # PIP_NO_BUILD_ISOLATION=0 means "add --no-build-isolation" (ref: https://github.com/pypa/pip/issues/573 | |
| if [[ "${PIP_NO_BUILD_ISOLATION:-}" != "0" ]]; then | |
| RAPIDS_PIP_WHEEL_ARGS+=(--build-constraint="${PIP_CONSTRAINT}") | |
| fi | |
| # Only use --build-constraint when build isolation is enabled. | |
| # | |
| # Passing '--build-constraint' and '--no-build-isolation` together results in an error from 'pip', | |
| # but we want to keep environment variable PIP_CONSTRAINT set unconditionally. | |
| # PIP_NO_BUILD_ISOLATION=0 means "add --no-build-isolation" (ref: https://github.com/pypa/pip/issues/5735) | |
| if [[ "${PIP_NO_BUILD_ISOLATION:-}" != "0" ]]; then | |
| RAPIDS_PIP_WHEEL_ARGS+=(--build-constraint="${PIP_CONSTRAINT}") | |
| fi |
🤖 Prompt for AI Agents
In `@ci/build_wheel.sh` around lines 32 - 39, Update the incorrect GitHub issue
reference in the comment that explains PIP_NO_BUILD_ISOLATION behavior: replace
the URL and issue number referencing "#573" with the correct issue "#5735" (the
comment near the PIP_NO_BUILD_ISOLATION check that mentions PIP_CONSTRAINT,
RAPIDS_PIP_WHEEL_ARGS, and --build-constraint). Ensure the comment text still
accurately describes that PIP_NO_BUILD_ISOLATION=0 means "add
--no-build-isolation" and that the URL now points to the correct GitHub issue
discussing that behavior.
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.
Good rabbit!
That's totally correct, it should end with
(ref: https://github.com/pypa/pip/issues/5735)
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.
@jameslamb, thank you for confirming! I'm glad I could help catch that incorrect reference. Let me know if you need anything else! 🐰
(ᵔᴥᵔ)
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.
This is all looking good. Checked the logs and for other uses of PIP_CONSTRAINT, no issues.
Let's merge it, that incorrect issue link in a comment can be fixed as part of rapidsai/build-planning#244 the next time we need to touch this repo for some other build/packaging thing.
|
/merge |
Description
Contributes to rapidsai/build-planning#242
Modifying
ci/build_wheel.shto--build-constraint="${PIP_CONSTRAINT}"unless build isolation is enabled.PIP_CONSTRAINT(set by rapids-init-pip)... it doesn't affect builds as of pip 25.3, and results in an error frompip wheelwhen set and--build-constraintis also passedSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.