Skip to content

Studio: keep the operator's package-manager policy in the installer - #10902

Open
danielhanchen wants to merge 19 commits into
mainfrom
studio-installer-keeps-operator-pm-policy
Open

Studio: keep the operator's package-manager policy in the installer#10902
danielhanchen wants to merge 19 commits into
mainfrom
studio-installer-keeps-operator-pm-policy

Conversation

@danielhanchen

@danielhanchen danielhanchen commented Sep 14, 2026

Copy link
Copy Markdown
Member

The problem

#8579 kept the installer working on a machine with a hardened uv.toml / pip.conf (#8530), but it did it with a blanket amnesty. Every pinned-index command dropped no-build, only-binary and exclude-newer out of the environment and ran with PIP_CONFIG_FILE at devnull and UV_NO_CONFIG=1, so an operator's build-time code execution controls were discarded along with the index settings the pin actually needed gone. A compromised or malicious index could then get a source build executed on a host whose owner had explicitly forbidden exactly that.

The fix narrows the relaxation to the one control the installer genuinely cannot satisfy, and leaves the rest alone.

In plain terms

Some people, mostly on locked-down work machines, tell pip and uv "never build a package from source, only use prebuilt ones, and go through this company proxy with this certificate". Those settings live in a config file.

Our installer sometimes has to fetch a very specific build of PyTorch from a very specific URL. To stop a stale company mirror from hijacking that, the installer switched the config file off for those commands. The problem is that switching the file off threw out everything in it, including the "never build from source" rule and the company certificate. So on those machines the installer was quietly doing the one thing the owner had forbidden, and at the same time losing the certificate it needed to reach the company's own server.

This PR keeps the part that has to go (the index settings, which is what hijacks the pin) and puts back the parts that never should have gone: the source-build ban, the certificate, the proxy, the trusted hosts and the timeouts. Then it does the same for uv, which ignores pip's settings entirely and so had to be handed the rule directly on its command line.

If you have never configured pip or uv, which is almost everyone, nothing about your install changes at all. That is measured, not assumed: see the byte-for-byte comparison below.

What changes

Control Before Now
PIP_REQUIRE_HASHES / UV_REQUIRE_HASHES cleared cleared (unchanged)
PIP_NO_BINARY / UV_NO_BINARY cleared cleared (unchanged)
PIP_ONLY_BINARY cleared for pinned installs honoured
UV_EXCLUDE_NEWER cleared for pinned installs cleared (see below)
pip.conf policy on pinned installs dropped with the config file only-binary re-asserted as an env var
pip.conf transport on pinned installs dropped with the config file cert, client-cert, proxy, trusted-host, timeout, retries, keyring-provider re-asserted
Relaxation trigger any argv TOKEN equal to install, download or wheel the pip SUBCOMMAND

Hash enforcement stays off: every requirements file we ship is pinned but unhashed, so require-hashes has no artifact to check against and can only abort the install. no-binary stays off for pinned commands because it would force a source build of a wheel the pin exists to fetch, which is less build-time execution, not more.

UV_EXCLUDE_NEWER stays cleared, which is the pre-existing behaviour, because only one of the two tools reads it. uv honours it, _build_pip_cmd never adds pip's --uploaded-prior-to, and pip_install falls back to pip whenever uv fails. Honouring it on the uv leg alone would mean a pinned install that fell back quietly installed an artifact the cutoff forbids, which is the outcome _uv_upload_cutoff_args exists to prevent. Consistently off is the honest answer until the fallback can carry it.

PIP_CONFIG_FILE still points at os.devnull, because that is the only spelling that stops a site or global pip.conf reaching a pinned install. What devnull switches off is now put back one key at a time by _pinned_pip_config_overrides(), which reads pip config list once per run and translates an allowlist into PIP_ environment variables. The transport half of that list also fixes a pre-existing bug: devnull used to drop the CA, proxy and trusted-host that a private index needs, so a pinned torch repair on a corporate host could not fetch what the pin named.

For uv there is deliberately no equivalent. Measured against uv 0.10.7, UV_NO_BUILD, UV_NO_BINARY and UV_ONLY_BINARY are not uv environment variables at all: uv reads no-build from its config file, which the pin has to disable, so there is no way to carry it onto a pinned command and no point pretending otherwise. Nothing is lost, because every pinned index we install from serves wheels. A uv.toml no-build still applies to every non-pinned command, which is where source builds actually happen.

Deliberately unchanged

The package-scoped --no-binary exemptions for the four audited wheel-less requirements and for the diffusers source archive. They are per-package, they are ratified by the nobuild allowlists in CI, and removing them is what re-breaks #8530.

