Fix Maven parent relativePath naming a dir (#534)

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 `<relativePath>../corp-parent</relativePath>` 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);

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<Vec<u8>> {
-        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<PathBuf, String> {
+        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<PathBuf, String> {
         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 `<module>`
+/// 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<Vec<u8>> {
@@ -1397,6 +1418,28 @@ mod tests {
         assert!(!outside.path().join("vendor").exists());
     }
 
+    /// #534: probing `<dir>/pom.xml` where `<dir>` 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"), "<project/>\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() {

crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs

@@ -1840,12 +1840,7 @@ fn resolve_module(
             format!("{declaring}: <module>{module}</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 `<module>` or
+/// `<relativePath>` names: a directory means its `pom.xml` (`""`, from
+/// `..`, is the root), else the file itself.
+fn pom_candidates(path: String) -> Vec<String> {
+    if path.is_empty() {
+        vec!["pom.xml".to_string()]
+    } else {
+        vec![format!("{path}/pom.xml"), path]
+    }
+}
+
 /// The local parent of `rel`: its `<parent>` 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 `<module>`: 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);
     }

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 `<parent><relativePath>` 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!(
+                "<project>\n  <modelVersion>4.0.0</modelVersion>\n  <parent>\n    \
+                 <groupId>com.x</groupId>\n    <artifactId>{artifact}</artifactId>\n    \
+                 <version>1</version>\n    <relativePath>{relative}</relativePath>\n  \
+                 </parent>\n  <artifactId>a</artifactId>\n  <dependencies>\n    \
+                 <dependency>\n      <groupId>org.apache.commons</groupId>\n      \
+                 <artifactId>commons-text</artifactId>\n      <version>1.10.0</version>\n    \
+                 </dependency>\n  </dependencies>\n</project>\n"
+            )
+        };
+        let pom = |artifact: &str, modules: &str| {
+            format!(
+                "<project>\n  <modelVersion>4.0.0</modelVersion>\n  <groupId>com.x</groupId>\n  \
+                 <artifactId>{artifact}</artifactId>\n  <version>1</version>\n  \
+                 <packaging>pom</packaging>\n  <modules>{modules}</modules>\n</project>\n"
+            )
+        };
+        let up = pom("root", "<module>a</module>");
+        let up_a = parent("root", "..");
+        let corp = pom("root", "<module>corp-parent</module><module>a</module>");
+        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