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 36abe8385..d458fb83f 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_jvm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_jvm_build.rs @@ -505,6 +505,37 @@ fn maven_reactor_vendor_fresh_checkout_offline_build_and_byte_exact_revert() { "re-vendor: changed {changed:?}, added {added:?}, removed {removed:?}" ); + // #534: `b`'s `../corp-parent` names a + // directory; `vex` and `vendor --check` read it as Maven does. + let (code, doc, stderr) = socket_in( + &proj, + &m2, + None, + &[ + "vex", + "--cwd", + proj.to_str().unwrap(), + "--product", + "pkg:maven/com.example/aggregator@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}"); + // FRESH CHECKOUT: commons-text only from the committed tree. let fresh = root.join("fresh"); fresh_checkout_all(&proj, &fresh); diff --git a/crates/socket-patch-core/src/vendor/jvm/apply.rs b/crates/socket-patch-core/src/vendor/jvm/apply.rs index e9d17f762..2b591294f 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -172,10 +172,11 @@ impl ProjectReader { } } - /// Regular files only; a directory, special file or unsafe path reads - /// as missing. + /// Regular files only. A path that does not exist, or that runs through + /// a regular file, reads as missing; a directory, special file or + /// unsafe path reads as `None` and records a read error. pub fn read(&self, rel: &str) -> Option> { - let path = match self.resolve(rel) { + let path = match self.resolve_in(rel, true) { Ok(path) => path, Err(e) => { self.read_error.borrow_mut().get_or_insert(e); @@ -184,7 +185,7 @@ impl ProjectReader { }; match group_commit::read(&path).unwrap_or_else(|| read_regular_to_bytes_sync(&path)) { Ok(bytes) => Some(bytes), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) if is_missing(&e) => None, Err(e) => { self.read_error .borrow_mut() @@ -232,6 +233,13 @@ impl ProjectReader { /// `rel` under the canonical root, following in-checkout symlinks. fn resolve(&self, rel: &str) -> Result { + self.resolve_in(rel, false) + } + + /// [`Self::resolve`]; `through_file` lets a path that runs through a + /// regular file resolve (a read then sees it as missing). Writes keep + /// refusing it up front. + fn resolve_in(&self, rel: &str, through_file: bool) -> Result { if !safe_rel(rel) { return Err(format!("unsafe path {rel:?}")); } @@ -246,7 +254,10 @@ impl ProjectReader { for (i, seg) in segments.iter().enumerate() { let next = cur.join(seg); match std::fs::symlink_metadata(&next) { - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + Err(e) + if e.kind() == std::io::ErrorKind::NotFound + || through_file && is_missing(&e) => + { return Ok(segments[i + 1..].iter().fold(next, |p, s| p.join(s))); } Err(e) => return Err(format!("cannot inspect {}: {e}", next.display())), @@ -270,6 +281,16 @@ impl ProjectReader { } } +/// 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). +fn is_missing(e: &std::io::Error) -> bool { + matches!( + e.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::NotADirectory + ) +} + /// Read a project file for the planners' tests (see [`ProjectReader::read`]). #[cfg(test)] pub(crate) fn read_project_file(root: &Path, rel: &str) -> Option> { @@ -1397,6 +1418,28 @@ mod tests { assert!(!outside.path().join("vendor").exists()); } + /// #534: probing `/pom.xml` where `` is a pom file reads as + /// missing, not as a read error that withholds `vex` liveness; a + /// directory read itself still records one, and writes still refuse + /// a path through a file. + #[test] + fn a_path_through_a_file_reads_as_missing() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("parent")).unwrap(); + std::fs::write(root.join("pom.xml"), "\n").unwrap(); + let reader = ProjectReader::new(root); + assert_eq!(reader.read("pom.xml/pom.xml"), None); + assert_eq!(reader.read("pom.xml/x/pom.xml"), None); + assert_eq!(reader.read_error.borrow().as_deref(), None); + assert_eq!(reader.read("parent"), None); + assert!(reader.read_error.borrow().is_some()); + // A write through a file is still refused before anything lands + // (Unix reports ENOTDIR; Windows reports such a path as not found). + #[cfg(unix)] + assert!(reader.resolve("pom.xml/pom.xml").is_err()); + } + #[cfg(unix)] #[tokio::test] async fn an_in_checkout_symlinked_file_is_edited_through_its_link() { 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 c8c54f681..fb65b54c4 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -1840,12 +1840,7 @@ fn resolve_module( format!("{declaring}: {module} leaves the project root"), )); }; - let candidates = if path.is_empty() { - vec!["pom.xml".to_string()] - } else { - vec![format!("{path}/pom.xml"), path] - }; - candidates + pom_candidates(path) .into_iter() .find_map(|c| read(&c).map(|bytes| (c, bytes))) .ok_or_else(|| { @@ -1856,6 +1851,17 @@ fn resolve_module( }) } +/// Where Maven looks for the pom at the normalized `path` a `` or +/// `` names: a directory means its `pom.xml` (`""`, from +/// `..`, is the root), else the file itself. +fn pom_candidates(path: String) -> Vec { + if path.is_empty() { + vec!["pom.xml".to_string()] + } else { + vec![format!("{path}/pom.xml"), path] + } +} + /// The local parent of `rel`: its `` file exists in the checkout /// and names the same groupId/artifactId (and version, when both are /// literal). Loads the parent into `poms`. @@ -1881,22 +1887,18 @@ fn local_parent_of( parent.artifact.clone(), parent.version.clone(), ); - let candidates = [ - path.clone(), - if path.is_empty() { - "pom.xml".to_string() + // Like a ``: a directory names its `pom.xml` (#534). + let found = pom_candidates(path).into_iter().find_map(|c| { + if poms.contains_key(&c) { + Some((c, None)) } else { - format!("{path}/pom.xml") - }, - ]; - let Some(parent_rel) = candidates - .into_iter() - .find(|c| poms.contains_key(c) || read(c).is_some()) - else { + read(&c).map(|bytes| (c, Some(bytes))) + } + }); + let Some((parent_rel, bytes)) = found else { return Ok(None); }; - if !poms.contains_key(&parent_rel) { - let bytes = read(&parent_rel).unwrap_or_default(); + if let Some(bytes) = bytes { let parsed = Pom::parse(&parent_rel, bytes)?; poms.insert(parent_rel.clone(), parsed); } diff --git a/crates/socket-patch-core/src/vendor/jvm/mod.rs b/crates/socket-patch-core/src/vendor/jvm/mod.rs index f7ac0adf9..258d518be 100644 --- a/crates/socket-patch-core/src/vendor/jvm/mod.rs +++ b/crates/socket-patch-core/src/vendor/jvm/mod.rs @@ -985,6 +985,62 @@ mod tests { assert_eq!(testing::dirs(root), pristine_dirs); } + /// #534: Maven reads a `` naming a directory + /// (`..`, `../corp-parent`) as that directory's `pom.xml`. A reactor + /// vendored that way stays live for `vex` and passes `vendor --check` + /// instead of failing on "is not a regular file" / "unsafe path". + #[tokio::test] + async fn parent_relative_path_naming_a_directory_stays_checkable() { + let parent = |artifact: &str, relative: &str| { + format!( + "\n 4.0.0\n \n \ + com.x\n {artifact}\n \ + 1\n {relative}\n \ + \n a\n \n \ + \n org.apache.commons\n \ + commons-text\n 1.10.0\n \ + \n \n\n" + ) + }; + let pom = |artifact: &str, modules: &str| { + format!( + "\n 4.0.0\n com.x\n \ + {artifact}\n 1\n \ + pom\n {modules}\n\n" + ) + }; + let up = pom("root", "a"); + let up_a = parent("root", ".."); + let corp = pom("root", "corp-parenta"); + let corp_parent = pom("corp", ""); + let corp_a = parent("corp", "../corp-parent"); + let layouts: [&[(&str, &str)]; 2] = [ + &[("pom.xml", &up), ("a/pom.xml", &up_a)], + &[ + ("pom.xml", &corp), + ("corp-parent/pom.xml", &corp_parent), + ("a/pom.xml", &corp_a), + ], + ]; + for files in layouts { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + testing::populate(root, files); + let mut ledger = std::collections::BTreeMap::new(); + let p = mixed_patch(); + testing::vendor(root, Shape::MavenReactor, &p, &mut ledger) + .await + .unwrap(); + let entry = ledger.values().next().unwrap().clone(); + assert_eq!( + apply::entry_wired_checked(root, &entry), + Ok(true), + "{files:?}" + ); + apply::check_entry(root, &entry, None).unwrap_or_else(|e| panic!("{files:?}: {e}")); + } + } + /// #488: `vendor --check` weighs an imported BOM from the local /// repository. A BOM it cannot read accepts the pin `vendor` wrote; a /// BOM bumped to another version of the artifact reports the pin as