Defects this branch caught in itself

Nine, all in code added here, all fixed on the branch with a test each. The largest is below. The rest: the exec-extraction in test_xpu_triton_swap.py stopped pulling the parser once it was split out, so the "real scrub" that file executes returned {} for every case while still reporting 62 passed; an over-broad except Exception was what hid it, and on a corporate host would have turned any programming error into a silent loss of cert / proxy / trusted-host; [wheel] and [download] sections could overwrite [install], downgrading an install-wide only-binary = :all: to one package; the memoisation cached failures, so one transient miss cost the operator their transport settings for the whole run; whitespace collapsing corrupted C:\Program Files\ca.pem and turned a two-package only-binary into one bogus name (pip reads that key comma separated, measured); pip --cache-dir X install y read as not-an-install and would have lost the hash relaxation; and UV_EXCLUDE_NEWER was being honoured on a leg that cannot carry it.

The regression that mattered most

The first pass pointed PIP_CONFIG_FILE at a rewritten config instead of devnull, on the assumption that it would suppress the other config files. It does not. pip documents that only os.devnull "disables the loading of all configuration files", and that naming an existing file skips the per-user file alone. Measured on pip 26.2 with a venv-level pip.conf:

PIP_CONFIG_FILE=<rewritten file>  pip install --index-url <pin> packaging==24.0
  ERROR: No matching distribution found for packaging==24.0     # site no-index still live
PIP_CONFIG_FILE=/dev/null         pip install --index-url <pin> packaging==24.0
  Would install packaging-24.0

That would have broken every torch repair, the XPU triton fetch and the torchao pin on any host with a site or global pip.conf. It is reverted, and tests/python/test_cross_platform_parity.py now asserts the devnull spelling with that reason attached.

Evidence

All of the following runs are real pip and real uv in throwaway uv venv sandboxes, driving the actual child command with the environment the installer builds.

Behaviour, against a site pip.conf carrying require-hashes, only-binary = :all:, no-index, extra-index-url, cert and trusted-host:

