Skip to content

Commit 6bcfeed

Browse files
Fix pip constraints files broken by link pins (#740) (#1726)
* Start fix for #740 Assisted-by: Claude Code:claude-opus-5-5 * Warn when a pip constraints file gets a link pin A common pip layout reuses requirements.txt as constraints, e.g. a requirements-dev.txt with `-c requirements.txt`. Hosted and vendored modes rewrite the patched pin there into a link (a URL or a vendored wheel path), which pip 20.3.x and 21.0.x refuse in a constraints file ("Links are not allowed as constraints"). Installs on those pips then failed with no hint from the scan. Both modes still rewrite the pin, but now name every sibling file that reads an edited requirements file as constraints: redirect_requirements_constraints_link (hosted) and vendor_requirements_constraints_link (vendored, dry run included). The `-c` reader shares pip's option grammar with the `-r` include reader, so `-c a -r b` still follows only `b`, as pip does. Fixes #740 Assisted-by: Claude Code:claude-opus-5-5 * Keep warning about -c readers on every run Review follow-ups for the constraints-link warning: - Hosted and vendored re-runs keep naming each `-c` reader while the link pin stays in place, not only the run that writes it. The hosted check now runs once after the rewrite settles, from the confirmed requirements pins, with each package named once. - A vendored dry run takes its target files from the real plan (the planner's files, or the superseded entry's), so it matches the wet run for uv / Pipenv exports and re-wires. - Readers are also found in other top-level directories when named like requirements (docs/requirements.txt), skipping hidden and dependency dirs; files without `-c` bytes are not parsed. - `-r` and `-c` targets resolve through one helper, and an empty `--constraint=` no longer swallows the next word. Assisted-by: Claude Code:claude-opus-5-5 * Test constraints warning on re-runs Assisted-by: Claude Code:claude-opus-5-5 * Warn about -c readers on in-sync sibling runs Bugbot follow-ups: an already-wired uv / Pipenv lock whose exported requirements.txt is wired (or, in a dry run, would be) now names its -c readers too, and so does an in-sync run that rebuilds a missing wheel. Assisted-by: Claude Code:claude-opus-5-5 * Document docs/ constraint readers in the CLI contract constraint_readers_in also scans requirements-named files in other top-level directories (docs/requirements.txt, the Read the Docs layout), but both contract rows said only the root and requirements*/ dirs were searched, so a user reading the contract would not expect the warning for a docs/ reader. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6395098 commit 6bcfeed

5 files changed

Lines changed: 650 additions & 10 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,6 +1623,7 @@ command's integration tests check its output.
16231623
| `vendor_artifact_missing` | reason | The recorded artifact is missing; repair requires an online exact redownload. |
16241624
| `vendor_artifact_corrupt` | reason | The artifact does not match the recorded hash or inventory; repair requires an online exact redownload. |
16251625
| `vendor_artifact_reused` | `skipped` (verbose note) | vendor / scan `--mode vendored` (pypi): the wiring was dropped by a relock but the committed wheel the ledger vouches for verified, so it was re-wired as-is — no service download, no rebuild; the lock pins the first run's sha again. |
1626+
| `vendor_requirements_constraints_link` | `skipped` (advisory) | vendor / scan `--mode vendored` (pypi / requirements.txt), dry run included: a requirements file the run wired (or would wire) a vendored wheel path into is read as `-c` / `--constraint` by a sibling requirements file (a `*.txt` / `*.in` in the project root or a root `requirements*/` directory, or a requirements-named `*.txt` / any `*.in` in another top-level directory, such as `docs/requirements.txt`; hidden and dependency directories are skipped). pip 20.3.x and 21.0.x refuse a link in a constraints file ("Links are not allowed as constraints"), so installing with that file fails on those pips. One advisory per reading file; the package is still vendored (#740). |
16261627
| `vendor_redownload_failed` | `failed` | vendor: the same-UUID artifact could not be downloaded and verified against its original ledger. Existing files and fingerprints are preserved. |
16271628
| `vendor_artifact_redownload_failed` | `failed` | repair: download unavailable, integrity mismatch, or downloaded bytes/inventory differ from the ledger. Existing files are preserved. |
16281629
| `vendor_artifact_unrepairable` | `failed` | repair: the ledger identity or patch record cannot be trusted or recovered. |
@@ -1675,6 +1676,7 @@ command's integration tests check its output.
16751676
| `redirect_takeover_not_pinned` | `skipped` (hosted event) | scan/get `--mode hosted`: the skip reason of a retracted takeover when no rewriter warning names the cause. |
16761677
| `redirect_hatch_lock_regenerated` | `warnings[]` (warning) | scan/get `--mode hosted` (pypi / Hatch 1.17+): the project's Hatch environments are locked (`lock-envs = true`, an environment's `locked = true` or `lock-filename`), so its `pylock*.toml` is derived from pyproject and Hatch regenerates it on the next sync. The package is wired in the Hatch declarations as well as in the lock (rewriting only the lock was discarded by Hatch, #479); run `hatch dep lock` to refresh the lock (`hatch dep lock --check` reports it stale until then). Vendored routes such a project to the Hatch lane and its pip-installer refusal. |
16771678
| `redirect_requirements_direct_reference` | `warnings[]` (warning) | scan/get `--mode hosted` (pypi / requirements.txt): the root `requirements.txt` installs the patched release from a PEP 508 direct reference the user wrote (`name @ <url>` whose archive names that release: a `files.pythonhosted.org` file, a private mirror, an internal fork build, a `file://` path), not from socket-patch's own hosted artifact. It is the user's own source choice, so the line is left byte-for-byte and the package is not redirected (#542). Before v5.0 the line was swapped for the hosted build, and rollback then wrote a plain `name==version` index pin. |
1679+
| `redirect_requirements_constraints_link` | `warnings[]` (warning) | scan/get `--mode hosted` (pypi / requirements.txt): the root `requirements.txt` was rewritten, and a sibling requirements file (a `*.txt` / `*.in` in the project root or a root `requirements*/` directory, or a requirements-named `*.txt` / any `*.in` in another top-level directory, such as `docs/requirements.txt`; hidden and dependency directories are skipped) reads it as a constraints file (`-c` / `--constraint`). The rewritten pin is a URL, and pip 20.3.x and 21.0.x refuse a link in a constraints file ("Links are not allowed as constraints"), so installing with that file fails on those pips; pip 20.2 and 21.1+ accept it. One warning per reading file; the redirect still happens (#740). A constraints file named only on the pip command line is not detected. |
16781680
| `redirect_requirements_takeover_unreachable` | `warnings[]` (warning) | scan/get `--mode hosted` (pypi / requirements.txt): a vendored → hosted takeover of a package that vendored mode wired through a pin in a `-r` include, or through a `(transitive)` line it appended to the root `requirements.txt`. Hosted mode only rewrites an existing pin in the root `requirements.txt`, so the staged takeover is retracted (wet and `--dry-run`): the vendored wiring, ledger entry and wheel stay byte-identical, the purl is skipped with this code as `skipped` (hosted event), and nothing is redirected for it. Exit 0. The detail names the remedy and its reach: run `socket-patch vendor --revert` (it reverts EVERY vendored package in the project, not just this one), move the pin from the include into the root `requirements.txt` and delete it from the include (or, for a `(transitive)` line, add an exact `==` pin to the root file), then re-run `scan --mode hosted`. |
16791681
| `redirect_uv_takeover_version_unreachable` | `warnings[]` (warning) | scan/get `--mode hosted` (pypi / uv): a vendored → hosted takeover of a package whose recorded pre-vendor `uv.lock` entry is at another version than the patch (vendored uv pins the lock entry down to the patch's version, for example when the lock resolved a newer release). Reverting would bring the lock's own version back, and hosted mode only pins the version the lock resolves, so the staged takeover is retracted (wet and `--dry-run`): the vendored wiring, ledger entry and wheel stay byte-identical, the purl is skipped with this code as `skipped` (hosted event), and nothing is redirected for it. Exit 0. The detail names the remedy and its reach: run `socket-patch vendor --revert` (it reverts EVERY vendored package in the project, not just this one), make the project resolve the patch's version (for example an exact `==` requirement) and re-lock, then re-run `scan --mode hosted`. |
16801682
| `redirect_vlt_missing_sha512` / `redirect_vlt_entry_not_found` / `redirect_vlt_entry_vendored` / `redirect_vlt_unsupported_lock_key` | `warnings[]` (warning) | scan/get `--mode hosted` (vlt): the grant has no sha512 / the lock has no default-registry node for `name@version` (not emitted when `redirect_vlt_custom_registry_skipped` already covers the dep's only nodes) / the only match is a vendored `file` node under `.socket/vendor/npm/<uuid>/` / a default-registry instance is outside vlt's node-line grammar or still unpatched after the splice. Per dep; none of the dep's instances is written. `redirect_vlt_missing_sha512` and `redirect_vlt_unsupported_lock_key` refuse the dep: it is never confirmed, whichever lock drives (a sibling lock may still carry its rewritten URL). `redirect_vlt_entry_not_found` and `redirect_vlt_entry_vendored` only say `vlt-lock.json` does not wire it: while vlt drives it is not confirmed; otherwise a sibling lock's rules may confirm it. Exit 0. |

‎crates/socket-patch-cli/tests/mode_migration_pypi.rs‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,162 @@ async fn requirements_sole_pin_vendored_to_hosted() {
433433
assert_vendored_to_hosted(&root, &["requirements.txt"]).await;
434434
}
435435

436+
/// #740: a sibling requirements file that reads the root requirements.txt
437+
/// as `-c` constraints breaks on pip 20.3.x / 21.0.x once the root pin is a
438+
/// link ("Links are not allowed as constraints"). Both writers still
439+
/// rewrite the pin, but now name every such reader in a warning: hosted
440+
/// `redirect_requirements_constraints_link`, vendored (dry run and wet)
441+
/// `vendor_requirements_constraints_link`. A line that names the file as
442+
/// `-r` (or `-r` after `-c`, which pip follows instead) is not a reader,
443+
/// and a project with no reader gets no warning.
444+
#[tokio::test]
445+
async fn requirements_read_as_constraints_warn_in_both_modes() {
446+
fn stage(root: &Path, readers: bool) {
447+
std::fs::write(root.join("requirements.txt"), "six==1.16.0\n").unwrap();
448+
std::fs::write(root.join("include.txt"), "-r requirements.txt\n").unwrap();
449+
std::fs::write(
450+
root.join("both.txt"),
451+
"-c requirements.txt -r requirements.txt\n",
452+
)
453+
.unwrap();
454+
if readers {
455+
std::fs::write(
456+
root.join("requirements-dev.txt"),
457+
"-c requirements.txt\nidna==3.7\n",
458+
)
459+
.unwrap();
460+
std::fs::create_dir_all(root.join("requirements")).unwrap();
461+
std::fs::write(
462+
root.join("requirements/test.txt"),
463+
"--constraint=../requirements.txt # pins\n",
464+
)
465+
.unwrap();
466+
}
467+
}
468+
fn reported(env: &Value, code: &str) -> Vec<String> {
469+
let mut readers: Vec<String> = Vec::new();
470+
let mut walk = vec![env];
471+
while let Some(v) = walk.pop() {
472+
match v {
473+
Value::Object(map) => {
474+
if ["code", "errorCode"]
475+
.iter()
476+
.any(|key| map.get(*key).and_then(Value::as_str) == Some(code))
477+
{
478+
let text = v.to_string();
479+
for reader in [
480+
"requirements-dev.txt",
481+
"requirements/test.txt",
482+
"both.txt",
483+
"include.txt",
484+
] {
485+
if text.contains(&format!("{reader} reads requirements.txt")) {
486+
readers.push(reader.to_string());
487+
}
488+
}
489+
assert!(
490+
text.contains("Links are not allowed as constraints"),
491+
"{text}"
492+
);
493+
}
494+
walk.extend(map.values());
495+
}
496+
Value::Array(items) => walk.extend(items),
497+
_ => {}
498+
}
499+
}
500+
readers.sort();
501+
readers
502+
}
503+
let expected = vec![
504+
"requirements-dev.txt".to_string(),
505+
"requirements/test.txt".to_string(),
506+
];
507+
for readers in [true, false] {
508+
let want = if readers {
509+
expected.clone()
510+
} else {
511+
Vec::new()
512+
};
513+
514+
let (_tmp, root) = project();
515+
stage(&root, readers);
516+
let server = MockServer::start().await;
517+
mount_hosted_api(&server, true).await;
518+
let (code, env) = hosted_scan(&root, &server);
519+
assert_eq!(code, 0, "{env:#}");
520+
assert_eq!(
521+
hosted_pinned(&env),
522+
1,
523+
"the pin is still redirected: {env:#}"
524+
);
525+
assert_eq!(
526+
reported(&env, "redirect_requirements_constraints_link"),
527+
want,
528+
"{env:#}"
529+
);
530+
// A re-run rewrites nothing; the URL pin still breaks the readers.
531+
let (code, env) = hosted_scan(&root, &server);
532+
assert_eq!(code, 0, "{env:#}");
533+
assert_eq!(
534+
reported(&env, "redirect_requirements_constraints_link"),
535+
want,
536+
"re-run: {env:#}"
537+
);
538+
539+
let (_tmp, root) = project();
540+
stage(&root, readers);
541+
stage_manifest(&root);
542+
let (code, env) = run_cli(&root, &["vendor", "--dry-run"], &[]);
543+
assert_eq!(code, 0, "{env:#}");
544+
assert_eq!(
545+
reported(&env, "vendor_requirements_constraints_link"),
546+
want,
547+
"dry run: {env:#}"
548+
);
549+
assert!(
550+
!readers || env.to_string().contains("would pin six==1.16.0"),
551+
"{env:#}"
552+
);
553+
let (code, env) = run_cli(&root, &["vendor"], &[]);
554+
assert_eq!(code, 0, "{env:#}");
555+
assert!(std::fs::read_to_string(root.join("requirements.txt"))
556+
.unwrap()
557+
.contains(&format!(".socket/vendor/pypi/{UUID}/")));
558+
assert_eq!(
559+
reported(&env, "vendor_requirements_constraints_link"),
560+
want,
561+
"{env:#}"
562+
);
563+
assert!(
564+
!readers || env.to_string().contains("now pins six==1.16.0"),
565+
"{env:#}"
566+
);
567+
let (code, env) = run_cli(&root, &["vendor"], &[]);
568+
assert_eq!(code, 0, "{env:#}");
569+
assert_eq!(
570+
reported(&env, "vendor_requirements_constraints_link"),
571+
want,
572+
"in-sync re-run: {env:#}"
573+
);
574+
// An in-sync run that rebuilds a missing wheel keeps the wiring too.
575+
std::fs::remove_dir_all(root.join(format!(".socket/vendor/pypi/{UUID}"))).unwrap();
576+
let (code, env) = run_cli(&root, &["vendor"], &[]);
577+
assert_eq!(code, 0, "{env:#}");
578+
assert!(
579+
env["events"]
580+
.as_array()
581+
.is_some_and(|events| events.iter().any(|e| e["action"] == "rebuilt")),
582+
"{env:#}"
583+
);
584+
assert_eq!(
585+
reported(&env, "vendor_requirements_constraints_link"),
586+
want,
587+
"in-sync rebuild: {env:#}"
588+
);
589+
}
590+
}
591+
436592
/// #410: a requirements.txt in which every requirement is the hosted pin
437593
/// (a lone `six==1.16.0`, or one beside `-e .`) can be unwound again.
438594
/// Hosted `rollback`, `remove` and the hosted → vendored takeover restore

‎crates/socket-patch-core/src/hosted/engine.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use crate::constants::npm_family::{
2828
RUSH_COMMON_LOCK_REL, RUSH_SUBSPACES_DIR, VLT_HIDDEN_LOCK_REL, VLT_LOCK,
2929
};
3030
use crate::formats::cargo::source_replacement::CratesIoSource;
31+
use crate::formats::governing_locks::PYPI_REQUIREMENTS;
3132
use crate::patch::redirect::npmrc::{
3233
effective_replace_registry_host, plan_npmrc_allow_remote_with, replace_registry_host_rewrites,
3334
NpmrcPlan, OuterAllowRemote, NPMRC_ALLOW_REMOTE_EDIT_KIND, NPMRC_REL,
@@ -1765,6 +1766,8 @@ pub async fn rewrite(
17651766
)
17661767
.await;
17671768
if vetoed.is_empty() {
1769+
let constraints = constraints_link_warnings(view, &done, &kept).await;
1770+
done.rewrite.warnings.extend(constraints);
17681771
done.unattributed = unattributed;
17691772
done.rewrite.warnings.extend(lockless);
17701773
done.rewrite.warnings.extend(copy_left);
@@ -1777,6 +1780,48 @@ pub async fn rewrite(
17771780
}
17781781
}
17791782

1783+
/// A `redirect_requirements_constraints_link` warning for each sibling
1784+
/// requirements file that reads the root requirements.txt as `-c`
1785+
/// constraints while a hosted pin there is a URL (#740), which pip 20.3
1786+
/// and 21.0 refuse in a constraints file. Said on every run that confirms
1787+
/// such a pin, not only the one that writes it: the hazard lasts as long
1788+
/// as the pin does.
1789+
async fn constraints_link_warnings(
1790+
view: &ProjectView<'_>,
1791+
done: &Rewritten,
1792+
kept: &[Candidate],
1793+
) -> Vec<RewriteWarning> {
1794+
let confirmed = &done.rewrite.confirmed_requirements_uuids;
1795+
let mut pinned: Vec<&str> = kept
1796+
.iter()
1797+
.filter(|c| confirmed.contains(&c.dep.patch_uuid))
1798+
.map(|c| c.dep.name.as_str())
1799+
.collect();
1800+
pinned.sort_unstable();
1801+
pinned.dedup();
1802+
if pinned.is_empty() {
1803+
return Vec::new();
1804+
}
1805+
crate::vendor::pypi_requirements::constraint_readers_in(*view, &[PYPI_REQUIREMENTS.to_string()])
1806+
.await
1807+
.into_iter()
1808+
.map(|found| {
1809+
warning(
1810+
"redirect_requirements_constraints_link",
1811+
format!(
1812+
"{} reads {} as a constraints file (`-c`), and the hosted pin of {} in {} is a \
1813+
URL; {}",
1814+
found.reader,
1815+
found.target,
1816+
pinned.join(", "),
1817+
found.target,
1818+
crate::vendor::pypi_requirements::CONSTRAINT_LINK_REMEDY
1819+
),
1820+
)
1821+
})
1822+
.collect()
1823+
}
1824+
17801825
/// Lockfile discovery over the project as `done` would leave it, read as
17811826
/// the management commands read it ([`HostedInventory`]): the skips for
17821827
/// the confirmed candidates whose pin would be contested wiring, a

0 commit comments

Comments
 (0)