Skip to content

Commit c07af61

Browse files
Fix Maven vendor reporting unwired patch applied (#513) (#1826)
* Start fix for #513 Assisted-by: Claude Code:claude-opus-5-5 * Refuse a Maven vendor that wires nothing When every reactor declaration of the patched package was left at an uncertain ${property} or another version, and no root could be pinned, vendor still wrote the tree, exited 0 and counted the patch as applied, while the build kept resolving the unpatched jar. vendor --check then reported the artifact as no longer wired. vendor now refuses such a plan (vendor_jvm_shape_unsupported, reason nothing_wired) with the degraded reasons that left it unwired, and writes nothing. Fixes #513 Assisted-by: Claude Code:claude-opus-5-5 * Narrow nothing_wired reasons, cover probe run The refusal now lists only the reasons that left a declaration or root unwired, not the repository and config notes. A probe run for a patch without files refuses the same reactor shapes the real vendor does, so the preview and the run agree. Assisted-by: Claude Code:claude-opus-5-5 * Pass the directory lister to the unwired check main now discovers Maven 4 implicit subprojects by listing directories (#1807), so wired_checked takes a lister. The nothing_wired check passes the same lister the plan used, so it sees the same reactor. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b291caa commit c07af61

3 files changed

Lines changed: 146 additions & 9 deletions

File tree

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,8 @@ fn gradle_vendor_533_unpatched_classifier_copy_withholds_vex() {
11561156
/// #513 / #459: a reactor `${prop}` version that can't be pinned with
11571157
/// certainty (undefined in the checkout, or raised by a profile) is left
11581158
/// as is with a warning, and `vex` does not attest the package the build
1159-
/// still takes unpatched.
1159+
/// still takes unpatched. With nothing else to wire, `vendor` refuses
1160+
/// rather than counting the patch as applied (#513).
11601161
#[test]
11611162
fn maven_reactor_uncertain_property_version_withholds_vex() {
11621163
let cases = [
@@ -1199,6 +1200,26 @@ fn maven_reactor_uncertain_property_version_withholds_vex() {
11991200
let b = pristine.replace("<artifactId>a</artifactId>", "<artifactId>b</artifactId>");
12001201
write(root, "proj/b/pom.xml", b.as_bytes());
12011202
}
1203+
let vex = root.join("vex.json");
1204+
if !sibling {
1205+
assert_refused(root, "nothing_wired", reason);
1206+
let (_, env) = socket(
1207+
root,
1208+
&[
1209+
"vex",
1210+
"-O",
1211+
vex.to_str().unwrap(),
1212+
"--product",
1213+
"pkg:generic/x@1",
1214+
],
1215+
);
1216+
assert!(!env.to_string().contains("not_affected"), "{reason}: {env}");
1217+
if let Ok(bytes) = std::fs::read(&vex) {
1218+
let doc = String::from_utf8_lossy(&bytes);
1219+
assert!(!doc.contains("not_affected"), "{reason}: {env}");
1220+
}
1221+
continue;
1222+
}
12021223
let env = ok(root, &["vendor"]);
12031224
assert!(
12041225
env.to_string().contains(&format!("reason: {reason}")),
@@ -1208,12 +1229,7 @@ fn maven_reactor_uncertain_property_version_withholds_vex() {
12081229
std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap(),
12091230
a
12101231
);
1211-
assert_eq!(
1212-
wiring_text(root).contains("-socket."),
1213-
sibling,
1214-
"{reason}: wired"
1215-
);
1216-
let vex = root.join("vex.json");
1232+
assert!(wiring_text(root).contains("-socket."), "{reason}: wired");
12171233
let (_, env) = socket(
12181234
root,
12191235
&[

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

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,65 @@ pub fn wired_checked(
478478
})
479479
}
480480

481+
/// Refuse `plan` when, written over `read`, no pom of the reactor would
482+
/// name `c`'s suffixed version: every declaration was left as is and no
483+
/// root pinned (#513). Vendoring it would count the patch as applied while
484+
/// the build keeps resolving the base; the refusal carries the degraded
485+
/// reasons that left it unwired.
486+
pub fn refuse_unwired(
487+
read: ReadFn<'_>,
488+
list: ListFn<'_>,
489+
c: &Coords<'_>,
490+
plan: &JvmPlan,
491+
) -> Result<(), JvmRefusal> {
492+
let planned = |rel: &str| match plan.writes.iter().find(|w| w.rel == rel) {
493+
Some(w) => Some(w.bytes.clone()),
494+
None => read(rel),
495+
};
496+
if wired_checked(&planned, list, c)? {
497+
return Ok(());
498+
}
499+
// Only the reasons that leave a declaration or a root alone; the
500+
// repository and config notes (and a mixed root's Gradle warnings)
501+
// say nothing about why the reactor is unwired.
502+
const UNWIRING: [&str; 7] = [
503+
"property_unresolved",
504+
"conflicting_literal_version",
505+
"conflicting_managed_version",
506+
"management_unresolved",
507+
"range",
508+
"coordinates_overridden",
509+
"classifier_declared",
510+
];
511+
let why: Vec<&str> = plan
512+
.warnings
513+
.iter()
514+
.filter(|w| w.code == DEGRADED)
515+
.map(|w| w.detail.as_str())
516+
.filter(|d| {
517+
UNWIRING
518+
.iter()
519+
.any(|r| d.starts_with(&format!("reason: {r}: ")))
520+
})
521+
.collect();
522+
let why = if why.is_empty() {
523+
"no declaration or pin of the reactor can take it".to_string()
524+
} else {
525+
why.join("; ")
526+
};
527+
Err(JvmRefusal {
528+
code: SHAPE_UNSUPPORTED,
529+
detail: format!(
530+
"reason: nothing_wired: no pom of the reactor would build {}:{}:{}, so Maven \
531+
keeps resolving {} ({why})",
532+
c.group_id,
533+
c.artifact_id,
534+
c.suffixed_version(),
535+
c.version
536+
),
537+
})
538+
}
539+
481540
/// [`wired_checked`], and no declaration of `c`'s g:a is left at a version
482541
/// that may still read the base: the base itself, as a module or
483542
/// dependency added since vendoring declares it (#584), or a `${...}` the
@@ -5379,6 +5438,51 @@ mod tests {
53795438
assert!(unwired(&after));
53805439
}
53815440

5441+
/// #513: a plan that leaves every declaration and root alone wires
5442+
/// nothing, so `vendor` refuses it with the reasons instead of
5443+
/// counting the patch as applied.
5444+
#[test]
5445+
fn a_plan_that_wires_nothing_is_refused() {
5446+
let parent = "<parent><groupId>com.corp</groupId><artifactId>corp-parent</artifactId>\
5447+
<version>3</version><relativePath/></parent>";
5448+
let mut files = external_reactor(parent, "");
5449+
let a = text(&files, "a/pom.xml").replace(
5450+
"</artifactId></dependency>",
5451+
"</artifactId><version>${ct.version}</version></dependency>",
5452+
);
5453+
files.insert("a/pom.xml".into(), a.into_bytes());
5454+
let plan = run(&files).unwrap();
5455+
let read = |p: &str| files.get(p).cloned();
5456+
let r = refuse_unwired(&read, &lister(&files), &patch().coords(), &plan).unwrap_err();
5457+
assert_eq!(r.code, SHAPE_UNSUPPORTED);
5458+
assert_eq!(refusal_reason(&r), "nothing_wired");
5459+
assert!(
5460+
r.detail.contains("reason: property_unresolved: "),
5461+
"{}",
5462+
r.detail
5463+
);
5464+
assert!(!r.detail.contains("maven_f_outside_root"), "{}", r.detail);
5465+
assert!(!r.detail.contains("maven_mirror_of_all"), "{}", r.detail);
5466+
}
5467+
5468+
/// The same check passes a plan that pins the root or rewrites a
5469+
/// declaration, and an already vendored reactor whose re-plan writes
5470+
/// nothing.
5471+
#[test]
5472+
fn a_wired_plan_is_not_refused() {
5473+
let files = fs(&[
5474+
("pom.xml", ROOT),
5475+
("a/pom.xml", &module("a", &dep("1.10.0"))),
5476+
("b/pom.xml", &module("b", "")),
5477+
]);
5478+
let (plan, after) = vendor(&files);
5479+
let read = |p: &str| files.get(p).cloned();
5480+
refuse_unwired(&read, &lister(&files), &patch().coords(), &plan).unwrap();
5481+
let again = run(&after).unwrap();
5482+
let read = |p: &str| after.get(p).cloned();
5483+
refuse_unwired(&read, &lister(&after), &patch().coords(), &again).unwrap();
5484+
}
5485+
53825486
/// #513 case 2: Maven 4.1.0 infers an empty `<parent/>` from
53835487
/// `../pom.xml`, so the root's property resolves and is rewritten.
53845488
#[test]

‎crates/socket-patch-core/src/vendor/maven_repo.rs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -779,10 +779,20 @@ async fn vendor_maven_jvm(
779779
);
780780
}
781781
// A declared classifier is sourced only for a real patch.
782-
if let Err(e) = plan {
783-
if !e.detail.starts_with("reason: classifier_unavailable:") {
782+
match plan {
783+
Err(e) if !e.detail.starts_with("reason: classifier_unavailable:") => {
784784
return refused(e.code, e.detail);
785785
}
786+
Ok(plan) if matches!(shape, Shape::MavenReactor | Shape::Mixed) => {
787+
let read = |rel: &str| reader.read(rel);
788+
let list = |dir: &str| reader.list(dir);
789+
if let Err(e) =
790+
super::jvm::maven_reactor::refuse_unwired(&read, &list, &patch.coords(), &plan)
791+
{
792+
return refused(e.code, e.detail);
793+
}
794+
}
795+
_ => {}
786796
}
787797
return done(
788798
synthesized_result(purl, &display_path, Vec::new(), true, None),
@@ -976,6 +986,13 @@ async fn vendor_maven_jvm(
976986
Ok(plan) => plan,
977987
Err(refusal) => return refused(refusal.code, refusal.detail),
978988
};
989+
if matches!(shape, Shape::MavenReactor | Shape::Mixed) {
990+
if let Err(refusal) =
991+
super::jvm::maven_reactor::refuse_unwired(&read, &list, &patch.coords(), &plan)
992+
{
993+
return refused(refusal.code, refusal.detail);
994+
}
995+
}
979996
if gradle
980997
&& reader
981998
.read(super::jvm::gradle::VERIFICATION_REL)

0 commit comments

Comments
 (0)