Skip to content

Commit 5ddf5e0

Browse files
Fix vendored uv drift-keep after uv remove (#1287) (#1337)
* Start v5 blocker fix (uv-transitive-drift) Empty commit to open the draft PR. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Unwind vendored uv transitive deps after uv remove After `uv remove <parent>` of a vendored transitive package, uv drops the package's [[package]] unit but keeps the `[tool.uv]` override and source and the lock's `[manifest]` records socket-patch wrote, which still name the vendored wheel. The revert treated any surviving reference as the user's, so the vanished unit read as drift and everything was kept. `vendor --check` then stayed red, and its `scan --prune` remedy (and `vendor --revert`, `remove`, `rollback`) changed nothing. uv projects: references inside this entry's own still-verbatim fragments no longer count. The vanished unit is reported as `vendor_lock_entry_removed` and the surviving records are restored. PEP 723 script locks: a vendored unit that has left the lock, with no remaining dependent and no declaration in the script, is dropped from the recorded documents before the document restore. A user edit that still routes through the wheel is still drift. Fixes #1287 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent da800c5 commit 5ddf5e0

5 files changed

Lines changed: 390 additions & 19 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,11 @@ worse, lets a warm cache silently serve unpatched bytes):
833833
that no longer exists at all — the user removed the dependency — is not drift: it warns
834834
`vendor_lock_entry_removed` and the artifact and entry are kept unless every wired file that exists
835835
was read and none mentions the uuid in any spelling (an unreadable lock keeps them), so `rollback` / `remove` / `scan --prune` clean up
836-
after `npm uninstall` / `yarn remove` / `pnpm remove` / `bun remove`), removes the artifacts, prunes the
836+
after `npm uninstall` / `yarn remove` / `pnpm remove` / `bun remove`; in uv projects and PEP 723
837+
script locks the same holds after `uv remove` of the package, and after `uv remove` of the parent
838+
of a TRANSITIVE vendored package, whose `[tool.uv]` override + source and `[manifest]` records
839+
uv leaves in place: those are socket-patch's own records, which the revert restores, so only a
840+
reference outside them counts as drift, #1287), removes the artifacts, prunes the
837841
ledger, sweeps orphan uuid dirs, and (v5.0) prunes the now-empty `.socket/vendor/<eco>/` and
838842
`.socket/vendor/` levels — `.socket/` itself is removed by the lock guard when nothing else is
839843
left. It works without a manifest: with no manifest and no ledger it is a clean exit-0 no-op.

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,24 @@ fn uv_vendor_revert_after_uv_remove() {
12161216
);
12171217
}
12181218

