Skip to content

Commit a4ebd05

Browse files
Fix Maven reactor ${prop} groupId/artifactId (#655) (#1773)
* Start fix for #655 Assisted-by: Claude Code:claude-opus-5-5 * Patch Maven deps named via ${prop} coordinates A reactor module that declares the patched library as <groupId>${ct.group}</groupId> / <artifactId>${ct.artifact}</artifactId> was skipped by the vendored Maven planner, which compared the raw element text with the patch coordinates. Maven interpolates those elements before matching, so the module kept its literal base version and built Central's unpatched jar while vendor, vendor --check and vex all reported it patched. The planner, the external-management check and the revert now match declarations on interpolated coordinates. A coordinate whose property is not defined in the checkout gets a property_unresolved warning instead of passing silently. Fixes #655 Assisted-by: Claude Code:claude-opus-5-5 * Guard ${prop} Maven coordinates edge cases Follow-ups from review of the #655 fix: - A module that inherits a ${prop}-named declaration and overrides the property names a different artifact; vendor now leaves the declaration alone and warns coordinates_overridden instead of pointing that module at a version that does not exist. - Declarations matched through properties get their own record-key ordinals, so revert of a project vendored by an older release still finds each literal declaration's original version. - ${project.groupId} and ${project.artifactId} resolve. - The unresolved-coordinate warning respects literal text around the property and skips classifier and non-jar variants, so unrelated dependencies no longer warn. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ec9aabc commit a4ebd05

2 files changed

Lines changed: 407 additions & 13 deletions

File tree

‎crates/socket-patch-cli/tests/e2e_vendor_jvm_build.rs‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,90 @@ fn maven_reactor_vendor_fresh_checkout_offline_build_and_byte_exact_revert() {
544544
);
545545
}
546546

547+
/// #655: `a` declares the library through `${ct.group}:${ct.artifact}`,
548+
/// properties the local corp parent defines. Maven interpolates them before
549+
/// matching, so vendor must rewrite `a`'s literal version too, or `a` builds
550+
/// Central's unpatched jar.
551+
#[test]
552+
#[ignore = "real Maven + Maven Central (fixture); run with --ignored"]
553+
fn maven_reactor_property_coordinates_vendor_fresh_offline_build_and_revert() {
554+
const SUITE: &str = "e2e_vendor_jvm_build::maven_reactor_property_coordinates";
555+
let Some(mvn) = Mvn::detect(SUITE) else {
556+
return;
557+
};
558+
let tmp = tempfile::tempdir().unwrap();
559+
let root = fixture_root(&tmp);
560+
let m2 = root.join("m2");
561+
let proj = root.join("proj");
562+
let settings = root.join("settings.xml");
563+
write_settings(&settings, &[]);
564+
let Some((jar, _)) = warm_fixture(SUITE, &mvn, &root.join("warm"), &m2, &settings) else {
565+
return;
566+
};
567+
write_reactor(&proj);
568+
let corp = CORP_PARENT_POM.replace(
569+
" </properties>",
570+
" <ct.group>org.apache.commons</ct.group>\n \
571+
<ct.artifact>commons-text</ct.artifact>\n </properties>",
572+
);
573+
std::fs::write(proj.join("corp-parent/pom.xml"), corp).unwrap();
574+
let a = a_pom()
575+
.replace(
576+
"<groupId>org.apache.commons</groupId>",
577+
"<groupId>${ct.group}</groupId>",
578+
)
579+
.replace(
580+
"<artifactId>commons-text</artifactId>",
581+
"<artifactId>${ct.artifact}</artifactId>",
582+
);
583+
std::fs::write(proj.join("a/pom.xml"), &a).unwrap();
584+
let out = with_central_fallback(SUITE, "pre-vendor reactor build", &settings, |s, flags| {
585+
mvn_classpath(&mvn, &proj, &m2, s, flags)
586+
});
587+
assert!(ok(&out), "pre-vendor reactor build:\n{}", dump(&out));
588+
assert_eq!(
589+
std::fs::read(classpath_entry(&proj.join("a"))).unwrap(),
590+
jar,
591+
"pre-vendor `a` resolves Central's jar through its property coordinates"
592+
);
593+
for dir in [
594+
"target",
595+
"corp-parent/target",
596+
"a/target",
597+
"b/target",
598+
".mvn/target",
599+
] {
600+
let _ = std::fs::remove_dir_all(proj.join(dir));
601+
}
602+
let _ = std::fs::remove_dir(proj.join(".mvn"));
603+
604+
let (orig, patched) = patched_member(&jar, UUID);
605+
stage_manifest(&proj, &orig, &patched);
606+
let before = snapshot(&proj);
607+
let env = vendor(&proj, &m2);
608+
assert_eq!(env["summary"]["applied"], 1, "{env}");
609+
let vendored = snapshot(&proj);
610+
assert_eq!(
611+
text(&vendored["a/pom.xml"]),
612+
a.replace(
613+
"<version>1.10.0</version>",
614+
&format!("<version>{SV}</version>")
615+
),
616+
"`a`'s literal version is rewritten"
617+
);
618+
619+
let fresh = root.join("fresh");
620+
fresh_checkout_all(&proj, &fresh);
621+
purge(&m2);
622+
let out = mvn_classpath(&mvn, &fresh, &m2, &settings, &["-o"]);
623+
assert!(ok(&out), "fresh offline reactor build:\n{}", dump(&out));
624+
assert_vendored_on_classpath(&fresh.join("a"), &patched, "module a");
625+
assert_vendored_on_classpath(&fresh.join("b"), &patched, "module b");
626+
627+
revert(&proj, &m2);
628+
assert_restored(&proj, &before);
629+
}
630+
547631
// ── P2: Gradle multi-project with dependency locking ────────────────────
548632

549633
const GRADLE_SETTINGS: &str = r#"buildscript {

0 commit comments

Comments
 (0)