Skip to content

Commit ca6dc05

Browse files
Fix Maven local repository lookup (#423) (#1801)
* Start fix for #423 Assisted-by: Claude Code:claude-opus-5-5 * Find the Maven local repository like Maven does Agent mode and -g looked for the Maven local repository in $M2_HOME/repository, which is the Maven install, and never read <localRepository> from settings.xml. On Windows they also preferred HOME over the profile directory Java uses. With any of these, scan -g found 0 packages, and rollback -g dropped the patch record while the real local repository stayed patched. The lookup now follows Maven: MAVEN_REPO_LOCAL, then -Dmaven.repo.local in MAVEN_ARGS or MAVEN_OPTS, then <localRepository> in the user settings, then in the install's conf/settings.xml, then .m2/repository under user.home. user.home honours -Duser.home and is USERPROFILE on Windows. ${user.home} and ${env.X} expand in settings. Fixes #423 Assisted-by: Claude Code:claude-opus-5-5 * Pin M2_HOME as the Maven install in crawler tests The integration test asserted the old $M2_HOME/repository lookup. It now checks that $M2_HOME/repository is not scanned and that the install's conf/settings.xml <localRepository> is. Assisted-by: Claude Code:claude-opus-5-5 * Read settings.xml localRepository as XML Maven reads a CDATA <localRepository> as its text and decodes entities, ignores a profile property of the same name, and fixes user.home before command-line -D flags in MAVEN_ARGS apply. The lookup now does the same. CLI e2e children also drop MAVEN_OPTS, MAVEN_ARGS, MAVEN_HOME and M2_HOME so a developer's Maven setup can't redirect their crawl. Assisted-by: Claude Code:claude-opus-5-5 * Read Maven settings.xml FIFO-safely A FIFO planted where settings.xml is expected is now refused instead of blocking the crawl in open(2), as the crawler architecture test requires for every crawler read. Assisted-by: Claude Code:claude-opus-5-5 * Skip comments in settings.xml localRepository A comment inside <localRepository> in settings.xml was kept as part of the path, so the crawler fell back to ~/.m2/repository while Maven used the real local repository. The value is now read as Maven's parser reads it: comments drop out, entities decode, CDATA is kept verbatim. The Maven crawler e2e tests also clear M2_HOME now, so a host Maven install's settings.xml can't steer tests that pin the repo list. Assisted-by: Claude Code:claude-opus-5-5 * Make the Maven local-repo tests hermetic and clippy-clean m2_repo_path_treats_empty_m2_home_as_unset now reads the user's settings.xml, so a developer whose ~/.m2/settings.xml sets <localRepository> got a red cargo test. Point HOME/USERPROFILE at an empty temp dir for that test. Also drop a format! with no placeholders in local_repo_tests, which clippy --all-targets flags as useless_format. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Parse MAVEN_OPTS in local-repo tests as the host OS does Three local_repo_tests put a tempdir path into MAVEN_OPTS/MAVEN_ARGS but resolved with Os::Unix. Unix-mode split_opts treats backslashes as escapes, so on Windows `C:\Users\...\Temp\.tmpX` became the drive-relative `C:Users...`, failed has_root(), and the tests got the settings or HOME path instead. Resolve with Os::current() so the options are split the way the host would split them; with only HOME set the Windows branch falls back to HOME, so expectations are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 04e3c3a commit ca6dc05

8 files changed

Lines changed: 546 additions & 45 deletions

File tree

‎crates/socket-patch-cli/tests/common/jvm_env.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ pub const AMBIENT: &[&str] = &[
3434
// ambient value would aim a test's crawl at a real cache.
3535
"COURSIER_CACHE",
3636
"SBT_OPTS",
37+
// The Maven local-repository lookup reads these (#423): `-D`s in
38+
// MAVEN_OPTS / MAVEN_ARGS and the install's conf/settings.xml.
39+
"MAVEN_OPTS",
40+
"MAVEN_ARGS",
41+
"MAVEN_HOME",
42+
"M2_HOME",
3743
];
3844

3945
/// The cache roots a test may hand the CLI explicitly (they survive

‎crates/socket-patch-core/src/crawlers/maven_crawler.rs‎

Lines changed: 182 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ use crate::gradle::{Env, Os};
1212
use crate::utils::fs::is_dir;
1313
use crate::vendor::jvm::layout::{self, BuildTool};
1414

15+
#[cfg(test)]
16+
mod local_repo_tests;
1517
#[cfg(test)]
1618
mod oracle;
1719
#[cfg(test)]
@@ -541,8 +543,8 @@ pub struct JvmEnv {
541543
}
542544

543545
impl JvmEnv {
544-
/// The caches of this process: `$MAVEN_REPO_LOCAL` / `$M2_HOME` /
545-
/// `~/.m2`, and [`gradle_cache::home_from_process_env`].
546+
/// The caches of this process: the Maven local repository
547+
/// ([`m2_repo_path_with`]), and [`gradle_cache::home_from_process_env`].
546548
pub fn from_process() -> Self {
547549
Self {
548550
m2_repo: MavenCrawler::m2_repo_path(),
@@ -554,7 +556,7 @@ impl JvmEnv {
554556
/// the passwd entry's), which Gradle prefers to `$HOME`.
555557
pub fn resolve(env: &dyn Env, os: Os, home_dir: Option<&Path>) -> Self {
556558
Self {
557-
m2_repo: m2_repo_path_with(env, home_dir),
559+
m2_repo: m2_repo_path_with(env, os, home_dir),
558560
gradle: GradleHome::resolve(env, os, home_dir),
559561
}
560562
}
@@ -594,28 +596,158 @@ pub fn is_ro_root(path: &Path) -> bool {
594596
JvmEnv::from_process().is_ro_root(path)
595597
}
596598

597-
/// The Maven local repository `env` names: `$MAVEN_REPO_LOCAL`, else
598-
/// `$M2_HOME/repository`, else `<home>/.m2/repository` with `<home>` =
599-
/// `$HOME`, `$USERPROFILE` or `home_dir`, the first that is set and rooted
600-
/// ([`crate::utils::fs::home_from_env`]). A set-but-empty variable counts
601-
/// as unset (see [`MavenCrawler::m2_repo_path`]). `None` when there is no
602-
/// usable home: a relative or literal `~` would resolve against the working
603-
/// directory and treat a `~/.m2` inside the scanned project as the user's
604-
/// local repository (B66).
605-
pub fn m2_repo_path_with(env: &dyn Env, home_dir: Option<&Path>) -> Option<PathBuf> {
599+
/// The Maven local repository `env` names, in Maven's own order (#423):
600+
///
601+
/// 1. `$MAVEN_REPO_LOCAL` (socket-patch's override);
602+
/// 2. `-Dmaven.repo.local` in `$MAVEN_ARGS`, then `$MAVEN_OPTS`;
603+
/// 3. `<localRepository>` in the user settings
604+
/// (`<user.home>/.m2/settings.xml`), then in the global settings
605+
/// (`conf/settings.xml` under `$MAVEN_HOME`, else `$M2_HOME`);
606+
/// 4. `<user.home>/.m2/repository`.
607+
///
608+
/// `M2_HOME` is the Maven install, never a local repository. `user.home`
609+
/// is `-Duser.home` in `$MAVEN_OPTS`, else the JVM's: on Windows the
610+
/// profile directory (`$USERPROFILE`, else `$HOME`, else `home_dir`),
611+
/// elsewhere `$HOME`, `$USERPROFILE` or `home_dir`, the first that is set
612+
/// and rooted ([`crate::utils::fs::home_from_env`]). A set-but-empty
613+
/// variable counts as unset (see [`MavenCrawler::m2_repo_path`]). A
614+
/// relative or unresolvable location (an undefined `${…}`) is skipped: it
615+
/// would resolve against whatever directory Maven runs in, not ours. `None`
616+
/// when nothing names a repository and there is no usable home: a relative
617+
/// or literal `~` would resolve against the working directory and treat a
618+
/// `~/.m2` inside the scanned project as the user's local repository (B66).
619+
pub fn m2_repo_path_with(env: &dyn Env, os: Os, home_dir: Option<&Path>) -> Option<PathBuf> {
620+
use crate::gradle::home::system_property;
621+
use crate::utils::fs::is_usable_home;
622+
606623
let set = |k: &str| env.var(k).filter(|v| !v.is_empty());
607624
if let Some(repo_local) = set("MAVEN_REPO_LOCAL") {
608625
return Some(PathBuf::from(repo_local));
609626
}
610-
if let Some(m2_home) = set("M2_HOME") {
611-
return Some(PathBuf::from(m2_home).join("repository"));
627+
// Later sources override earlier ones on the JVM: command-line `-D`s
628+
// (MAVEN_ARGS) are set over the launcher's MAVEN_OPTS.
629+
let property = |name: &str| {
630+
["MAVEN_ARGS", "MAVEN_OPTS"].iter().find_map(|k| {
631+
set(k)
632+
.and_then(|opts| system_property(&opts, name, os))
633+
.filter(|v| !v.is_empty())
634+
})
635+
};
636+
// `user.home` is fixed when the JVM starts: the launcher's MAVEN_OPTS
637+
// can move it, a command-line `-D` (MAVEN_ARGS) comes too late for the
638+
// `~/.m2` paths Maven derives from it.
639+
let user_home = set("MAVEN_OPTS")
640+
.and_then(|opts| system_property(&opts, "user.home", os))
641+
.map(PathBuf::from)
642+
.filter(|h| is_usable_home(h))
643+
.or_else(|| match os {
644+
Os::Windows => ["USERPROFILE", "HOME"]
645+
.iter()
646+
.filter_map(|k| set(k).map(PathBuf::from))
647+
.find(|h| is_usable_home(h)),
648+
Os::Unix => crate::utils::fs::home_from_env(|k| set(k).map(Into::into)),
649+
})
650+
.or_else(|| {
651+
home_dir
652+
.filter(|h| is_usable_home(h))
653+
.map(Path::to_path_buf)
654+
});
655+
if let Some(repo) = property("maven.repo.local")
656+
.map(PathBuf::from)
657+
.filter(|p| p.has_root())
658+
{
659+
return Some(repo);
660+
}
661+
let maven_home = set("MAVEN_HOME").or_else(|| set("M2_HOME"));
662+
let settings_files = [
663+
user_home
664+
.as_ref()
665+
.map(|h| h.join(".m2").join("settings.xml")),
666+
maven_home.map(|m| PathBuf::from(m).join("conf").join("settings.xml")),
667+
];
668+
let lookup = |name: &str| match name {
669+
"user.home" => user_home.as_ref().map(|h| h.display().to_string()),
670+
_ => match name.strip_prefix("env.") {
671+
Some(var) => set(var),
672+
None => property(name),
673+
},
674+
};
675+
for path in settings_files.into_iter().flatten() {
676+
if let Some(repo) = settings_local_repository(&path, &lookup) {
677+
return Some(repo);
678+
}
679+
}
680+
Some(user_home?.join(".m2").join("repository"))
681+
}
682+
683+
/// The `<localRepository>` a Maven `settings.xml` at `path` names, with
684+
/// `${…}` placeholders resolved through `lookup`. `None` when the file is
685+
/// missing, unreadable or not well formed where it matters, sets no
686+
/// `<localRepository>`, or names a relative or unresolvable location.
687+
fn settings_local_repository(
688+
path: &Path,
689+
lookup: &dyn Fn(&str) -> Option<String>,
690+
) -> Option<PathBuf> {
691+
use crate::formats::xml;
692+
693+
// FIFO-safe: a FIFO planted as settings.xml is refused, not opened.
694+
let raw = crate::utils::fs::read_regular_to_string_sync(path).ok()?;
695+
let text = xml::blank_non_markup(&raw).ok()?;
696+
let settings = *xml::elements(&text, "settings").ok()?.first()?;
697+
// `<localRepository>` is a child of `<settings>`; a profile property of
698+
// the same name is not it.
699+
let profiles = xml::children(&text, &settings, "profiles").ok()?;
700+
let element = xml::children(&text, &settings, "localRepository")
701+
.ok()?
702+
.into_iter()
703+
.find(|e| {
704+
!profiles
705+
.iter()
706+
.any(|p| e.start > p.start && e.start < p.end)
707+
})?;
708+
// Read from the unmasked text (offsets are kept), so CDATA is its
709+
// character data, entities decode and comments drop out, as Maven's
710+
// parser does.
711+
let value = character_data(&raw[element.inner_start..element.inner_end])?;
712+
let repo = PathBuf::from(interpolate(value.trim(), lookup)?);
713+
repo.has_root().then_some(repo)
714+
}
715+
716+
/// The character data of a text-only element's raw content: text with its
717+
/// entities decoded, CDATA sections verbatim, comments skipped. `None` when
718+
/// a comment or CDATA section is unterminated.
719+
fn character_data(inner: &str) -> Option<String> {
720+
let mut out = String::new();
721+
let mut rest = inner;
722+
while let Some(at) = rest.find("<!") {
723+
out.push_str(&crate::formats::nuget::decode_entities(&rest[..at]));
724+
rest = &rest[at..];
725+
if let Some(body) = rest.strip_prefix("<![CDATA[") {
726+
let end = body.find("]]>")?;
727+
out.push_str(&body[..end]);
728+
rest = &body[end + 3..];
729+
} else {
730+
let end = rest.strip_prefix("<!--")?.find("-->")?;
731+
rest = &rest[4 + end + 3..];
732+
}
612733
}
613-
let home = crate::utils::fs::home_from_env(|k| set(k).map(Into::into)).or_else(|| {
614-
home_dir
615-
.filter(|h| crate::utils::fs::is_usable_home(h))
616-
.map(Path::to_path_buf)
617-
})?;
618-
Some(home.join(".m2").join("repository"))
734+
out.push_str(&crate::formats::nuget::decode_entities(rest));
735+
Some(out)
736+
}
737+
738+
/// `text` with every `${name}` replaced by `lookup(name)`; `None` when one
739+
/// is undefined or unterminated.
740+
fn interpolate(text: &str, lookup: &dyn Fn(&str) -> Option<String>) -> Option<String> {
741+
let mut out = String::new();
742+
let mut rest = text;
743+
while let Some(at) = rest.find("${") {
744+
out.push_str(&rest[..at]);
745+
let end = rest[at..].find('}')? + at;
746+
out.push_str(&lookup(&rest[at + 2..end])?);
747+
rest = &rest[end + 1..];
748+
}
749+
out.push_str(rest);
750+
Some(out)
619751
}
620752

621753
/// A `--global-prefix` as the cache root it names: a Gradle user home
@@ -1103,7 +1235,7 @@ impl MavenCrawler {
11031235

11041236
/// Get the Maven local repository path.
11051237
///
1106-
/// Checks `$MAVEN_REPO_LOCAL`, `$M2_HOME/repository`, `$HOME/.m2/repository`.
1238+
/// See [`m2_repo_path_with`] for the order Maven itself uses.
11071239
///
11081240
/// A set-but-EMPTY variable counts as unset. `std::env::var` yields
11091241
/// `Ok("")` for `export MAVEN_REPO_LOCAL="$M2"` with `$M2` undefined —
@@ -1122,7 +1254,7 @@ impl MavenCrawler {
11221254
/// Same rule as `nuget_home()`, `deno_dir()`, `go_crawler`'s
11231255
/// `get_gomodcache`, and `utils::fs::home_dir`.
11241256
fn m2_repo_path() -> Option<PathBuf> {
1125-
m2_repo_path_with(&gradle_cache::ProcessEnv, None)
1257+
m2_repo_path_with(&gradle_cache::ProcessEnv, Os::current(), None)
11261258
}
11271259

11281260
/// Scan a Maven repository directory and return all valid packages found.
@@ -2414,22 +2546,27 @@ mod tests {
24142546
.map(|(k, v)| (k.to_string(), v.to_string()))
24152547
.collect()
24162548
};
2417-
assert_eq!(m2_repo_path_with(&env(&[]), None), None);
2418-
assert_eq!(m2_repo_path_with(&env(&[("HOME", "")]), None), None);
2419-
assert_eq!(m2_repo_path_with(&env(&[("HOME", "~")]), None), None);
2549+
let m2_repo_path_with_unix =
2550+
|e: &HashMap<String, String>, h| m2_repo_path_with(e, Os::Unix, h);
2551+
assert_eq!(m2_repo_path_with_unix(&env(&[]), None), None);
2552+
assert_eq!(m2_repo_path_with_unix(&env(&[("HOME", "")]), None), None);
2553+
assert_eq!(m2_repo_path_with_unix(&env(&[("HOME", "~")]), None), None);
24202554
assert_eq!(
2421-
m2_repo_path_with(&env(&[("HOME", "rel/home")]), Some(Path::new("also-rel"))),
2555+
m2_repo_path_with_unix(&env(&[("HOME", "rel/home")]), Some(Path::new("also-rel"))),
24222556
None
24232557
);
24242558
let abs = std::env::temp_dir();
24252559
let abs_s = abs.to_str().unwrap();
24262560
let want = Some(abs.join(".m2").join("repository"));
24272561
assert_eq!(
2428-
m2_repo_path_with(&env(&[("HOME", "rel"), ("USERPROFILE", abs_s)]), None),
2562+
m2_repo_path_with_unix(&env(&[("HOME", "rel"), ("USERPROFILE", abs_s)]), None),
24292563
want
24302564
);
2431-
assert_eq!(m2_repo_path_with(&env(&[("HOME", "~")]), Some(&abs)), want);
2432-
assert_eq!(m2_repo_path_with(&env(&[("HOME", abs_s)]), None), want);
2565+
assert_eq!(
2566+
m2_repo_path_with_unix(&env(&[("HOME", "~")]), Some(&abs)),
2567+
want
2568+
);
2569+
assert_eq!(m2_repo_path_with_unix(&env(&[("HOME", abs_s)]), None), want);
24332570
}
24342571

24352572
#[test]
@@ -2442,15 +2579,19 @@ mod tests {
24422579
// Maven patch was skipped — while Maven itself would have used the
24432580
// default local repo. Empty must fall through, exactly as
24442581
// `nuget_home()` treats an empty `NUGET_PACKAGES`.
2445-
let m2_home = tempfile::tempdir().unwrap();
2582+
let repo_local = tempfile::tempdir().unwrap();
24462583
let _local = EnvGuard::set("MAVEN_REPO_LOCAL", "");
2447-
let _m2 = EnvGuard::set("M2_HOME", m2_home.path().to_str().unwrap());
2584+
let _args = EnvGuard::unset("MAVEN_ARGS");
2585+
let _opts = EnvGuard::set(
2586+
"MAVEN_OPTS",
2587+
&format!("-Dmaven.repo.local={}", repo_local.path().display()),
2588+
);
24482589

24492590
let repo = MavenCrawler::m2_repo_path();
24502591
assert_eq!(
24512592
repo,
2452-
Some(m2_home.path().join("repository")),
2453-
"empty MAVEN_REPO_LOCAL must fall through to the M2_HOME arm, got {repo:?}"
2593+
Some(repo_local.path().to_path_buf()),
2594+
"empty MAVEN_REPO_LOCAL must fall through to -Dmaven.repo.local, got {repo:?}"
24542595
);
24552596
}
24562597

@@ -2464,7 +2605,15 @@ mod tests {
24642605
// directory inside the user's own project. Same CWD-relative hazard
24652606
// the `utils::fs::home_dir` and `go_crawler` empty-HOME fixes closed.
24662607
let _local = EnvGuard::unset("MAVEN_REPO_LOCAL");
2608+
let _args = EnvGuard::unset("MAVEN_ARGS");
2609+
let _opts = EnvGuard::unset("MAVEN_OPTS");
2610+
let _maven_home = EnvGuard::unset("MAVEN_HOME");
24672611
let _m2 = EnvGuard::set("M2_HOME", "");
2612+
// Point the home directory at an empty dir so a developer's own
2613+
// `~/.m2/settings.xml` <localRepository> can't change the result.
2614+
let home = tempfile::tempdir().unwrap();
2615+
let _home = EnvGuard::set("HOME", home.path().to_str().unwrap());
2616+
let _profile = EnvGuard::set("USERPROFILE", home.path().to_str().unwrap());
24682617

24692618
let repo = MavenCrawler::m2_repo_path();
24702619
assert_ne!(

0 commit comments

Comments
 (0)