1219+
/// #1287: six vendored TRANSITIVELY (through `[tool.uv]
1220+
/// override-dependencies` + sources, here beside a user
1221+
/// `constraint-dependencies` pin), then `uv remove python-dateutil` drops
1222+
/// six's `[[package]]` unit but leaves the `[tool.uv]` lines and the lock's
1223+
/// `[manifest]` records socket-patch wrote. `vendor --revert` must read the
1224+
/// vanished unit as removed and unwind the rest, instead of drift-keeping
1225+
/// everything (which left `vendor --check` red with a prune remedy that
1226+
/// changed nothing).
1227+
#[test]
1228+
#[serial_test::serial]
1229+
fn uv_vendor_revert_after_uv_remove_of_the_transitive_parent() {
1230+
uv_relock_then_revert(
1231+
"uv-remove-parent",
1232+
"[project]\nname = \"vendor-capstone\"\nversion = \"0.1.0\"\nrequires-python = \">=3.9\"\ndependencies = [\"python-dateutil==2.8.2\", \"attrs>=20\"]\n\n[tool.uv]\nconstraint-dependencies = [\"six==1.16.0\"]\n",
1233+
&["remove", "-q", "python-dateutil"],
1234+
);
1235+
}
1236+
12191237
/// #821: six in a PEP 735 dev group, then `uv add --dev zipp` rewrites the
12201238
/// whole `requires-dev` group line.
12211239
#[test]

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

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,67 @@ fn uv_remove_script_six(root: &Path) {
852852
/// `vendor --check` stayed red and its own `scan --prune` remedy looped.
853853
#[tokio::test]
854854
async fn script_lock_unwinds_after_uv_remove_script() {
855+
assert_script_lock_unwinds(stage_script_lock, uv_remove_script_six).await;
856+
}
857+
858+
/// A PEP 723 script whose six arrives only TRANSITIVELY (through
859+
/// python-dateutil), with its `.py.lock`; returns its wiring files.
860+
fn stage_transitive_script_lock(root: &Path) -> &'static [&'static str] {
861+
std::fs::write(
862+
root.join("job.py"),
863+
"# /// script\n# requires-python = \">=3.9\"\n# dependencies = [\"python-dateutil==2.8.2\"]\n# ///\nimport six\n",
864+
)
865+
.unwrap();
866+
std::fs::write(
867+
root.join("job.py.lock"),
868+
format!(
869+
"version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n\n[manifest]\nrequirements = [{{ name = \"python-dateutil\", specifier = \"==2.8.2\" }}]\n\n[[package]]\nname = \"python-dateutil\"\nversion = \"2.8.2\"\nsource = {{ registry = \"https://pypi.org/simple\" }}\ndependencies = [{{ name = \"six\" }}]\nwheels = [{{ url = \"https://files.pythonhosted.org/python_dateutil-2.8.2-py2.py3-none-any.whl\", hash = \"sha256:{}\" }}]\n\n[[package]]\nname = \"six\"\nversion = \"1.16.0\"\nsource = {{ registry = \"https://pypi.org/simple\" }}\nwheels = [{{ url = \"https://files.pythonhosted.org/six-1.16.0-py2.py3-none-any.whl\", hash = \"sha256:{WHEEL_SHA}\" }}]\n",
870+
"d".repeat(64)
871+
),
872+
)
873+
.unwrap();
874+
&["job.py", "job.py.lock"]
875+
}
876+
877+
/// What `uv remove --script job.py python-dateutil` leaves of the vendored
878+
/// [`stage_transitive_script_lock`] (checked against uv 0.11.19): the
879+
/// dependency and both lock units go, while the script's `[tool.uv]`
880+
/// override + source and the lock's `[manifest] overrides` socket-patch
881+
/// wrote stay, still naming the vendored wheel.
882+
fn uv_remove_script_parent(root: &Path) {
883+
let script = std::fs::read_to_string(root.join("job.py")).unwrap();
884+
std::fs::write(
885+
root.join("job.py"),
886+
script.replace("\"python-dateutil==2.8.2\"", ""),
887+
)
888+
.unwrap();
889+
let lock = std::fs::read_to_string(root.join("job.py.lock")).unwrap();
890+
let overrides = lock
891+
.lines()
892+
.find(|line| line.starts_with("overrides = "))
893+
.expect("the vendored lock carries the override");
894+
std::fs::write(
895+
root.join("job.py.lock"),
896+
format!(
897+
"version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n\n[manifest]\n{overrides}\n"
898+
),
899+
)
900+
.unwrap();
901+
}
902+
903+
/// #1287: the script lane of a vendored TRANSITIVE package whose parent
904+
/// `uv remove --script` dropped: every unwind retires the entry instead of
905+
/// drift-keeping it, and `vendor --check` turns green.
906+
#[tokio::test]
907+
async fn transitive_script_lock_unwinds_after_uv_remove_of_its_parent() {
908+
assert_script_lock_unwinds(stage_transitive_script_lock, uv_remove_script_parent).await;
909+
}
910+
911+
/// Vendor the script lock `stage` writes, apply `remove` (a `uv remove
912+
/// --script`), then every unwind must retire the entry (see
913+
/// [`script_lock_unwinds_after_uv_remove_script`]). Files the unwind
914+
/// restores must no longer name the vendored wheel.
915+
async fn assert_script_lock_unwinds(stage: StageFn, remove: fn(&Path)) {
855916
let server = MockServer::start().await;
856917
mount_hosted_api(&server, true).await;
857918
let uri = server.uri();
@@ -876,13 +937,16 @@ async fn script_lock_unwinds_after_uv_remove_script() {
876937
hosted_scan_args(&uri),
877938
] {
878939
let (_tmp, root) = project();
879-
let files = stage_script_lock(&root);
940+
let files = stage(&root);
880941
vendor_project(&root, files);
881-
uv_remove_script_six(&root);
942+
remove(&root);
882943
let removed: Vec<String> = files
883944
.iter()
884945
.map(|f| std::fs::read_to_string(root.join(f)).unwrap())
885946
.collect();
947+
// What remains after the unwind: the user's own content, with any
948+
// surviving socket-patch wiring gone.
949+
let transitive = removed.iter().any(|text| text.contains(UUID));
886950
let (code, env) = run_cli(&root, &["vendor", "--check"], &[]);
887951
assert_eq!(code, 1, "{unwind:?}: the removal is flagged first: {env:#}");
888952

@@ -915,11 +979,15 @@ async fn script_lock_unwinds_after_uv_remove_script() {
915979
std::fs::read_to_string(root.join(".socket/vendor/state.json")).unwrap_or_default();
916980
assert!(!ledger.contains(UUID), "{unwind:?}: {ledger}");
917981
for (f, text) in files.iter().zip(&removed) {
918-
assert_eq!(
919-
&std::fs::read_to_string(root.join(f)).unwrap(),
920-
text,
921-
"{unwind:?}: {f} stays as uv left it"
922-
);
982+
let after = std::fs::read_to_string(root.join(f)).unwrap();
983+
if transitive {
984+
assert!(
985+
!after.contains(UUID) && !after.contains("override"),
986+
"{unwind:?}: {f} is unwired:\n{after}"
987+
);
988+
} else {
989+
assert_eq!(&after, text, "{unwind:?}: {f} stays as uv left it");
990+
}
923991
}
924992
// `vendor --revert` and `rollback` keep the manifest record, so
925993
// check then reports the patch as not vendored; the unwinds that

‎crates/socket-patch-core/src/vendor/pypi_lock.rs‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,74 @@ pub(super) fn still_references_artifact(restored: &str, original: &str, uuid: &s
720720
restored.contains(&needle) && !original.contains(&needle)
721721
}
722722

723+
/// Whether `name` (PEP 503) left the live lock `text` altogether: no unit
724+
/// of that name, and no unit lists it among its `dependencies` (#1287).
725+
/// For a script lock, `script` is the live script, which must not declare
726+
/// it either. That is a dependency the user dropped (`uv remove --script`
727+
/// of the parent of a transitive package), not a hand edit of its unit.
728+
fn package_vanished(text: &str, name: &str, script: Option<&str>) -> bool {
729+
let Ok(doc) = text.parse::<DocumentMut>() else {
730+
return false;
731+
};
732+
let (collection, _) = crate::utils::python_lock::lock_package_collection(&doc);
733+
let names = |item: Option<&Item>| -> bool {
734+
item.and_then(Item::as_array).is_some_and(|deps| {
735+
deps.iter().any(|dep| {
736+
dep.as_inline_table()
737+
.and_then(|t| t.get("name"))
738+
.and_then(Value::as_str)
739+
.is_some_and(|n| canonicalize_pypi_name(n) == name)
740+
})
741+
})
742+
};
743+
let in_lock = doc
744+
.get(collection)
745+
.and_then(Item::as_array_of_tables)
746+
.is_some_and(|packages| {
747+
packages.iter().any(|unit| {
748+
unit.get("name")
749+
.and_then(Item::as_str)
750+
.is_some_and(|n| canonicalize_pypi_name(n) == name)
751+
|| names(unit.get("dependencies"))
752+
})
753+
});
754+
let declared = script.is_some_and(|script| {
755+
script_metadata(script).ok().is_some_and(|(_, metadata)| {
756+
metadata.parse::<DocumentMut>().ok().is_some_and(|doc| {
757+
doc.get("dependencies")
758+
.and_then(Item::as_array)
759+
.is_some_and(|deps| {
760+
deps.iter().filter_map(Value::as_str).any(|spec| {
761+
canonicalize_pypi_name(crate::vendor::common::pep508_name(spec)) == name
762+
})
763+
})
764+
})
765+
})
766+
});
767+
!in_lock && !declared
768+
}
769+
770+
/// `text` with every lock unit named `name` (PEP 503) dropped: the vendored
771+
/// unit of a dependency that has since left the lock, which the document
772+
/// restore then no longer tries to pair (#1287).
773+
fn without_package(text: &str, name: &str) -> Result<String, String> {
774+
let mut doc: DocumentMut = text
775+
.parse()
776+
.map_err(|error| format!("invalid recorded TOML: {error}"))?;
777+
let (collection, _) = crate::utils::python_lock::lock_package_collection(&doc);
778+
if let Some(packages) = doc
779+
.get_mut(collection)
780+
.and_then(Item::as_array_of_tables_mut)
781+
{
782+
packages.retain(|unit| {
783+
unit.get("name")
784+
.and_then(Item::as_str)
785+
.is_none_or(|n| canonicalize_pypi_name(n) != name)
786+
});
787+
}
788+
Ok(doc.to_string())
789+
}
790+
723791
pub(super) async fn revert_python_locks(
724792
entry: &VendorEntry,
725793
root: &Path,
@@ -752,6 +820,25 @@ pub(super) async fn revert_python_locks(
752820
Err((_, error)) => return RevertOutcome::failed(error),
753821
}
754822
}
823+
// The vendored package's PEP 503 name, and each script lock's live
824+
// script (read once, before any record is reverted).
825+
let package_name = entry
826+
.base_purl
827+
.strip_prefix("pkg:pypi/")
828+
.and_then(|rest| rest.rsplit_once('@'))
829+
.map(|(name, _)| canonicalize_pypi_name(name));
830+
let mut script_texts = std::collections::BTreeMap::new();
831+
for record in entry.wiring.iter().filter(|r| r.kind == KIND) {
832+
if let Some(script) = record
833+
.file
834+
.strip_suffix(".lock")
835+
.filter(|s| s.ends_with(".py"))
836+
{
837+
if let Ok(text) = read_file(&root.join(script)).await {
838+
script_texts.insert(record.file.as_str(), text);
839+
}
840+
}
841+
}
755842
for record in entry.wiring.iter().rev() {
756843
if !allowed_file(&record.file, &record.kind) {
757844
warnings.push(VendorWarning::new(
@@ -788,6 +875,40 @@ pub(super) async fn revert_python_locks(
788875
Ok(live) => live,
789876
Err((_, error)) => return RevertOutcome::failed(error),
790877
};
878+
// #1287: the vendored package's own unit is gone from the lock along
879+
// with every dependent (`uv remove` of the parent of a transitive
880+
// package), while the overrides it was wired through survive. That
881+
// unit has nothing left to restore: drop it from both recorded
882+
// documents so the rest of the record reverts normally.
883+
let (original_owned, new_owned);
884+
let (original, new) = if record.kind == KIND
885+
&& package_name.as_deref().is_some_and(|name| {
886+
package_vanished(
887+
&live,
888+
name,
889+
script_texts.get(record.file.as_str()).map(String::as_str),
890+
)
891+
}) {
892+
let name = package_name.as_deref().unwrap_or_default();
893+
match (without_package(original, name), without_package(new, name)) {
894+
(Ok(o), Ok(n)) => {
895+
warnings.push(VendorWarning::new(
896+
super::LOCK_ENTRY_REMOVED_CODE,
897+
format!(
898+
"{}: the {name} unit no longer exists and nothing depends on it \
899+
(the dependency was removed); its other wiring is restored",
900+
record.file
901+
),
902+
));
903+
original_owned = o;
904+
new_owned = n;
905+
(original_owned.as_str(), new_owned.as_str())
906+
}
907+
(Err(error), _) | (_, Err(error)) => return RevertOutcome::failed(error),
908+
}
909+
} else {
910+
(original, new)
911+
};
791912
let restored = if record.kind == SCRIPT_KIND {
792913
(|| {
793914
if live == new || live == original {
@@ -1710,6 +1831,71 @@ mod tests {
17101831
(script.to_string(), lock.to_string())
17111832
}
17121833

1834+
/// A script whose `one` arrives only TRANSITIVELY (through `parent`),
1835+
/// vendored: wired through the script's `[tool.uv]` override + source
1836+
/// and the lock's `[manifest] overrides`.
1837+
async fn vendor_transitive_script_pair(root: &Path) -> (VendorEntry, String, String) {
1838+
let lock = "version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n\n[manifest]\nrequirements = [{name = \"attrs\", specifier = \">=20\"}, {name = \"parent\", specifier = \"==1\"}]\n\n[[package]]\nname = \"attrs\"\nversion = \"25.3.0\"\nsource = {registry = \"https://pypi.org/simple\"}\n\n[[package]]\nname = \"one\"\nversion = \"1\"\nsource = {registry = \"https://pypi.org/simple\"}\n\n[[package]]\nname = \"parent\"\nversion = \"1\"\nsource = {registry = \"https://pypi.org/simple\"}\ndependencies = [{name = \"one\"}]\n";
1839+
let script = "# /// script\n# requires-python = \">=3.9\"\n# dependencies = [\"parent==1\", \"attrs>=20\"]\n# ///\nimport one\n";
1840+
write_pylock(root, "job.py.lock", lock).await;
1841+
tokio::fs::write(root.join("job.py"), script).await.unwrap();
1842+
let project = load_python_locks(root, "one", "1", UUID).await.unwrap();
1843+
let wheel =
1844+
".socket/vendor/pypi/11111111-1111-4111-8111-111111111111/one-1-py3-none-any.whl";
1845+
let records = wire_python_locks(&project, root, "one", "1", wheel, &"a".repeat(64))
1846+
.await
1847+
.unwrap();
1848+
let entry: VendorEntry = serde_json::from_value(serde_json::json!({
1849+
"ecosystem": "pypi",
1850+
"basePurl": "pkg:pypi/one@1",
1851+
"uuid": UUID,
1852+
"artifact": { "path": wheel, "sha256": "a".repeat(64) },
1853+
"wiring": serde_json::to_value(&records).unwrap(),
1854+
"flavor": "python-lock",
1855+
}))
1856+
.unwrap();
1857+
let wired_script = std::fs::read_to_string(root.join("job.py")).unwrap();
1858+
let wired_lock = std::fs::read_to_string(root.join("job.py.lock")).unwrap();
1859+
(entry, wired_script, wired_lock)
1860+
}
1861+
1862+
/// #1287: `uv remove --script job.py parent` drops the transitive
1863+
/// `one`'s unit (and its parent's) but keeps the script's `[tool.uv]`
1864+
/// override + source and the lock's `[manifest] overrides`, which still
1865+
/// name the uuid. The vanished unit is removed, not drift, and the
1866+
/// surviving wiring reverts, so the script and lock end up as uv writes
1867+
/// them for the project without `one`.
1868+
#[tokio::test]
1869+
async fn script_revert_after_uv_remove_of_the_transitive_parent() {
1870+
let temp = tempfile::tempdir().unwrap();
1871+
let root = temp.path();
1872+
let (entry, wired_script, wired_lock) = vendor_transitive_script_pair(root).await;
1873+
let removed_script = wired_script.replace("\"parent==1\", ", "");
1874+
let mut removed_lock = wired_lock.replace(", {name = \"parent\", specifier = \"==1\"}", "");
1875+
let one = removed_lock.find("\n[[package]]\nname = \"one\"").unwrap();
1876+
removed_lock.truncate(one);
1877+
tokio::fs::write(root.join("job.py"), &removed_script)
1878+
.await
1879+
.unwrap();
1880+
write_pylock(root, "job.py.lock", &removed_lock).await;
1881+
1882+
let outcome = revert_python_locks(&entry, root, false).await;
1883+
assert!(outcome.success, "{outcome:?}");
1884+
assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings);
1885+
assert!(outcome.lock_entry_removed(), "{:?}", outcome.warnings);
1886+
let script = std::fs::read_to_string(root.join("job.py")).unwrap();
1887+
let lock = std::fs::read_to_string(root.join("job.py.lock")).unwrap();
1888+
assert_eq!(
1889+
script,
1890+
"# /// script\n# requires-python = \">=3.9\"\n# dependencies = [\"attrs>=20\"]\n# ///\nimport one\n"
1891+
);
1892+
assert!(
1893+
!lock.contains(UUID) && !lock.contains("overrides"),
1894+
"{lock}"
1895+
);
1896+
assert!(lock.contains("name = \"attrs\""), "{lock}");
1897+
}
1898+
17131899
/// #1214: after `uv remove --script` drops the vendored dependency from a
17141900
/// PEP 723 script and its lock, the revert has nothing left to restore.
17151901
/// It must warn `vendor_lock_entry_removed` and finish (the caller then

0 commit comments

Comments
 (0)