Skip to content

Conversation

@mmccarty
Copy link
Contributor

@mmccarty mmccarty commented Jan 16, 2026

Description

Contributes to rapidsai/build-planning#242

Modifying ci/build_wheel.sh to

  • pass--build-constraint="${PIP_CONSTRAINT}" unless build isolation is enabled.
  • unset PIP_CONSTRAINT (set by rapids-init-pip)... it doesn't affect builds as of pip 25.3, and results in an error from pip wheel when set and --build-constraint is also passed

Summary by CodeRabbit

  • Chores
    • Updated pip wheel build configuration to consolidate invocation arguments and improve constraint handling during the wheel creation process.
    • Refined build documentation comments for clarity on constraint behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

The 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

Cohort / File(s) Summary
Pip wheel argument handling
ci/build_wheel.sh
Introduces RAPIDS_PIP_WHEEL_ARGS array containing pip wheel options (-w dist, -v, --no-deps, --disable-pip-version-check, --extra-index-url). Adds conditional logic to append --build-constraint when PIP_NO_BUILD_ISOLATION is not "0". Unsets PIP_CONSTRAINT environment variable to prevent conflicts. Replaces direct argument list with array expansion.
Documentation clarification
ci/build_wheel_cuopt.sh
Updates inline comments to clarify that PIP_CONSTRAINT is set by rapids-init-pip and constrains subsequent pip commands, excluding those in pip wheel invocations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: updating wheel build scripts to handle pip's new handling of build constraints, which aligns with the modifications to ci/build_wheel.sh and ci/build_wheel_cuopt.sh.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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.

Comment on lines +32 to +39
# 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
Copy link

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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_ISOLATION behaves 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:

    1. 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)
    2. 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)
    3. 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

    1. Prefer the explicit CLI flag to avoid ambiguity: pip install --no-build-isolation . (pip.pypa.io)
    2. 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)
    3. 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)
    4. 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)
    5. 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:


🏁 Script executed:

find . -name "build_wheel.sh" -type f

Repository: 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.sh

Repository: 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.

Suggested change
# 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.

Copy link
Member

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)

Copy link

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 jameslamb added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Jan 16, 2026
Copy link
Member

@jameslamb jameslamb left a 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.

@jameslamb
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 1df9dcb into NVIDIA:main Jan 16, 2026
198 of 199 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants