Skip to content

Commit 08ebe64

Browse files
mikolalysenkoclaude
andcommitted
Name the real vendor dir in composer hints
After a vendored or hosted Composer run, the "remove the package dir before composer install" hints always said vendor/<vendor>/<name>. In a project with config.vendor-dir (or COMPOSER_VENDOR_DIR) the package is installed elsewhere, so users deleted a path that does not exist, composer install did nothing, and the package stayed unpatched. The hints now resolve the vendor dir the same way the composer crawler does (COMPOSER_VENDOR_DIR, else config.vendor-dir, else vendor), and print e.g. `remove lib/acme/tool first`. A config value the crawler refuses prints a <vendor-dir> placeholder rather than a wrong path. Fixes #658 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent a8e9397 commit 08ebe64

7 files changed

Lines changed: 340 additions & 48 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ to **six flavors**.
747747
| npm / vlt (`vlt-lock.json`, lockfileVersion 0 or 1 — A0 locks without a version and every other version refuse `vendor_lockfile_version_unsupported`; flavor `vlt`) | patched package **directory** `.socket/vendor/npm/<uuid>/[@scope/]<name>-<version>/node_modules/<name>/` (the extra `node_modules/<name>` level lets a package `require()` its own name), its `package.json` without `devDependencies`, plus `<uuid>/.gitignore` (re-includes the payload against the project's ignores, ignores vlt's links inside it) and `<uuid>/.gitattributes` (`-text`) | direct dependencies of the root or a workspace member only: the lock node becomes a `file` node for the directory, its importer edges and outgoing edges are re-keyed, and each importer's `package.json` spec becomes `file:<path>`; every moved entry lands where vlt's serializer puts it. A node whose only extra is one peer context (`ṗ:N`, `peer.N`, `peer.<16 hex>`: from vlt 1.0.8 a root dependency with resolved peers, from rc.15 a workspace member's) becomes a `file` node without the extra, as vlt writes `file:` dependencies, keeping its peer edges; revert restores the extra-bearing DepID. Refused before any write: transitive targets (`vendor_vlt_transitive_unsupported`), two or more instances of one `name@version` or a modifier extra, importer `peer` edges, foreign registries, a git, remote-tarball or local-directory node of the same package name (vlt records no version for it), a package `vlt build` would build in place (`vendor_vlt_build_scripts_unsupported`), a name declared in several dependency fields (`vendor_lock_entry_unsupported`), a spec that disagrees with the lock (`vendor_vlt_lock_out_of_sync`), a payload git would ignore (`vendor_artifact_gitignored`), a purl vendored under another flavor (`vendor_flavor_changed`); era-A locks warn `vendor_vlt_legacy_lockfile`; an optional dependency (or any dependency node_modules still links to its installed upstream copy) gets `vendor_vlt_reinstall_required` | fresh checkout, `vlt ci` with cold caches: the patched bytes load and `vlt-lock.json` stays byte-identical, also through a warm and a cold `vlt install --frozen-lockfile` (checked on 1.2.0, 1.0.10, 1.0.4, 1.0.0-rc.32 and 1.0.0-rc.14, and on every release by `docs/testing/vlt-compatibility.md`); no-op installs, `vlt install <new>`, `uninstall` and `vlt update` keep the direct dependency vendored. `vendor --revert` restores the registry node, edges and specs, keeping what vlt re-laid since, and refuses on drift |
748748
| cargo | crate dir `<name>-<version>/` (no `.cargo-checksum.json`) | (v5.0) `[patch.crates-io]` path entry in the **workspace-root `Cargo.toml`** (the manifest beside the `Cargo.lock` it detaches — never `.cargo/config*`) **+** Cargo.lock surgery (the `[[package]]` entry's `source`/`checksum` removed and its `version` set to the copy's TAGGED version `<version>+socket.<uuid>` — `<core>+<meta>.socket.<uuid>` when the version already has build metadata — with every lock reference that spells the old version rewritten, formats v1–v4; the copy's own `Cargo.toml` version carries the same tag, so the patched crate sees it in `CARGO_PKG_VERSION`; revert restores the lock byte for byte). Key: always the Socket-owned `<name>-socket-<first 8 hex of the uuid>` with `package = "<name>"` (the full uuid hex when that key is taken), never the bare crate name — cargo lets a config-file `[patch]` item (project, ancestor directory or `$CARGO_HOME`) replace the manifest item with the same key whatever its version, so keys any of those configs use are avoided and a re-run moves an entry off a now-shadowed key; two versions of one crate are wired side by side. Pre-v5 wiring in `.cargo/config.toml` / `.cargo/config` is moved into `Cargo.toml` by a re-run (`vendor`, `scan`/`get --mode vendored`) or `repair` (`cargo_wiring_migrated` note; the ledger's `cargo_patch_entry` record then names `Cargo.toml`); a detached lock entry left unwired by the pre-v5 multi-version overwrite is re-wired the same way (`cargo_wiring_restored`); every revert removes both spellings | `cargo build --locked --offline` on a fresh checkout — single-version manifest `[patch]` also builds with no network on cargo older than 1.56 (the old config-file wiring's floor); two vendored versions of ONE crate need cargo 1.45 or newer (`--offline` from an empty CARGO_HOME is enough there); older cargo fails closed whatever the index state, and a project that does not pin cargo ≥ 1.45 (`rust-version` or toolchain file) gets the `cargo_multi_version_old_cargo` warning. Note: path deps build **without** `--cap-lints allow` |
749749
| golang | module dir `<module>@<version>/` | `go.mod` `replace <module> <ver> => ./.socket/vendor/golang/<uuid>/<module>@<ver>` | `go build` with `GOPROXY=off` + empty `GOMODCACHE` (directory replaces bypass go.sum entirely; survives `go mod tidy`) |
750-
| composer | package dir `<vendor>/<name>@<version>/`; the copy's `.gitignore` / `.hgignore` are emptied and its `.gitattributes` `export-ignore` rules dropped, because Composer's path mirror skips the files they match (`vendor_composer_mirror_filters_neutralized`; a patch that rewrites one of them is refused `vendor_composer_mirror_filter_conflict`). Re-runs heal copies vendored before this | `composer.lock` only: entry's `dist` → `{type: "path", url, reference: "<patch-uuid>"}`, `source` removed, `transport-options: {symlink: false}` added. `content-hash` unaffected; `composer.json` untouched | `composer install` (from the lock alone, real copy not symlink, works under `--network none`). Composer 1 does not reinstall an already-installed package whose dist changed: remove `vendor/<vendor>/<name>` first. `composer update <pkg>` reverts it. See `docs/testing/composer-compatibility.md` |
750+
| composer | package dir `<vendor>/<name>@<version>/`; the copy's `.gitignore` / `.hgignore` are emptied and its `.gitattributes` `export-ignore` rules dropped, because Composer's path mirror skips the files they match (`vendor_composer_mirror_filters_neutralized`; a patch that rewrites one of them is refused `vendor_composer_mirror_filter_conflict`). Re-runs heal copies vendored before this | `composer.lock` only: entry's `dist` → `{type: "path", url, reference: "<patch-uuid>"}`, `source` removed, `transport-options: {symlink: false}` added. `content-hash` unaffected; `composer.json` untouched | `composer install` (from the lock alone, real copy not symlink, works under `--network none`). Composer 1 does not reinstall an already-installed package whose dist changed: remove `<vendor-dir>/<vendor>/<name>` first (the hint names the project's resolved vendor dir: `COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`). `composer update <pkg>` reverts it. See `docs/testing/composer-compatibility.md` |
751751
| gem | gem dir `<name>-<version>/` + gemspec materialized from `specifications/` | **Gemfile + Gemfile.lock pair**: the `gem` line gains `path:` (or a managed block for transitive deps); the lock's spec block moves GEM→PATH and the DEPENDENCIES entry becomes `<name> (= <ver>)!`, in bundler's exact canonical form | `bundle install` (normal **and** `BUNDLE_FROZEN=true`), byte-stable lock. Lock-only edits are a silent unpatch — hence the mandatory pair |
752752
| pypi / uv (uv.lock) | rebuilt wheel (canonical PEP 427 filename; RECORD regenerated) | `[tool.uv.sources] <name> = {path}` in pyproject + surgical uv.lock rewrite; transitive deps via `[tool.uv] override-dependencies` | `uv sync --locked` / `--frozen --offline` (hash-verified, byte-stable lock) |
753753
| pypi / poetry (poetry.lock: legacy `[metadata.hashes]`, lock 1.0/1.1 `[metadata.files]`, 2.x `files`) | (rebuilt wheel) | lock-only: the target `[[package]]` gets `[package.source] type="file"` (+ `reference = ""` on the 0.12/1.0 layouts, which read it unconditionally) and the single `{file, hash: sha256-of-our-wheel}` entry in whichever table the generation keeps it. pyproject + `metadata.content-hash` untouched; CRLF locks keep their line endings. A lock written by Poetry < 1.4 emits `pypi_poetry_integrity_unverified` (that installer verifies no local hashes and skips an already-installed version) | `poetry check --lock && poetry sync`, cold cache (hash fail-closed from Poetry 1.4; byte-stable lock) — see `docs/testing/poetry-compatibility.md` |

‎crates/socket-patch-cli/src/commands/composer_hints.rs‎

Lines changed: 115 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,40 @@
77
//! dist reference to the patch uuid; a hosted redirect keeps it and drops
88
//! the entry's `source`, so an entry that had no `source` changes nothing
99
//! Composer compares. Composer 1 never reinstalls a changed package. In both
10-
//! cases `vendor/<vendor>/<name>` must be removed first.
10+
//! cases `<vendor-dir>/<vendor>/<name>` must be removed first, where
11+
//! `<vendor-dir>` is the directory Composer installs into
12+
//! (`COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`): a hint
13+
//! naming `vendor/` in a project that relocated it sends the user to delete
14+
//! a path that does not exist, and `composer install` then leaves the
15+
//! package unpatched (#658).
16+
17+
use std::path::Path;
1118

1219
use serde_json::{Map, Value};
1320
use socket_patch_core::patch::redirect::FileEdit;
1421

22+
/// The project's Composer vendor directory as the hints print it: relative
23+
/// to `cwd` with `/` separators when it lies inside the project (`vendor`,
24+
/// `lib`, `deps/php`), else the absolute path `COMPOSER_VENDOR_DIR` named.
25+
/// A `config.vendor-dir` the crawler refuses (absolute, or climbing out of
26+
/// the project) prints as the `<vendor-dir>` placeholder rather than a
27+
/// directory that would be wrong.
28+
pub(crate) fn vendor_dir_label(cwd: &Path) -> String {
29+
let Some(dir) =
30+
socket_patch_core::crawlers::composer_crawler::resolve_local_vendor_dir_sync(cwd)
31+
else {
32+
return "<vendor-dir>".to_string();
33+
};
34+
match dir.strip_prefix(cwd) {
35+
Ok(rel) if !rel.as_os_str().is_empty() => rel
36+
.components()
37+
.map(|c| c.as_os_str().to_string_lossy())
38+
.collect::<Vec<_>>()
39+
.join("/"),
40+
_ => dir.display().to_string(),
41+
}
42+
}
43+
1544
const VENDOR_PREFIX: &str = ".socket/vendor/composer/";
1645

1746
/// The `<vendor>/<name>` of every composer.lock package (`packages` and
@@ -45,15 +74,21 @@ pub(crate) fn vendored_composer_packages(lock_text: &str) -> Vec<String> {
4574

4675
/// The lines to print after a vendored run that left `packages` wired, or
4776
/// nothing when none are.
48-
pub(crate) fn vendored_reinstall_hints(packages: &[String]) -> Vec<String> {
77+
/// `vendor_dir` is the [`vendor_dir_label`] the package dirs are under.
78+
pub(crate) fn vendored_reinstall_hints(packages: &[String], vendor_dir: &str) -> Vec<String> {
4979
if packages.is_empty() {
5080
return Vec::new();
5181
}
52-
let dirs: Vec<String> = packages.iter().map(|p| format!("vendor/{p}")).collect();
82+
let dirs: Vec<String> = packages
83+
.iter()
84+
.map(|p| format!("{vendor_dir}/{p}"))
85+
.collect();
5386
vec![
54-
"Run `composer install` to update vendor/ — vendoring rewires composer.lock only, so \
55-
the installed vendor/ tree keeps the unpatched bytes until Composer reinstalls it."
56-
.to_string(),
87+
format!(
88+
"Run `composer install` to update {vendor_dir}/ — vendoring rewires composer.lock \
89+
only, so the installed {vendor_dir}/ tree keeps the unpatched bytes until Composer \
90+
reinstalls it."
91+
),
5792
format!(
5893
"Composer 1 does not reinstall a locked package whose dist changed: remove {} \
5994
first, then run `composer install`.",
@@ -65,30 +100,34 @@ pub(crate) fn vendored_reinstall_hints(packages: &[String]) -> Vec<String> {
65100
/// The reinstall example for a hosted run's next steps when it rewrote
66101
/// `composer.lock`. `edits` are the run's rewrite edits: a redirected entry
67102
/// whose edit removed no `source` must have its vendor dir removed on every
68-
/// Composer version.
69-
pub(crate) fn hosted_reinstall_hint(files: &[String], edits: &[FileEdit]) -> Option<String> {
103+
/// Composer version. `vendor_dir` is the [`vendor_dir_label`] the package
104+
/// dirs are under.
105+
pub(crate) fn hosted_reinstall_hint(
106+
files: &[String],
107+
edits: &[FileEdit],
108+
vendor_dir: &str,
109+
) -> Option<String> {
70110
if !files.iter().any(|f| f == "composer.lock") {
71111
return None;
72112
}
73113
let mut unchanged: Vec<String> = edits
74114
.iter()
75115
.filter(|e| e.path == "composer.lock" && e.kind == "redirect_composer_dist")
76116
.filter(|e| !removes_source(e))
77-
.filter_map(|e| Some(format!("vendor/{}", e.key.as_deref()?.to_lowercase())))
117+
.filter_map(|e| Some(format!("{vendor_dir}/{}", e.key.as_deref()?.to_lowercase())))
78118
.collect();
79119
unchanged.sort_unstable();
80120
unchanged.dedup();
81121
if unchanged.is_empty() {
82-
return Some(
122+
return Some(format!(
83123
" (e.g. `composer install`; on Composer 1 first remove the patched packages' \
84-
vendor/<vendor>/<name> directories)"
85-
.to_string(),
86-
);
124+
{vendor_dir}/<vendor>/<name> directories)"
125+
));
87126
}
88127
Some(format!(
89128
" (e.g. `composer install`; first remove {} — Composer does not reinstall a package \
90129
whose lock entry has no `source` when only its dist url changes — and on Composer 1 \
91-
every other patched package's vendor/<vendor>/<name> directory)",
130+
every other patched package's {vendor_dir}/<vendor>/<name> directory)",
92131
unchanged.join(", ")
93132
))
94133
}
@@ -146,8 +185,8 @@ mod tests {
146185

147186
#[test]
148187
fn vendored_hints_name_every_package_dir() {
149-
assert!(vendored_reinstall_hints(&[]).is_empty());
150-
let hints = vendored_reinstall_hints(&["a/b".to_string(), "c/d".to_string()]);
188+
assert!(vendored_reinstall_hints(&[], "vendor").is_empty());
189+
let hints = vendored_reinstall_hints(&["a/b".to_string(), "c/d".to_string()], "vendor");
151190
assert_eq!(hints.len(), 2);
152191
assert!(
153192
hints[0].starts_with("Run `composer install`"),
@@ -163,8 +202,8 @@ mod tests {
163202

164203
#[test]
165204
fn hosted_hint_needs_a_rewritten_composer_lock() {
166-
assert!(hosted_reinstall_hint(&["package-lock.json".to_string()], &[]).is_none());
167-
let hint = hosted_reinstall_hint(&["composer.lock".to_string()], &[]).unwrap();
205+
assert!(hosted_reinstall_hint(&["package-lock.json".to_string()], &[], "vendor").is_none());
206+
let hint = hosted_reinstall_hint(&["composer.lock".to_string()], &[], "vendor").unwrap();
168207
assert!(hint.contains("composer install") && hint.contains("Composer 1"));
169208
}
170209

@@ -200,14 +239,70 @@ mod tests {
200239
&format!(r#"{NEW_DIST}, "extra": {{"source": {{}}}}"#),
201240
);
202241
for edits in [vec![with_source.clone()], vec![source_after_extra]] {
203-
let hint = hosted_reinstall_hint(&files, &edits).unwrap();
242+
let hint = hosted_reinstall_hint(&files, &edits, "vendor").unwrap();
204243
assert!(hint.contains("on Composer 1 first remove"), "{hint}");
205244
}
206245

207246
let dist_only = dist_edit("Acme/Lib", OLD_DIST, NEW_DIST);
208-
let hint = hosted_reinstall_hint(&files, &[with_source, dist_only]).unwrap();
247+
let hint =
248+
hosted_reinstall_hint(&files, &[with_source.clone(), dist_only.clone()], "vendor")
249+
.unwrap();
209250
assert!(hint.contains("first remove vendor/acme/lib —"), "{hint}");
210251
assert!(!hint.contains("vendor/psr/log"), "{hint}");
211252
assert!(hint.contains("on Composer 1"), "{hint}");
253+
254+
// A relocated vendor dir is the one named (#658).
255+
let hint = hosted_reinstall_hint(&files, &[with_source, dist_only], "lib").unwrap();
256+
assert!(hint.contains("first remove lib/acme/lib —"), "{hint}");
257+
assert!(
258+
hint.contains("patched package's lib/<vendor>/<name>"),
259+
"{hint}"
260+
);
261+
assert!(!hint.contains("vendor/"), "{hint}");
262+
let hint = hosted_reinstall_hint(&files, &[], "lib").unwrap();
263+
assert!(
264+
hint.contains("remove the patched packages' lib/<vendor>/<name>"),
265+
"{hint}"
266+
);
267+
}
268+
269+
/// #658: with `config.vendor-dir: lib` the vendored hints name
270+
/// `lib/<vendor>/<name>`, the directory Composer installed into.
271+
#[test]
272+
fn vendored_hints_name_a_relocated_vendor_dir() {
273+
let hints = vendored_reinstall_hints(&["acme/tool".to_string()], "lib");
274+
assert!(hints[0].contains("update lib/"), "{}", hints[0]);
275+
assert!(
276+
hints[1].contains("remove lib/acme/tool first"),
277+
"{}",
278+
hints[1]
279+
);
280+
assert!(!hints.join("\n").contains("vendor/"), "{hints:?}");
281+
}
282+
283+
/// The label follows Composer's own resolution: `config.vendor-dir`
284+
/// (normalized, `/`-separated) else `vendor`; a refused value prints a
285+
/// placeholder instead of a wrong path. `COMPOSER_VENDOR_DIR` is left
286+
/// unset by the test environment, as everywhere in this crate.
287+
#[test]
288+
fn vendor_dir_label_follows_config_vendor_dir() {
289+
let label = |json: Option<&str>| {
290+
let dir = tempfile::tempdir().unwrap();
291+
if let Some(json) = json {
292+
std::fs::write(dir.path().join("composer.json"), json).unwrap();
293+
}
294+
vendor_dir_label(dir.path())
295+
};
296+
assert_eq!(label(None), "vendor");
297+
assert_eq!(label(Some("{}")), "vendor");
298+
assert_eq!(label(Some(r#"{"config":{"vendor-dir":"lib"}}"#)), "lib");
299+
assert_eq!(
300+
label(Some(r#"{"config":{"vendor-dir":"./deps\\php/"}}"#)),
301+
"deps/php"
302+
);
303+
assert_eq!(
304+
label(Some(r#"{"config":{"vendor-dir":"../outside"}}"#)),
305+
"<vendor-dir>"
306+
);
212307
}
213308
}

0 commit comments

Comments
 (0)