diff --git a/crates/socket-patch-cli/tests/e2e_vendor_jvm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_jvm_build.rs index d458fb83f..f36104b75 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_jvm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_jvm_build.rs @@ -575,6 +575,171 @@ fn maven_reactor_vendor_fresh_checkout_offline_build_and_byte_exact_revert() { ); } +/// A Maven 4 (model 4.1.0) aggregator that lists no subprojects: Maven 4 +/// discovers `a` and `b` itself. +const IMPLICIT_ROOT_POM: &str = r#" + + 4.1.0 + com.example + root + 1.0.0 + pom + +"#; + +const IMPLICIT_A_POM: &str = r#" + + 4.1.0 + + a + + + org.apache.commons + commons-text + 1.10.0 + + + +"#; + +const IMPLICIT_B_POM: &str = r#" + + 4.1.0 + + b + + + com.example + a + ${project.version} + + + +"#; + +/// #622: Maven 4 builds `a` and `b` of a 4.1.0 root that lists no +/// ``. Vendoring must plan them as the reactor (suffixed +/// version in `a`, the root pinned), so the developer's own warm `~/.m2` +/// build, which already holds Central's 1.10.0, resolves the patched jar, +/// and so does a fresh offline checkout. Before the fix the root was a +/// reactor of one: `a` kept the base version and built Central's jar. +#[test] +#[ignore = "real Maven + Maven Central (fixture); run with --ignored"] +fn maven_reactor_implicit_subprojects_warm_and_offline_build_and_revert() { + const SUITE: &str = "e2e_vendor_jvm_build::maven_reactor_implicit_subprojects"; + let Some(mvn) = Mvn::detect(SUITE) else { + return; + }; + if mvn.major() < 4 { + println!( + "{SUITE}: SKIP (Maven {} has no subproject discovery)", + mvn.version + ); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = fixture_root(&tmp); + let m2 = root.join("m2"); + let proj = root.join("proj"); + let settings = root.join("settings.xml"); + write_settings(&settings, &[]); + let Some((jar, _)) = warm_fixture(SUITE, &mvn, &root.join("warm"), &m2, &settings) else { + return; + }; + for (rel, body) in [ + ("pom.xml", IMPLICIT_ROOT_POM), + ("a/pom.xml", IMPLICIT_A_POM), + ("b/pom.xml", IMPLICIT_B_POM), + ] { + let path = proj.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, body).unwrap(); + } + let out = with_central_fallback(SUITE, "pre-vendor reactor build", &settings, |s, flags| { + mvn_classpath(&mvn, &proj, &m2, s, flags) + }); + assert!(ok(&out), "pre-vendor reactor build:\n{}", dump(&out)); + assert_eq!( + std::fs::read(classpath_entry(&proj.join("b"))).unwrap(), + jar, + "pre-vendor `b` resolves Central's jar through the discovered `a`" + ); + for dir in ["target", "a/target", "b/target", ".mvn/target"] { + let _ = std::fs::remove_dir_all(proj.join(dir)); + } + let _ = std::fs::remove_dir(proj.join(".mvn")); + + let (orig, patched) = patched_member(&jar, UUID); + stage_manifest(&proj, &orig, &patched); + let before = snapshot(&proj); + let env = vendor(&proj, &m2); + assert_eq!(env["summary"]["applied"], 1, "{env}"); + let vendored = snapshot(&proj); + assert_eq!( + text(&vendored["a/pom.xml"]), + IMPLICIT_A_POM.replace( + "1.10.0", + &format!("{SV}") + ), + "the discovered module's declaration is suffixed" + ); + assert_eq!(text(&vendored["b/pom.xml"]), IMPLICIT_B_POM); + assert!( + text(&vendored["pom.xml"]).contains(&format!("{SV}")), + "the root is pinned" + ); + + // The developer's own tree: `~/.m2` still holds Central's 1.10.0. + let out = mvn_classpath(&mvn, &proj, &m2, &settings, &["-o"]); + assert!(ok(&out), "warm offline reactor build:\n{}", dump(&out)); + assert_vendored_on_classpath(&proj.join("a"), &patched, "warm build, module a"); + assert_vendored_on_classpath(&proj.join("b"), &patched, "warm build, module b"); + for dir in ["target", "a/target", "b/target", ".mvn/target"] { + let _ = std::fs::remove_dir_all(proj.join(dir)); + } + + let (code, doc, stderr) = socket_in( + &proj, + &m2, + None, + &[ + "vex", + "--cwd", + proj.to_str().unwrap(), + "--product", + "pkg:maven/com.example/root@1.0.0", + ], + ); + assert_eq!(code, Some(0), "vex: {doc}\n{stderr}"); + assert_eq!(doc["statements"][0]["status"], "not_affected", "{doc}"); + let (code, env, stderr) = socket_in( + &proj, + &m2, + None, + &[ + "vendor", + "--check", + "--json", + "--offline", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!(code, Some(0), "vendor --check: {env}\n{stderr}"); + + let fresh = root.join("fresh"); + fresh_checkout_all(&proj, &fresh); + purge(&m2); + let out = mvn_classpath(&mvn, &fresh, &m2, &settings, &["-o"]); + assert!(ok(&out), "fresh offline reactor build:\n{}", dump(&out)); + assert_vendored_on_classpath(&fresh.join("a"), &patched, "fresh build, module a"); + assert_vendored_on_classpath(&fresh.join("b"), &patched, "fresh build, module b"); + + revert(&proj, &m2); + assert_restored(&proj, &before); + assert!(!proj.join(".mvn").exists(), ".mvn residue"); +} + /// #655: `a` declares the library through `${ct.group}:${ct.artifact}`, /// properties the local corp parent defines. Maven interpolates them before /// matching, so vendor must rewrite `a`'s literal version too, or `a` builds diff --git a/crates/socket-patch-core/src/vendor/jvm/apply.rs b/crates/socket-patch-core/src/vendor/jvm/apply.rs index f81caa585..4a78b2821 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -30,7 +30,7 @@ use super::super::state::{VendorEntry, WiringAction, WiringRecord}; use super::super::{RevertOpts, RevertOutcome, VendorWarning}; use super::{ coursier_tree, gradle, layout, maven_reactor, op_of, op_str, sbt, scala_cli, sha256_hex, - Coords, JvmPlan, JvmUnplan, ReadFn, Shape, CONFIG_LINE_KIND, COURSIER_INDEX_KIND, + Coords, JvmPlan, JvmUnplan, ListFn, ReadFn, Shape, CONFIG_LINE_KIND, COURSIER_INDEX_KIND, CREATED_DIR_KIND, DERIVED_METADATA_KIND, KINDS, OWNED_FILE_KIND, POM_FRAGMENT_KIND, SBT_FRAGMENT_KIND, SETTINGS_FRAGMENT_KIND, TREE_KIND, VERIFICATION_FRAGMENT_KIND, }; @@ -198,15 +198,31 @@ impl ProjectReader { /// The children of the project directory `rel` (`""` = the root) for /// the script graph: names, directories ending in `/`. Resolved like /// [`Self::read`]; anything unsafe or unreadable lists as empty. + /// + /// A directory reached through a symlink that leaves the checkout is + /// only peeked at: its files are listed, its subdirectories are not, + /// and no escape is recorded. A probe (Maven 4 subproject discovery + /// asking whether a linked `target/` holds a `pom.xml`) then refuses + /// nothing for a link it never reads through, while reading any file + /// under the link still records the escape the caller refuses. pub fn list(&self, rel: &str) -> Vec { - let path = if rel.is_empty() { + let (path, outside) = if rel.is_empty() { match &self.canonical { - Some(c) => c.clone(), + Some(c) => (c.clone(), false), None => return Vec::new(), } } else { - match self.resolve(rel.trim_end_matches('/')) { - Ok(path) => path, + let rel = rel.trim_end_matches('/'); + let before = self.escaped.borrow().clone(); + match self.resolve(rel) { + Ok(path) => (path, false), + Err(e) if e.contains(LEAVES_PROJECT) => { + *self.escaped.borrow_mut() = before; + match self.outside_target(rel) { + Some(target) => (target, true), + None => return Vec::new(), + } + } Err(_) => return Vec::new(), } }; @@ -219,13 +235,28 @@ impl ProjectReader { // Followed like `read` (in-checkout links only). let dir = e.metadata().ok()?.is_dir() || (e.file_type().ok()?.is_symlink() && e.path().is_dir()); - Some(if dir { format!("{name}/") } else { name }) + match (dir, outside) { + (true, true) => None, + (true, false) => Some(format!("{name}/")), + (false, _) => Some(name), + } }) .collect(); out.sort(); out } + /// The directory a project-relative `rel` names once its symlinks are + /// followed wherever they lead (for [`Self::list`]'s peek). `None` for + /// an unsafe path or one that does not resolve to a directory. + fn outside_target(&self, rel: &str) -> Option { + if !safe_rel(rel) { + return None; + } + let target = std::fs::canonicalize(self.canonical.as_ref()?.join(rel)).ok()?; + target.is_dir().then_some(target) + } + /// The first path that resolved outside the checkout. pub fn escaped(&self) -> Option { self.escaped.borrow().clone() @@ -269,7 +300,7 @@ impl ProjectReader { let at = segments[..=i].join("/"); self.escaped.borrow_mut().get_or_insert(at.clone()); return Err(format!( - "{at} is a symlink that leaves the project (or dangles)" + "{at} is a symlink that {LEAVES_PROJECT} (or dangles)" )); }; cur = target; @@ -281,6 +312,9 @@ impl ProjectReader { } } +/// How [`ProjectReader`] words a symlink resolving outside the checkout. +const LEAVES_PROJECT: &str = "leaves the project"; + /// A path that does not exist, including one that runs through a regular /// file (`pom.xml/pom.xml`, probed when a `relativePath` or `` /// names a pom file rather than its directory, #534). @@ -593,6 +627,7 @@ pub async fn revert(root: &Path, entry: &VendorEntry, opts: RevertOpts) -> Rever } let reader = ProjectReader::new(root); let read = |rel: &str| reader.read(rel); + let list = |dir: &str| reader.list(dir); let peers = match super::super::state::load_state(root).await { Ok(state) => state, Err(e) => return RevertOutcome::failed(format!("vendor_state_unreadable: {e}")), @@ -625,7 +660,10 @@ pub async fn revert(root: &Path, entry: &VendorEntry, opts: RevertOpts) -> Rever unplan = merge_unplans(unplan, gradle::unplan(&read, &c, &records)); } if maven { - unplan = merge_unplans(unplan, maven_reactor::unplan(&read, &c, &entry.wiring)); + unplan = merge_unplans( + unplan, + maven_reactor::unplan(&read, &list, &c, &entry.wiring), + ); } unplan }; @@ -837,13 +875,14 @@ pub fn entry_references(root: &Path, entry: &VendorEntry) -> bool { }; let reader = ProjectReader::new(root); let read = |rel: &str| reader.read(rel); + let list = |dir: &str| reader.list(dir); let referenced = if sbt::owns(&entry.wiring) { sbt::wired_checked(&read, &c).unwrap_or(true) } else if scala_cli::owns(&entry.wiring) { scala_cli::wired_checked(&read, &c).unwrap_or(true) } else { let (maven, gradle) = sides(&entry.wiring); - (maven && maven_reactor::wired_checked(&read, &c).unwrap_or(true)) + (maven && maven_reactor::wired_checked(&read, &list, &c).unwrap_or(true)) || (gradle && gradle::references_checked(&read, &c).unwrap_or(true)) }; // `read` answers `None` for an unreadable file as for a missing one; @@ -878,7 +917,7 @@ pub fn entry_wired_checked(root: &Path, entry: &VendorEntry) -> Result Result, + list: ListFn<'_>, patch: &super::JvmPatch<'_>, local_repo: Option<&Path>, ) -> maven_reactor::ExternalPoms { let mut known = maven_reactor::ExternalPoms::new(); loop { - let need = maven_reactor::external_poms_needed(read, patch, &known); + let need = maven_reactor::external_poms_needed(read, list, patch, &known); if need.is_empty() { return known; } @@ -1000,7 +1040,7 @@ pub fn check_entry( // sync: the pin it wrote when it could read it, or none. let shape = shape_of(&entry.wiring); let external = if matches!(shape, Shape::MavenReactor | Shape::Mixed) { - Some(local_external_poms(&read, &patch, local_repo)) + Some(local_external_poms(&read, &list, &patch, local_repo)) } else { None }; @@ -1020,7 +1060,7 @@ pub fn check_entry( let referenced = if sbt_entry || scala_cli_entry { entry_wired_checked(root, entry)? } else { - (!maven || maven_reactor::wired_checked(&read, &c).map_err(|e| e.detail)?) + (!maven || maven_reactor::wired_checked(&read, &list, &c).map_err(|e| e.detail)?) && (!gradle || gradle::references(&read, &c)) }; if !referenced { diff --git a/crates/socket-patch-core/src/vendor/jvm/gradle.rs b/crates/socket-patch-core/src/vendor/jvm/gradle.rs index dcfd65308..7340b2034 100644 --- a/crates/socket-patch-core/src/vendor/jvm/gradle.rs +++ b/crates/socket-patch-core/src/vendor/jvm/gradle.rs @@ -2443,6 +2443,7 @@ fn line_start(text: &str, at: usize) -> usize { mod tests { use std::collections::BTreeMap; + use super::super::testing::list_of; use super::super::{apply, detect, Shape}; use super::*; @@ -2495,29 +2496,6 @@ mod tests { plan(&read, &|dir: &str| list_of(files, dir), p) } - /// The children of `dir` in an in-memory tree (the planner's ListFn). - fn list_of(files: &BTreeMap>, dir: &str) -> Vec { - let prefix = if dir.is_empty() { - String::new() - } else { - format!("{}/", dir.trim_end_matches('/')) - }; - let mut out: Vec = Vec::new(); - for key in files.keys() { - let Some(rest) = key.strip_prefix(&prefix) else { - continue; - }; - let child = match rest.find('/') { - Some(i) => format!("{}/", &rest[..i]), - None => rest.to_string(), - }; - if !out.contains(&child) { - out.push(child); - } - } - out - } - fn vendored_line(kotlin: bool, prefix: &str) -> String { apply_line(&WiringTarget::vendored(), kotlin, prefix) } diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index f4a8df996..b9aebaf57 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -16,7 +16,7 @@ use super::layout::{self, safe_coordinates}; use super::{ adopt, changes_between, finish_writes, fragment, op_of, op_str, owned_file, replace_op, sha1_hex, sha256_hex, undo_replace, Coords, FileWrite, JvmPatch, JvmPlan, JvmRefusal, - JvmUnplan, JvmWarning, ReadFn, CONFIG_LINE_KIND, OWNED_FILE_KIND, POM_FRAGMENT_KIND, + JvmUnplan, JvmWarning, ListFn, ReadFn, CONFIG_LINE_KIND, OWNED_FILE_KIND, POM_FRAGMENT_KIND, TREE_GITATTRIBUTES, }; @@ -98,17 +98,22 @@ pub fn committed(read: ReadFn<'_>, c: &Coords<'_>) -> Option<(Vec, Vec)> } /// Plan vendoring `patch` into the reactor rooted at `pom.xml`. -pub fn plan(read: ReadFn<'_>, patch: &JvmPatch<'_>) -> Result { - plan_with_config(read, patch, true) +pub fn plan( + read: ReadFn<'_>, + list: ListFn<'_>, + patch: &JvmPatch<'_>, +) -> Result { + plan_with_config(read, list, patch, true) } /// Plan with an explicit Maven config policy. A disabled policy is recorded for re-runs. pub fn plan_with_config( read: ReadFn<'_>, + list: ListFn<'_>, patch: &JvmPatch<'_>, config_enabled: bool, ) -> Result { - plan_with_external(read, patch, config_enabled, None) + plan_with_external(read, list, patch, config_enabled, None) } /// [`plan_with_config`] that also weighs the management a local root's pin @@ -118,6 +123,7 @@ pub fn plan_with_config( /// skips the check (callers that plan only to probe a shape). pub fn plan_with_external( read: ReadFn<'_>, + list: ListFn<'_>, patch: &JvmPatch<'_>, config_enabled: bool, external: Option<&ExternalPoms>, @@ -141,7 +147,7 @@ pub fn plan_with_external( ), })?; - let reactor = Reactor::discover(read)?; + let reactor = Reactor::discover(read, list)?; let mut warnings = Vec::new(); let wrapper = super::wrapper_version(read, "maven"); if config_enabled && wrapper.is_none_or(|v| ((3, 9, 2)..(3, 9, 9)).contains(&v)) { @@ -344,14 +350,19 @@ fn op_of_value(op: &Value) -> &str { /// versions go now; `pin_section` shells once empty; the repository block, /// the `maven.config` lines and the tree `.gitattributes` only once no pom /// references another patch. -pub fn unplan(read: ReadFn<'_>, c: &Coords<'_>, records: &[WiringRecord]) -> JvmUnplan { +pub fn unplan( + read: ReadFn<'_>, + list: ListFn<'_>, + c: &Coords<'_>, + records: &[WiringRecord], +) -> JvmUnplan { let sv = c.suffixed_version(); let mut files: BTreeSet = records .iter() .filter(|w| w.kind == POM_FRAGMENT_KIND) .map(|w| w.file.clone()) .collect(); - let reactor = Reactor::discover(read).ok(); + let reactor = Reactor::discover(read, list).ok(); if let Some(reactor) = &reactor { files.extend(reactor.scope.iter().cloned()); } @@ -447,13 +458,17 @@ pub fn unplan(read: ReadFn<'_>, c: &Coords<'_>, records: &[WiringRecord]) -> Jvm /// Whether a pom of the reactor still declares `c`'s suffixed version /// (outside comments): the liveness proof `vex` needs for this layout. -pub fn wired(read: ReadFn<'_>, c: &Coords<'_>) -> bool { - wired_checked(read, c).unwrap_or(false) +pub fn wired(read: ReadFn<'_>, list: ListFn<'_>, c: &Coords<'_>) -> bool { + wired_checked(read, list, c).unwrap_or(false) } -pub fn wired_checked(read: ReadFn<'_>, c: &Coords<'_>) -> Result { +pub fn wired_checked( + read: ReadFn<'_>, + list: ListFn<'_>, + c: &Coords<'_>, +) -> Result { let sv = c.suffixed_version(); - Reactor::discover(read).map(|reactor| { + Reactor::discover(read, list).map(|reactor| { reactor .scope .iter() @@ -465,9 +480,13 @@ pub fn wired_checked(read: ReadFn<'_>, c: &Coords<'_>) -> Result, c: &Coords<'_>) -> Result { +pub fn attestable_checked( + read: ReadFn<'_>, + list: ListFn<'_>, + c: &Coords<'_>, +) -> Result { let sv = c.suffixed_version(); - let reactor = Reactor::discover(read)?; + let reactor = Reactor::discover(read, list)?; if !reactor .scope .iter() @@ -723,8 +742,8 @@ fn degraded(reason: &str, detail: impl std::fmt::Display) -> JvmWarning { } /// Whether an ancestor reactor owns the requested pom. Used to reject partial vendoring. -pub fn contains_module(read: ReadFn<'_>, rel: &str) -> bool { - Reactor::discover(read).is_ok_and(|r| r.reactor.iter().any(|p| p == rel)) +pub fn contains_module(read: ReadFn<'_>, list: ListFn<'_>, rel: &str) -> bool { + Reactor::discover(read, list).is_ok_and(|r| r.reactor.iter().any(|p| p == rel)) } pub(crate) type Gav = (String, String, String); @@ -882,10 +901,11 @@ struct ManagedConflict { /// plan refuses it on its own). pub fn external_poms_needed( read: ReadFn<'_>, + list: ListFn<'_>, patch: &JvmPatch<'_>, known: &ExternalPoms, ) -> Vec { - let Ok(reactor) = Reactor::discover(read) else { + let Ok(reactor) = Reactor::discover(read, list) else { return Vec::new(); }; if known.len() >= MAX_EXTERNAL_POMS { @@ -1358,6 +1378,9 @@ struct Pom { /// Model 4.1.0 (Maven 4): omitted parent coordinates are inferred /// from the pom at the relative path. infers_parent: bool, + /// Maven 4 discovers this pom's subprojects itself: see + /// [`discovers_subprojects`]. + discovers_subprojects: bool, } /// A pom and one of its profiles' ``. @@ -1409,6 +1432,7 @@ impl Pom { || doc.masked.contains("maven.apache.org/POM/4.1.0"); Ok(Pom { infers_parent, + discovers_subprojects: discovers_subprojects(&doc), dir: rel.rsplit_once('/').map_or("", |(d, _)| d).to_string(), group: doc.child_text(project, "groupId"), artifact: doc.child_text(project, "artifactId"), @@ -1472,9 +1496,17 @@ struct Reactor { } impl Reactor { - fn discover(read: ReadFn<'_>) -> Result { + fn discover(read: ReadFn<'_>, list: ListFn<'_>) -> Result { let root = read("pom.xml") .ok_or_else(|| shape_refusal("no_build_file", "no pom.xml at the project root"))?; + let cli_props = read(MAVEN_CONFIG) + .map(|bytes| cli_properties(&String::from_utf8_lossy(&bytes))) + .unwrap_or_default(); + // Maven 4's opt-out, read with Java's `Boolean.parseBoolean`: once + // set, anything but `true` turns discovery off. + let discovery = cli_props + .get(DISCOVER_SUBPROJECTS) + .is_none_or(|v| v.eq_ignore_ascii_case("true")); let mut poms = BTreeMap::new(); poms.insert("pom.xml".to_string(), Pom::parse("pom.xml", root)?); let mut reactor = vec!["pom.xml".to_string()]; @@ -1482,7 +1514,11 @@ impl Reactor { while next < reactor.len() { let rel = reactor[next].clone(); next += 1; - for module in module_refs(&poms[&rel].doc) { + let mut modules = module_refs(&poms[&rel].doc); + if discovery && poms[&rel].discovers_subprojects { + modules.extend(implicit_subprojects(list, &poms[&rel].dir)); + } + for module in modules { let (module_rel, bytes) = resolve_module(read, &poms[&rel].dir, &rel, &module)?; if poms.contains_key(&module_rel) { if !reactor.contains(&module_rel) { @@ -1524,9 +1560,6 @@ impl Reactor { } } - let cli_props = read(MAVEN_CONFIG) - .map(|bytes| cli_properties(&String::from_utf8_lossy(&bytes))) - .unwrap_or_default(); Ok(Reactor { poms, reactor, @@ -1977,6 +2010,54 @@ fn module_refs(doc: &Doc) -> Vec { out } +/// The user property that turns Maven 4's subproject discovery off. +const DISCOVER_SUBPROJECTS: &str = "maven.project.discoverSubprojects"; + +/// Whether Maven 4 discovers the subprojects of this pom on its own +/// (`DefaultModelBuilder`, "subprojects discovery"): its model version is +/// not 4.0.0 (the ``, else the POM namespace), it is +/// packaged `pom`, and it lists no top-level `` or +/// `` entry. Maven 3 never discovers, and reads no 4.1.0 pom. +fn discovers_subprojects(doc: &Doc) -> bool { + let project = doc.project; + let model = doc.child_text(project, "modelVersion").or_else(|| { + let at = doc.masked.find(POM_NAMESPACE)? + POM_NAMESPACE.len(); + let rest = &doc.masked[at..]; + let end = rest.find(|c: char| !(c.is_ascii_alphanumeric() || c == '.'))?; + Some(rest[..end].to_string()) + }); + let lists = |list: &str, item: &str| { + doc.child(project, list) + .is_some_and(|l| doc.children(l, item).next().is_some()) + }; + model.is_some_and(|v| v != "4.0.0") + && doc.child_text(project, "packaging").as_deref() == Some("pom") + && !lists("subprojects", "subproject") + && !lists("modules", "module") +} + +const POM_NAMESPACE: &str = "maven.apache.org/POM/"; + +/// The subprojects Maven 4 discovers under the pom at `dir`: every direct +/// subdirectory holding a `pom.xml`, hidden ones included, as +/// `` names relative to `dir`. Probed through the listing, not +/// a read: a directory the reader refuses (`.git/`) lists as empty, where +/// reading its `pom.xml` would record a read error. +fn implicit_subprojects(list: ListFn<'_>, dir: &str) -> Vec { + list(dir) + .iter() + .filter_map(|entry| entry.strip_suffix('/')) + .filter(|name| { + let sub = match dir { + "" => name.to_string(), + _ => format!("{dir}/{name}"), + }; + list(&sub).iter().any(|e| e == "pom.xml") + }) + .map(str::to_string) + .collect() +} + /// `module` of the pom at `dir` → the module pom's path and bytes. fn resolve_module( read: ReadFn<'_>, @@ -3410,9 +3491,13 @@ mod tests { defaults } + fn lister(files: &Fs) -> impl Fn(&str) -> Vec + '_ { + move |dir: &str| super::super::testing::list_of(files, dir) + } + fn run(files: &Fs) -> Result { let read = |p: &str| files.get(p).cloned(); - plan(&read, &patch()) + plan(&read, &lister(files), &patch()) } fn applied(files: &Fs, plan: &JvmPlan) -> Fs { @@ -3546,7 +3631,7 @@ mod tests { let read = |p: &str| files.get(p).cloned(); let mut known = ExternalPoms::new(); loop { - let need = external_poms_needed(&read, &patch(), &known); + let need = external_poms_needed(&read, &lister(&files), &patch(), &known); if need.is_empty() { break; } @@ -3556,7 +3641,7 @@ mod tests { } } ( - plan_with_external(&read, &patch(), true, Some(&known)), + plan_with_external(&read, &lister(&files), &patch(), true, Some(&known)), known, ) } @@ -4540,6 +4625,7 @@ mod tests { let read = |p: &str| files.get(p).cloned(); let r = plan( &read, + &lister(&files), &patch_with("${revision}"), ) .unwrap_err(); @@ -4794,7 +4880,7 @@ mod tests { fn replan_writes(root: &std::path::Path, p: &JvmPatch<'_>) -> Vec { let reader = super::super::apply::ProjectReader::new(root); - plan(&|rel: &str| reader.read(rel), p) + plan(&|rel: &str| reader.read(rel), &|d: &str| reader.list(d), p) .unwrap() .writes .into_iter() @@ -4990,7 +5076,7 @@ mod tests { ]); let (_, after) = vendor(&files); let read = |rel: &str| after.get(rel).cloned(); - let undo = unplan(&read, &patch().coords(), &[]); + let undo = unplan(&read, &lister(&after), &patch().coords(), &[]); let changed: BTreeMap>> = undo.changes.into_iter().collect(); assert_eq!( changed["a/pom.xml"].as_deref(), @@ -5081,7 +5167,7 @@ mod tests { module("a", &cdata.replace("", SV)) ); let read = |rel: &str| after.get(rel).cloned(); - let undo = unplan(&read, &patch().coords(), &plan.records); + let undo = unplan(&read, &lister(&after), &patch().coords(), &plan.records); let a = undo.changes.iter().find(|(r, _)| r == "a/pom.xml").unwrap(); assert_eq!(a.1.as_deref(), Some(module("a", cdata).as_bytes())); } @@ -5107,7 +5193,7 @@ mod tests { assert!(plan.warnings.is_empty(), "{:?}", plan.warnings); assert_eq!(text(&after, "a/pom.xml"), a.replace("1.10.0", SV)); let read = |rel: &str| after.get(rel).cloned(); - let undo = unplan(&read, &patch().coords(), &plan.records); + let undo = unplan(&read, &lister(&after), &patch().coords(), &plan.records); assert!(undo.drifted.is_empty(), "{:?}", undo.drifted); let a_undo = undo.changes.iter().find(|(r, _)| r == "a/pom.xml").unwrap(); assert_eq!(a_undo.1.as_deref(), Some(a.as_bytes())); @@ -5372,6 +5458,206 @@ mod tests { assert!(root_pinned(&plan, &files)); } + /// A Maven 4 (model 4.1.0) aggregator root that lists no subprojects. + const IMPLICIT_ROOT: &str = "\n 4.1.0\n \ + com.example\n root\n \ + 1.0.0\n pom\n\n"; + + fn implicit_child(artifact: &str, packaging: &str, deps: &str) -> String { + format!( + "\n \ + 4.1.0\n \n \ + {artifact}\n {packaging}\n \ + \n {deps}\n \n\n" + ) + } + + /// #622: Maven 4 builds every child directory holding a `pom.xml` of a + /// 4.1.0 `pom`-packaged root that lists no ``, so those + /// children are the reactor: the declaring module is rewritten to the + /// suffixed version (immune to a warm `~/.m2`) and the root pinned, + /// instead of a reactor of one that wires nothing the modules read. + #[test] + fn maven_4_implicit_subprojects_are_planned_as_the_reactor() { + let a = implicit_child("a", "jar", &dep("1.10.0")); + let b = implicit_child( + "b", + "jar", + "com.examplea\ + ${project.version}", + ); + let files = fs(&[ + ("pom.xml", IMPLICIT_ROOT), + ("a/pom.xml", &a), + ("b/pom.xml", &b), + ]); + let (plan, after) = vendor(&files); + assert!(plan.warnings.is_empty(), "{:?}", plan.warnings); + assert_eq!(text(&after, "a/pom.xml"), a.replace("1.10.0", SV)); + assert_eq!(text(&after, "b/pom.xml"), b); + assert!(root_pinned(&plan, &files)); + let c = patch().coords(); + let read = |p: &str| after.get(p).cloned(); + assert!(wired_checked(&read, &lister(&after), &c).unwrap()); + assert!(attestable_checked(&read, &lister(&after), &c).unwrap()); + // `vendor` run from a discovered module is not the build root. + assert!(contains_module(&read, &lister(&after), "a/pom.xml")); + } + + fn reactor_of(files: &Fs) -> Vec { + let read = |p: &str| files.get(p).cloned(); + Reactor::discover(&read, &lister(files)).unwrap().reactor + } + + /// #622: discovery as Maven 4.0.0-rc-7 does it (`DefaultModelBuilder`), + /// checked against `mvn validate`'s reactor: every direct subdirectory + /// holding a `pom.xml`, hidden ones included, recursively through a + /// discovered `pom`-packaged child; not a deeper pom nor a directory + /// without one. + #[test] + fn maven_4_subproject_discovery_follows_maven() { + let files = fs(&[ + ("pom.xml", IMPLICIT_ROOT), + ("a/pom.xml", &implicit_child("a", "pom", "")), + ("a/sub/pom.xml", &implicit_child("sub", "jar", "")), + ("b/pom.xml", &implicit_child("b", "jar", "")), + ("b/nested/pom.xml", &implicit_child("nested", "jar", "")), + (".h/pom.xml", &implicit_child("h", "jar", "")), + ("nopom/c/pom.xml", &implicit_child("c", "jar", "")), + ("src/main/java/A.java", "class A {}"), + ]); + assert_eq!( + reactor_of(&files), + [ + "pom.xml", + ".h/pom.xml", + "a/pom.xml", + "b/pom.xml", + "a/sub/pom.xml" + ] + ); + } + + /// #622: in a git checkout, discovery lists `.git/` like any directory. + /// Probing it must not record a read error (the reader refuses every + /// path through `.git`), or `vex` and `vendor --check` fail on it. + #[tokio::test] + async fn maven_4_subproject_discovery_in_a_git_checkout_stays_readable() { + let a = implicit_child("a", "jar", &dep("1.10.0")); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + super::super::testing::populate( + root, + &[ + ("pom.xml", IMPLICIT_ROOT), + ("a/pom.xml", &a), + (".git/HEAD", "ref: refs/heads/main\n"), + ], + ); + let mut ledger = BTreeMap::new(); + let plan = super::super::testing::vendor(root, Shape::MavenReactor, &patch(), &mut ledger) + .await + .unwrap(); + assert!(plan.writes.iter().any(|w| w.rel == "a/pom.xml")); + let entry = ledger.values().next().unwrap(); + assert_eq!( + super::super::apply::entry_wired_checked(root, entry), + Ok(true) + ); + } + + /// #622: a directory symlinked outside the checkout (a `target/` on + /// another disk) is probed for a `pom.xml` without being reported. It + /// holds none, so Maven ignores it and so does the plan; one that holds + /// a pom is a subproject built from outside, reported as before. + #[cfg(unix)] + #[tokio::test] + async fn maven_4_subproject_discovery_through_an_outbound_symlink() { + let a = implicit_child("a", "jar", &dep("1.10.0")); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + super::super::testing::populate(root, &[("pom.xml", IMPLICIT_ROOT), ("a/pom.xml", &a)]); + let outside = tempfile::tempdir().unwrap(); + super::super::testing::populate(outside.path(), &[("classes/A.class", "x")]); + std::os::unix::fs::symlink(outside.path(), root.join("target")).unwrap(); + let mut ledger = BTreeMap::new(); + // `testing::vendor` asserts nothing escaped. + let planned = + super::super::testing::vendor(root, Shape::MavenReactor, &patch(), &mut ledger) + .await + .unwrap(); + assert!(planned.writes.iter().any(|w| w.rel == "a/pom.xml")); + let entry = ledger.values().next().unwrap(); + assert_eq!( + super::super::apply::entry_wired_checked(root, entry), + Ok(true) + ); + + let ext = tempfile::tempdir().unwrap(); + super::super::testing::populate( + ext.path(), + &[("pom.xml", &implicit_child("x", "jar", ""))], + ); + std::os::unix::fs::symlink(ext.path(), root.join("ext")).unwrap(); + let reader = super::super::apply::ProjectReader::new(root); + let _ = plan( + &|rel: &str| reader.read(rel), + &|d: &str| reader.list(d), + &patch(), + ); + assert_eq!(reader.escaped().as_deref(), Some("ext")); + } + + /// #622: no discovery for a 4.0.0 model, a non-`pom` packaging, a root + /// that lists its subprojects, or `maven.project.discoverSubprojects` + /// set to anything but `true` (Java's `Boolean.parseBoolean`). The + /// model version may come from the namespace alone, and an empty + /// `` lists nothing, so both still discover. + #[test] + fn maven_4_subproject_discovery_is_gated_like_maven() { + let a = implicit_child("a", "jar", ""); + let b = implicit_child("b", "jar", ""); + let tree = |root: &str, extra: &[(&str, &str)]| { + let mut files = fs(&[("pom.xml", root), ("a/pom.xml", &a), ("b/pom.xml", &b)]); + files.extend( + extra + .iter() + .map(|(p, c)| (p.to_string(), c.as_bytes().to_vec())), + ); + reactor_of(&files) + }; + let all = ["pom.xml", "a/pom.xml", "b/pom.xml"]; + let model_4_0 = IMPLICIT_ROOT + .replace("POM/4.1.0\" root=\"true\"", "POM/4.0.0\"") + .replace("4.1.0", "4.0.0"); + assert_eq!(tree(&model_4_0, &[]), ["pom.xml"]); + let jar = IMPLICIT_ROOT.replace("pom", "jar"); + assert_eq!(tree(&jar, &[]), ["pom.xml"]); + let no_packaging = IMPLICIT_ROOT.replace(" pom\n", ""); + assert_eq!(tree(&no_packaging, &[]), ["pom.xml"]); + let listed = IMPLICIT_ROOT.replace( + "", + "\n a", + ); + assert_eq!(tree(&listed, &[]), ["pom.xml", "a/pom.xml"]); + let empty_list = IMPLICIT_ROOT.replace("", "\n "); + assert_eq!(tree(&empty_list, &[]), all); + let namespace_only = IMPLICIT_ROOT.replace(" 4.1.0\n", ""); + assert_eq!(tree(&namespace_only, &[]), all); + for (value, discovers) in [ + ("true", true), + ("TRUE", true), + ("false", false), + ("no", false), + ] { + let config = format!("-Dmaven.project.discoverSubprojects={value}\n"); + let got = tree(IMPLICIT_ROOT, &[(MAVEN_CONFIG, &config)]); + let want: &[&str] = if discovers { &all } else { &["pom.xml"] }; + assert_eq!(got, want, "{value}"); + } + } + /// #459: a profile of the declaring module raising the property keeps /// the declaration and leaves the root unpinned. #[test] @@ -5479,8 +5765,8 @@ mod tests { uuid: UUID, }; ( - wired_checked(&read, &c).unwrap(), - attestable_checked(&read, &c).unwrap(), + wired_checked(&read, &lister(&files), &c).unwrap(), + attestable_checked(&read, &lister(&files), &c).unwrap(), ) } @@ -5672,7 +5958,7 @@ mod tests { ..patch() }; assert_eq!( - plan(&read, &p).unwrap_err().code, + plan(&read, &lister(&files), &p).unwrap_err().code, "unsafe_coordinates", "{g}:{a}:{v}" ); @@ -5686,7 +5972,11 @@ mod tests { let outside = disk(&[("pom.xml", &module("a", &dep("1.10.0")))]); std::os::unix::fs::symlink(outside.path(), dir.path().join("a")).unwrap(); let reader = super::super::apply::ProjectReader::new(dir.path()); - let _ = plan(&|rel: &str| reader.read(rel), &patch()); + let _ = plan( + &|rel: &str| reader.read(rel), + &|d: &str| reader.list(d), + &patch(), + ); assert_eq!(reader.escaped().as_deref(), Some("a")); } @@ -5705,7 +5995,7 @@ mod tests { upstream_pom: &pom, ..patch() }; - assert!(plan(&read, &p).unwrap().writes.is_empty()); + assert!(plan(&read, &lister(&after), &p).unwrap().writes.is_empty()); // A parent-inherited upstream version (inserted by the suffixing). let parent_only = "\n \n org.apache.commons\n \ commons-parent\n 1.10.0\n \n \ @@ -5717,6 +6007,7 @@ mod tests { ]); let first = plan( &|rel: &str| files.get(rel).cloned(), + &lister(&files), &patch_with(parent_only), ) .unwrap(); @@ -5728,7 +6019,7 @@ mod tests { upstream_pom: &pom, ..patch() }; - assert!(plan(&read, &p).unwrap().writes.is_empty()); + assert!(plan(&read, &lister(&after), &p).unwrap().writes.is_empty()); assert!(committed(&|_: &str| None, &patch().coords()).is_none()); } diff --git a/crates/socket-patch-core/src/vendor/jvm/mod.rs b/crates/socket-patch-core/src/vendor/jvm/mod.rs index 258d518be..996440192 100644 --- a/crates/socket-patch-core/src/vendor/jvm/mod.rs +++ b/crates/socket-patch-core/src/vendor/jvm/mod.rs @@ -399,12 +399,13 @@ pub fn plan_with_external( ) -> Result { match shape { Shape::MavenReactor => { - maven_reactor::plan_with_external(read, patch, config_enabled, external) + maven_reactor::plan_with_external(read, list, patch, config_enabled, external) } Shape::Gradle => gradle::plan(read, list, patch), Shape::Mixed => { // Both halves or neither: a refusal of either writes nothing. - let maven = maven_reactor::plan_with_external(read, patch, config_enabled, external)?; + let maven = + maven_reactor::plan_with_external(read, list, patch, config_enabled, external)?; let gradle = gradle::plan(read, list, patch)?; Ok(compose(maven, gradle)) } @@ -643,6 +644,29 @@ pub(crate) fn is_ours(current: Option<&[u8]>, ours: &str) -> bool { #[cfg(test)] pub(crate) mod testing { use std::collections::BTreeMap; + + /// The children of `dir` in an in-memory tree (the planners' ListFn). + pub(crate) fn list_of(files: &BTreeMap>, dir: &str) -> Vec { + let prefix = if dir.is_empty() { + String::new() + } else { + format!("{}/", dir.trim_end_matches('/')) + }; + let mut out: Vec = Vec::new(); + for key in files.keys() { + let Some(rest) = key.strip_prefix(&prefix) else { + continue; + }; + let child = match rest.find('/') { + Some(i) => format!("{}/", &rest[..i]), + None => rest.to_string(), + }; + if !out.contains(&child) { + out.push(child); + } + } + out + } use std::path::Path; use super::super::state::{carry_forward_wiring, VendorEntry}; diff --git a/crates/socket-patch-core/src/vendor/jvm/sbt.rs b/crates/socket-patch-core/src/vendor/jvm/sbt.rs index 928c12133..e17ea4fb3 100644 --- a/crates/socket-patch-core/src/vendor/jvm/sbt.rs +++ b/crates/socket-patch-core/src/vendor/jvm/sbt.rs @@ -798,7 +798,7 @@ mod tests { ), _ => None, }; - let planned = maven_reactor::plan(&reactor, &p).unwrap(); + let planned = maven_reactor::plan(&reactor, &super::super::no_list, &p).unwrap(); let tree: Vec<_> = planned.writes.into_iter().filter(|w| w.tree).collect(); assert_eq!(tree, writes); assert_eq!(planned.jar_rel, jar); diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index 1b7afe0f4..7bb6d3567 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -346,6 +346,7 @@ pub(super) fn not_build_root(project_root: &Path) -> Option { let shown = || shown_ancestor(project_root, ancestor, &canonical_root); if super::jvm::maven_reactor::contains_module( &|p| reader.read(p), + &|d| reader.list(d), &format!("{rel}/pom.xml"), ) { return Some(format!( @@ -953,7 +954,7 @@ async fn vendor_maven_jvm( super::jvm::sbt::plan_with_digest(&read, &patch, gate_pass.deps_digest.as_deref()) } Shape::MavenReactor | Shape::Mixed => { - let external = external_maven_poms(&read, &patch, &local, service).await; + let external = external_maven_poms(&read, &list, &patch, &local, service).await; super::jvm::plan_with_external( shape, &read, @@ -1342,13 +1343,14 @@ async fn collect_gradle_metadata( /// recorded unavailable, and the planner then leaves that root unpinned. async fn external_maven_poms( read: super::jvm::ReadFn<'_>, + list: super::jvm::ListFn<'_>, patch: &super::jvm::JvmPatch<'_>, local: &LocalSources, service: Option<&VendorServiceConfig>, ) -> super::jvm::maven_reactor::ExternalPoms { let mut known = super::jvm::maven_reactor::ExternalPoms::new(); loop { - let need = super::jvm::maven_reactor::external_poms_needed(read, patch, &known); + let need = super::jvm::maven_reactor::external_poms_needed(read, list, patch, &known); if need.is_empty() { return known; }