pinned install survives the site no-index                        PASS
pinned install refuses an sdist under only-binary                PASS   <- the fix
non-pinned install passes require-hashes (#8530 stays fixed)     PASS
non-pinned install still refuses an sdist                        PASS
raw pip with the same config is refused (control)                PASS
package-scoped --no-binary still builds the exempt sdist         PASS
pinned env keeps cert / proxy / trusted-host / timeout           PASS
pinned env never re-asserts index-url, no-index, no-binary,
  require-hashes                                                 PASS
a venv with no pip yet yields no overrides                       PASS
uv: a non-pinned command still honours uv.toml no-build          PASS
uv: the package-scoped exemption still overrides it              PASS
27/27

Differential against origin/main, 9 environments (clean, corporate transport, user mirror, uv index and backend, hardened hashes, hardened build policy, fully hardened, no-index env, hardened config FILE) by 10 command shapes:

identical environments: 78
intended differences:   12      (hardened hosts keeping their own policy)
unexpected differences:  0

On a machine with no package-manager policy, which is nearly every machine, the environment handed to every child process is byte-identical to main.

Old installs, real installs on each pair, since an existing Studio venv can be years old:

py3.9 / pip 21.3.1     py3.9 / pip 23.3.2     py3.10 / pip 24.0
py3.11 / pip 25.2      py3.13 / pip 26.2.1
config parses, pinned install works, only-binary still enforced,
non-pinned install passes require-hashes, unknown PIP_ vars ignored
30/30

Static audit and matrix: no pinned-index call site installs a path, sdist, archive or VCS ref, so leaving only-binary in force on pinned commands cannot fail one; the narrowed subcommand test classifies every real pip command in the module exactly as the old substring test did; 12 platform x accelerator combinations ([Windows, Linux, WSL, macOS] x [NVIDIA, AMD, CPU]) hold every invariant; the env builder is pure and memoises its one subprocess; degenerate commands cannot raise. 15/15.

Unit tests: tests/python, tests/studio -> 10768 passed. The 2 failures in test_torchcodec_torch_compat.py reproduce on unmodified main and are unrelated.

Does this change anything for a normal install or update

The honest answer, with the one exception stated rather than buried.

Structural containment (AST, not eyeballing). Comparing every module-level definition in studio/install_python_stack.py between base and head, with docstrings stripped so prose does not read as code:

added        16     the new helpers and their constants
changed       4     _install_env_for_cmd, _build_uv_cmd, _relaxed_pip_policy_env,
                    _pip_config_without_sources
removed       1     _PM_POLICY_ENV_VARS, replaced by the two narrower tuples
docstring     1     _pytorch_whl_leaf_url, whose text said this function strips pip.conf
UNTOUCHED   392     byte-identical ASTs

Every changed helper is private to that file. Nothing outside it imports them; the Studio backend only mentions the module in comments.

Routing equivalence, every call site. All 48 pip_install / pip_install_try call sites classify identically before and after, non-pinned to non-pinned and pinned to pinned. The old predicate was a whole-token test (any(arg in ("install", "download", "wheel") for arg in cmd)), and across the whole module it disagrees with the new structural test on exactly three commands:

pip uninstall -y wheel       main: relaxed require-hashes   head: does not
pip uninstall -y download    main: relaxed require-hashes   head: does not
pip uninstall -y install     main: relaxed require-hashes   head: does not

Those are reachable only with STUDIO_PACKAGE_NAME set to one of those three names. Measured on pip 26.2: pip uninstall ignores PIP_REQUIRE_HASHES completely, so the difference is inert. It is listed because a known difference beats a surprise.

Environment equality. Across 9 environments by 10 command shapes, the environment handed to every child process is byte-identical to main in 78 of 80 combinations. The 12 intended differences are all on hosts that configured a policy, which is the entire point of the PR.

The one real difference on an unconfigured machine. A pinned command now runs pip config list once per process to find out whether there is a policy to preserve. Measured here:

first pinned call (includes the one pip config list):  134 ms
every subsequent pinned call (memoised):                0.05 ms
non-pinned call (no subprocess at all):                 0.05 ms

So an install pays roughly a tenth of a second, once, and only if it issues a pinned command at all. A pip that hangs is budgeted at two attempts of 30 seconds for the whole run, not per command. A pip that is simply absent, which is normal early in a fresh venv, answers instantly and does not spend that budget, so the read still succeeds once pip exists.

Can only-binary make a pinned install fail that used to succeed? This is the one place the PR can change an outcome, so it is measured rather than argued. None of the pinned repair calls pass --no-deps, so it is the whole dependency CLOSURE that has to be wheels, not just torch. Resolved against the real indexes with and without --only-binary=:all::

cpu       14 packages    identical
cu126     33 packages    identical
cu128     33 packages    identical
rocm6.3   15 packages    identical
xpu       38 packages    identical

So on the indexes this installer actually pins, the operator's policy costs the pin nothing. On a private mirror that serves an sdist-only dependency, a host that configured only-binary would now abort where it used to build. That is the operator's stated policy being honoured rather than silently bypassed, which is the entire point of the PR, and it cannot happen on a machine that configured no policy.

Update from an existing install. unsloth studio update runs setup.sh / setup.ps1, which runs this module, so the update path is in scope and was tested as such: old venvs on pip 21.3.1, 23.3.2, 24.0, 25.2 and 26.2.1 across Python 3.9 to 3.13, 30 of 30 checks. Shortcuts, login and the running app do not execute this module at all. Notebooks install through pip install unsloth and never reach it.

Scope

studio/install_python_stack.py is run by setup.sh / setup.ps1 / install.sh / install.ps1, that is Studio and Desktop install and update. No frontend file is touched, so there is no UI surface and no browser behaviour to re-check. Notebooks install via pip install unsloth and never execute this module. The helpers changed here are private to the file; nothing else imports them.

#8579 made the installer's own pip and uv commands survive a hardened
uv.toml / pip.conf, but it did that with a blanket amnesty: every pinned
install dropped no-build, only-binary and exclude-newer from the
environment and ran with PIP_CONFIG_FILE at devnull and UV_NO_CONFIG=1,
so the operator's build-time code execution controls went with it.

Narrow that to the one thing the installer actually cannot satisfy.

* Hash enforcement (PIP_REQUIRE_HASHES / UV_REQUIRE_HASHES) is still
  cleared: every requirements file we ship is pinned but unhashed, so
  require-hashes has no artifact to check and can only abort the install.
* no-binary is still cleared for pinned commands, because it would FORCE
  a source build of a wheel the pin exists to fetch. That is less
  build-time execution, not more.
* UV_NO_BUILD, UV_NO_BUILD_PACKAGE, PIP_ONLY_BINARY and UV_EXCLUDE_NEWER
  are now left in force. Every pinned index we install from serves
  wheels, so honouring them costs the pin nothing.
* PIP_CONFIG_FILE points at a rewrite of pip's own config minus the four
  source keys (the existing _pip_config_without_sources) instead of
  devnull, so cert, proxy, trusted-host and only-binary survive while the
  pin still gets the index determinism it needed. Computed once per run.
* UV_NO_CONFIG=1 has to stay (uv has no per-key override and a discovered
  uv.toml outranks the CLI pin), so a no-build in that file is read back
  and re-asserted as UV_NO_BUILD=1 rather than silently lost.
* The hash relaxation is keyed on the pip SUBCOMMAND now, not on the word
  "install" appearing somewhere in argv.

Measured against a real pip.conf carrying require-hashes, only-binary,
no-index and cert: raw pip refuses the install, the installer path
installs the wheel, the installer path REFUSES the same package when only
an sdist is available, and a pinned --index-url install succeeds with
cert and only-binary intact.

The package-scoped --no-binary exemptions for the four audited wheel-less
requirements and the diffusers source archive are unchanged; they are
what keeps #8530 fixed.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T13:51:55.513541Z 49d8533 Manual request
🔒 Security Review Completed 2026-09-14T04:35:29.649150Z 3bdad8b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Simulation caught a real regression in the first pass of this branch. Pointing
PIP_CONFIG_FILE at a rewritten config does NOT suppress a SITE or GLOBAL
pip.conf: pip documents, and pip 26.2 confirms, that naming an existing file
skips the per-user file only, while os.devnull is what "disables the loading of
all configuration files". So a venv-level or /etc-level `no-index` came back to
life and killed the pinned install:

  pip install --index-url <pin> packaging==24.0   ->  No matching distribution

That would have broken every torch repair, the XPU triton fetch and the torchao
pin on any host with a site or global pip.conf. Reverted to os.devnull and put
the operator's settings back one key at a time instead:

* _pinned_pip_config_overrides() reads `pip config list` once per run and
  re-asserts an ALLOWLIST as PIP_ environment variables: cert, client-cert,
  proxy, trusted-host, timeout, retries, keyring-provider and only-binary. The
  transport half also fixes a pre-existing bug, since devnull used to drop the
  CA and proxy a private index needs.
* index-url, extra-index-url, find-links and no-index are never re-asserted
  (the pin replaces them), nor is no-binary (it would force a source build of a
  pinned wheel), nor require-hashes (unsatisfiable against unhashed
  requirements).
* Only global/install/download/wheel sections are read, so a `list.format` does
  not become a global PIP_ variable, and `:env:` rows are skipped so the child
  cannot re-inherit the variables the scrub just cleared.

Also dropped the uv side of the previous commit. Measured against uv 0.10.7,
UV_NO_BUILD, UV_NO_BINARY and UV_ONLY_BINARY are not uv environment variables at
all: uv reads no-build from its config file, and re-asserting the variable was
dead code promising a guarantee uv does not honour. UV_EXCLUDE_NEWER and
UV_REQUIRE_HASHES are real and are handled as before. A uv.toml no-build still
applies to every non-pinned command, which is where source builds actually
happen.

_executable_stem() splits on both separators, so a Windows argv0 is recognised
off-Windows too, and a degenerate command can no longer raise out of the
subcommand test.

Verified: 78 of 90 base-vs-head environment comparisons are byte-identical and
the other 12 are the intended hardened-host cases; real installs across
pip 21.3.1 to 26.2.1 and Python 3.9 to 3.13; a site-config no-index no longer
breaks a pinned install while only-binary still refuses an sdist.
danielhanchen added a commit to shimmyshimmer/unsloth-staging-4 that referenced this pull request Sep 14, 2026
danielhanchen added a commit to shimmyshimmer/unsloth-staging-4 that referenced this pull request Sep 14, 2026
danielhanchen added a commit to danielhanchen/unsloth-staging-2 that referenced this pull request Sep 14, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

Cross-platform validation status, for the record.

The change is pure Python in the installer, and the only platform-dependent parts of it are the devnull spelling (os.devnull, so nul on Windows), argv0 parsing (now split on both separators), and pip config list availability. All three are covered by the simulations in the description, including the 12 platform x accelerator combinations and a Windows-shaped argv table.

Staged the branch for real CI on ubuntu-latest, ubuntu-24.04-arm, windows-latest, windows-11-arm, macos-15 (Apple Silicon) and macos-13 (Intel), plus the full install/update suite. The hosted runner queue is saturated, so those are still pending; I will follow up here with the results rather than claim them now.

One job has finished: windows-11-arm failed, and it is not this change. It fails in the harness's own python -m pip install -r studio/backend/requirements/studio.txt step, before any installer code runs, on sqlite-vec==0.1.9, which publishes no win-arm64 wheel. That pin is identical on main, so the same step fails there.

Three defects found reviewing the previous commit, all in code this branch
introduced.

The `pip config list` read sits on the path to every pinned install, including
the final torch repair, so it must degrade to "no overrides" for ANY unexpected
answer and not just for a missing pip. It caught OSError and TimeoutExpired but
would have propagated anything else: a listing that is not bytes, a stdout of
None, a decode that raises. Parsing moved into _parse_pinned_pip_config() and
the whole read is now wrapped, with cases for each shape.

A command section now beats `global` for the same option, which is pip's own
precedence. It previously depended on the order `pip config list` happened to
print the two lines in, so `global.only-binary=:all:` plus
`install.only-binary=numpy` resolved differently run to run.

The tests were not hermetic. A CI image carrying its own /etc/pip.conf would
have leaked into the pinned-env assertions, and a test that mocks subprocess
could populate the memoised read with a Mock and poison whatever ran next under
random ordering. An autouse fixture clears the cache around every test and stubs
the read, with an opt-out marker for the few tests that drive it themselves.
Verified by running the three files repeatedly under random ordering.
danielhanchen added a commit to danielhanchen/unsloth-staging-2 that referenced this pull request Sep 14, 2026
All eight are in code this branch introduced.

The exec-extraction in tests/studio/test_xpu_triton_swap.py stopped pulling the
parser once it was split out, so the "real scrub" that file deliberately
executes raised NameError into a broad except and returned {} for every case:
the file still reported 62 passed while asserting nothing. The loader now
executes the extracted parser once against a known listing, so an inert
extraction fails loudly instead of agreeing with everything.

That mask was the second defect. The read caught OSError and TimeoutExpired
around the subprocess but then wrapped the parse in `except Exception`, turning
any programming error into "this operator has no pip config", which on a
corporate host is a silent loss of cert, proxy and trusted-host on every pinned
install. Narrowed to (OSError, subprocess.SubprocessError) around the call, with
the parser skipping unparseable lines as before.

Section precedence let [wheel] and [download] overwrite [install], which is not
"more specific", it is a different subcommand: an operator with
`[install] only-binary = :all:` and `[wheel] only-binary = numpy` had their
install-wide policy downgraded to one package. Only [global] and [install] are
read now, install winning, since the PIP_ variable is command-wide anyway.

The memoisation cached failures as well as successes, so one transient miss (the
60s timeout, or a read racing the pip bootstrap) cost the operator their
transport settings for the rest of the run. Only a successful read is cached.

Value joining applied one rule to keys with two. `cert` is a single value, and
collapsing whitespace turned `C:\Program  Files\ca.pem` into a path that does
not exist; `only-binary` is comma separated, so a two-package value became one
bogus name and the policy silently stopped applying. Measured against pip 26.2:
PIP_ONLY_BINARY="a,b" refuses both as sdists, PIP_TRUSTED_HOST="a b" is
accepted, and `pip config list` renders a multi-value setting on one line with
an escaped \n.

The subcommand scan skipped leading-dash tokens but not the VALUE of a global
option that takes one, so `pip --cache-dir /tmp/c install x` read as not an
install and would have lost the hash relaxation. It now looks for the first
token that names a pip subcommand.

UV_EXCLUDE_NEWER goes back to being cleared for pinned commands, which is the
pre-existing behaviour. Only uv reads it, _build_pip_cmd never adds pip's
--uploaded-prior-to, and pip_install falls back to pip whenever uv fails, so
honouring it on the uv leg alone means a pinned install that fell back quietly
installs an artifact the cutoff forbids.

And a dead REAL_PINNED_PIP_CONFIG_OVERRIDES assignment that read as if it were
the fixture's restore mechanism is gone.

Suites: 10768 passed. The 2 torchcodec failures reproduce on unmodified main.
danielhanchen added a commit to shimmyshimmer/unsloth-staging-4 that referenced this pull request Sep 14, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

Cross-platform results so far.

Per-OS runs of tests/python/test_cross_platform_parity.py, tests/python/test_install_python_stack.py and tests/studio/test_xpu_triton_swap.py, on real runners:

ubuntu-latest        success    365 passed
ubuntu-24.04-arm     success    365 passed
windows-latest       success    359 passed, 6 skipped
macos-15 (Apple Si)  success    365 passed
macos-13 (Intel)     queued

The staging install and update suite is green on the jobs that have finished, including the ones that exercise the paths this PR touches:

Unsloth Update CI                              success
Windows Unsloth Update CI                      success
Mac Unsloth Install Matrix CI                  success
Mac Unsloth UI + API + Update + Inference CI   success
Cross-platform parity                          success
Wheel CI, Lint CI, GGUF, API, Startup profile  success
Clean machine install, Core, Backend CI        queued

Those runs cover the design commit. The last two commits on the branch (the hermetic test fixture, and the eight review fixes) landed after they started, so I have re-staged all five per-OS jobs against the current head and will post the second set when they finish.

On windows-11-arm, which failed: it is not this change. It dies in the harness's own python -m pip install -r studio/backend/requirements/studio.txt, before any installer code runs, on sqlite-vec==0.1.9. PyPI publishes five artifacts for that version, macOS x86_64 and arm64, manylinux x86_64 and aarch64, and win_amd64, with no win_arm64 wheel and no sdist at all, so the requirement is unsatisfiable on Windows ARM64. The pin is identical on main, so the same step fails there. Worth its own issue, not this PR.

danielhanchen added a commit to danielhanchen/unsloth-staging-2 that referenced this pull request Sep 14, 2026
pre-commit-ci Bot and others added 2 commits September 14, 2026 07:57
Comment and docstring lines in the diff: 178 to 105. Every measured fact is
kept, in one line where it took four: the devnull suppression rule, the pip
26.2 and uv 0.10.7 measurements, the separator each config key is read with,
and the NameError trap in the exec-extraction. AST-gated comment-only.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex security review

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 14, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

pip config is per subcommand, and the re-assertion was reading [install] for
every pinned command. Measured on pip 26.2: a pip.conf carrying
[download] no-index stops a pip download and leaves pip install untouched, so
translating [install] onto _ensure_xpu_triton's pinned pip download both drops
that command's own cert, proxy and trusted-host and imposes an install-only
policy on it.

_pinned_pip_config_overrides() now takes the subcommand and reads [global] plus
that command's section, with the command section winning. _pip_subcommand_of()
resolves it from the command; a uv command defaults to install, since the PIP_
variables only ever reach its pip fallback. The listing is memoised instead of
the parsed result, so this still costs one subprocess per run.

Suites pass, and the real-pip, audit, differential and old-pip simulations are
unchanged: 27/27, 15/15, 78 identical environments with 12 intended, 30/30.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

… as set

Two ways the re-assertion could still lose the operator's policy, and one test
that could pass for the wrong reason.

pip ACCUMULATES a repeatable option across sections rather than letting the
command section replace the global one. Measured on pip 26.2: a pip.conf with
[global] only-binary = :all: plus [install] only-binary = numpy still refuses an
unrelated sdist, so emitting PIP_ONLY_BINARY=numpy dropped the :all: policy on
every pinned install. List keys now accumulate global plus the command section,
deduplicated, with the separator each key is read with; scalars still take the
command section alone, since two certs cannot be concatenated. Verified end to
end: that config now yields PIP_ONLY_BINARY=':all:,numpy' and a pinned sdist
install is refused.

An empty inherited value is no longer treated as an override. pip ignores an
empty environment value and falls through to the config file, which the pinned
branch has just removed with devnull, so PIP_CERT= in the parent environment
left the child with neither and a private index unreachable. Confirmed with
env_var:
  PIP_CACHE_DIR='/mnt/disks/unslothai/daniel1/.cache/pip'
env:
global:
  /etc/xdg/pip/pip.conf, exists: False
  /etc/pip.conf, exists: False
site:
  /mnt/disks/unslothai/daniel1/workspace_1/pip.conf, exists: False
user:
  /home/daniel1/.pip/pip.conf, exists: False
  /home/daniel1/.config/pip/pip.conf, exists: False: PIP_TIMEOUT='' shows the variable and then uses
global.timeout from the file. A whitespace value is left alone; that is a value
pip would use, not ours to second-guess.

The download-section test now clears PIP_CERT: the caller's environment
legitimately wins over the re-assertion, so on a host that exports one the test
asserted on the ambient value.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

pip applies a repeatable option IN ORDER, and :none: empties the set, so a
re-add after a reset has to survive. Deduplicating dropped it: with
[global] only-binary = a,b and [install] only-binary = :none:,a the parser
emitted a,b,:none:, which ends on the reset and allows exactly the sdist build
the operator forbade.

Measured on pip 26.2 against a venv pip.conf carrying that config: pip itself
refuses the sdist, the deduplicated string allows it, and the order-preserving
concatenation refuses it again. End to end, the installer and pip with its own
config now return the same exit code for the same install.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

pip's own parser, asked with [global] and [install] both set, resolves
trusted_hosts to the install value alone (an append option, assigned per
section) while format_control holds both (a callback that mutates in
place). Accumulating trusted-host re-trusted a host the install section
had dropped, which is a TLS decision, so the two keys are now separated:
_PINNED_PIP_CONFIG_SEPARATORS says how a value is spelled in the
environment, _PINNED_PIP_CONFIG_ACCUMULATING says which key carries
across sections, and only only-binary is in it.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 14, 2026
Backend CI's formatter fixed point check was failing on this branch. One
of the two files is mine: ruff 0.15 formats the new parser differently
from the 0.6.9 the hook pins, so what I committed was not what the hook
produces. The other, tests/python/test_docker_nvidia_toolkit_install.py,
is pre-existing drift that reached main in #10623 and is exactly the
case that test was written to catch; it is untouched by this branch
otherwise, and the change here is the hook's own output.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: f2414d6798

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector[bot]

This comment was marked as resolved.

A piped child encodes stdout with its own locale encoding, which on
Windows is the ANSI code page rather than UTF-8, so the unconditional
UTF-8 decode corrupted a cert or client-cert path spelled with non-ASCII
characters. That is worse than losing the setting: the corrupted path
exists nowhere, so pip fails the pinned install outright on exactly the
corporate host the allowlist exists to serve. UTF-8 is still tried
first and strictly, so the POSIX case and Windows UTF-8 mode are
unchanged; the locale codec is the fallback.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 14, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

…encoding

Two gaps the review found. uv reads neither pip.conf nor
PIP_ONLY_BINARY, and a pinned command runs with UV_NO_CONFIG=1 because a
discovered uv.toml outranks the CLI pin (#6898), so restoring the policy
in the environment alone left the leg that actually runs free to build a
source distribution the operator had forbidden. Measured against uv
0.10.7: a pinned uv install builds the sdist with PIP_ONLY_BINARY=:all:
set, and refuses it given --only-binary. _build_uv_cmd now translates
the restored value into uv flags for pinned commands only, since a
non-pinned one keeps its config file and uv applies the policy itself.

Second, UTF-8 first was not a reliable encoding detector: cp1252 bytes
can form valid UTF-8, so a mis-encoded cert path could decode cleanly
into the wrong path. The read now dictates PYTHONIOENCODING=utf-8 rather
than sniffing what the child wrote, at both call sites that read pip
config, and the locale decode stays only as a fallback.
danielhanchen added a commit to danielhanchen/unsloth-staging-2 that referenced this pull request Sep 14, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 3260a272f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment and docstring text only, with no code change: the AST gate
confirms it. Ten lines out, mostly from the two longest measurement
notes, with each measured fact kept.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 14, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 14, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 42222a489d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…s-operator-pm-policy

# Conflicts:
#	tests/python/test_docker_nvidia_toolkit_install.py
danielhanchen added a commit to shimmyshimmer/unsloth-staging-4 that referenced this pull request Sep 14, 2026
Three findings from an independent audit of this branch.

_build_uv_cmd asked for the pip config separately from _install_env_for_cmd,
so if the first read failed and the second succeeded the uv command went
out without --only-binary while its own environment carried
PIP_ONLY_BINARY. The flag now comes off the env that very command will
run with, so the two cannot disagree.

The 60s timeout was per attempt and unbounded across the run, because
failures are deliberately not memoised. A wedged pip therefore paid it
once per pinned command. A HANG is now budgeted, twice at 30s for the
whole run, which is what the comment always claimed.

Budgeting every failure would have been wrong in the other direction: a
fresh venv has no pip for the first part of the run, that answer is
instant, and spending the budget on it would lose the operator's cert
the moment pip appeared. Cheap failures stay freely retryable, with a
test for each direction.
danielhanchen added a commit to danielhanchen/unsloth-staging-2 that referenced this pull request Sep 14, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

Answering the "is this really zero effect" question properly, with five independent adversarial audits of this branch plus an AST containment check. Three of them found something real, and all three are fixed on the branch.

What the audits found and what changed

1. The uv flag and the environment could disagree. _build_uv_cmd asked for the pip config separately from _install_env_for_cmd. If the first read failed and the second succeeded, the uv command went out without --only-binary while its own environment carried PIP_ONLY_BINARY. The flag now comes off the env that very command runs with, so the two cannot disagree by construction.

2. A wedged pip could cost the run one timeout per pinned command. The 60 second timeout was per attempt and failures are deliberately not memoised, so the cost was unbounded across a run. A hang is now budgeted at two attempts of 30 seconds for the whole run, which is what the comment always claimed.

3. Budgeting every failure would have been wrong in the other direction. A fresh venv has no pip for the first part of the run. That answer is instant, and spending the budget on it would lose the operator's cert the moment pip appeared. Cheap failures stay freely retryable, with a test for each direction.

The audits also corrected something in my own description: the old predicate was a whole-token test, not a substring test. Fixed above.

Structural containment

Every module-level definition in studio/install_python_stack.py, base versus head, with docstrings stripped so prose does not read as code:

added        16     the new helpers and their constants
changed       4     _install_env_for_cmd, _build_uv_cmd, _relaxed_pip_policy_env,
                    _pip_config_without_sources
removed       1     _PM_POLICY_ENV_VARS, replaced by two narrower tuples
docstring     1     _pytorch_whl_leaf_url
UNTOUCHED   392     byte-identical ASTs

Routing, every call site

All 48 pip_install / pip_install_try call sites classify identically before and after, non-pinned to non-pinned and pinned to pinned. Across the whole module the old and new tests disagree on exactly three commands, all of the form pip uninstall -y <wheel|download|install>, reachable only with STUDIO_PACKAGE_NAME set to one of those three names. Measured on pip 26.2: pip uninstall ignores PIP_REQUIRE_HASHES entirely, so the difference is inert. Listed because a known difference beats a surprise.

The strongest objection, and the measurement that answers it

"The pinned indexes serve wheels" only covered torch, not its dependencies, and none of the pinned repair calls pass --no-deps. So the whole closure has to be wheels. Resolved against the real indexes with and without --only-binary=:all::

cpu       14 packages    identical
cu126     33 packages    identical
cu128     33 packages    identical
rocm6.3   15 packages    identical
xpu       38 packages    identical

The one real difference on an unconfigured machine

A pinned command now reads pip config list once per process:

first pinned call (includes the one read):  134 ms
every subsequent pinned call (memoised):      0.05 ms
non-pinned call (no subprocess at all):       0.05 ms

A tenth of a second, once, and only if the run issues a pinned command. Everything else is byte-identical: 78 of 80 environment combinations match main exactly, and the 12 intended differences are all on hosts that configured a policy.

Update path

unsloth studio update runs setup.sh / setup.ps1, which runs this module, so updates are in scope and were tested as such: old venvs on pip 21.3.1 through 26.2.1 across Python 3.9 to 3.13, 30 of 30. An update launched from an untouched older install runs that install's own installer code first, so the new code applies from the next pass onward. Shortcuts, login and the running app never execute this module; the Studio backend only mentions it in comments. Notebooks install through pip install unsloth and never reach it.

Cross-platform CI is re-running on six operating systems, including Intel macOS, macOS 26 and ARM64 Linux, which this PR does not touch.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 49d8533500

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

danielhanchen and others added 2 commits September 14, 2026 13:57
Comment text only, no code change; the AST gate confirms it.
@unslothai unslothai deleted a comment from danielhanchen Sep 14, 2026
@unslothai unslothai deleted a comment from danielhanchen Sep 14, 2026
@oobabooga

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b5bf58f21

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/install_python_stack.py Outdated
return []
# Off the env this command will run with, not a second read: two reads can disagree,
# putting the policy in the environment but not on the argv that decides.
value = (_install_env_for_cmd(cmd) or {}).get("PIP_ONLY_BINARY", "")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reuse the config read for uv's binary-policy flag

When only-binary exists only in pip.conf and this first pip config list read fails transiently, _pinned_pip_config_overrides() deliberately does not memoize the failure, so the second read in pip_install() can succeed while constructing the child environment. The resulting uv command has no --only-binary, while its environment contains only PIP_ONLY_BINARY; checked uv pip install --help, which exposes --only-binary <ONLY_BINARY> without a PIP_ONLY_BINARY environment binding, so uv can build an sdist despite the operator's policy. Compute the environment/config result once and reuse it for both argv and process execution.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixed in 90faa93. _uv_cmd_and_env() computes the environment once and derives the --only-binary flags from that same dict, and pip_install / pip_install_try run the uv command with it instead of calling _install_env_for_cmd a second time. The regression test stubs a config read that fails once and then succeeds, and asserts the argv flag matches PIP_ONLY_BINARY in the child env for both installers; it fails on 7b5bf58.

@oobabooga

Copy link
Copy Markdown
Member

@codex review

1 similar comment
@oobabooga

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 90faa93706

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants