Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions crates/socket-patch-cli/tests/vendor_jvm_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,8 @@ fn gradle_vendor_533_unpatched_classifier_copy_withholds_vex() {
/// #513 / #459: a reactor `${prop}` version that can't be pinned with
/// certainty (undefined in the checkout, or raised by a profile) is left
/// as is with a warning, and `vex` does not attest the package the build
/// still takes unpatched.
/// still takes unpatched. With nothing else to wire, `vendor` refuses
/// rather than counting the patch as applied (#513).
#[test]
fn maven_reactor_uncertain_property_version_withholds_vex() {
let cases = [
Expand Down Expand Up @@ -1199,6 +1200,26 @@ fn maven_reactor_uncertain_property_version_withholds_vex() {
let b = pristine.replace("<artifactId>a</artifactId>", "<artifactId>b</artifactId>");
write(root, "proj/b/pom.xml", b.as_bytes());
}
let vex = root.join("vex.json");
if !sibling {
assert_refused(root, "nothing_wired", reason);
let (_, env) = socket(
root,
&[
"vex",
"-O",
vex.to_str().unwrap(),
"--product",
"pkg:generic/x@1",
],
);
assert!(!env.to_string().contains("not_affected"), "{reason}: {env}");
if let Ok(bytes) = std::fs::read(&vex) {
let doc = String::from_utf8_lossy(&bytes);
assert!(!doc.contains("not_affected"), "{reason}: {env}");
}
continue;
}
let env = ok(root, &["vendor"]);
assert!(
env.to_string().contains(&format!("reason: {reason}")),
Expand All @@ -1208,12 +1229,7 @@ fn maven_reactor_uncertain_property_version_withholds_vex() {
std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap(),
a
);
assert_eq!(
wiring_text(root).contains("-socket."),
sibling,
"{reason}: wired"
);
let vex = root.join("vex.json");
assert!(wiring_text(root).contains("-socket."), "{reason}: wired");
let (_, env) = socket(
root,
&[
Expand Down
104 changes: 104 additions & 0 deletions crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,65 @@ pub fn wired_checked(
})
}

/// Refuse `plan` when, written over `read`, no pom of the reactor would
/// name `c`'s suffixed version: every declaration was left as is and no
/// root pinned (#513). Vendoring it would count the patch as applied while
/// the build keeps resolving the base; the refusal carries the degraded
/// reasons that left it unwired.
pub fn refuse_unwired(
read: ReadFn<'_>,
list: ListFn<'_>,
c: &Coords<'_>,
plan: &JvmPlan,
) -> Result<(), JvmRefusal> {
let planned = |rel: &str| match plan.writes.iter().find(|w| w.rel == rel) {
Some(w) => Some(w.bytes.clone()),
None => read(rel),
};
if wired_checked(&planned, list, c)? {
return Ok(());
}
// Only the reasons that leave a declaration or a root alone; the
// repository and config notes (and a mixed root's Gradle warnings)
// say nothing about why the reactor is unwired.
const UNWIRING: [&str; 7] = [
"property_unresolved",
"conflicting_literal_version",
"conflicting_managed_version",
"management_unresolved",
"range",
"coordinates_overridden",
"classifier_declared",
];
let why: Vec<&str> = plan
.warnings
.iter()
.filter(|w| w.code == DEGRADED)
.map(|w| w.detail.as_str())
.filter(|d| {
UNWIRING
.iter()
.any(|r| d.starts_with(&format!("reason: {r}: ")))
})
.collect();
let why = if why.is_empty() {
"no declaration or pin of the reactor can take it".to_string()
} else {
why.join("; ")
};
Err(JvmRefusal {
code: SHAPE_UNSUPPORTED,
detail: format!(
"reason: nothing_wired: no pom of the reactor would build {}:{}:{}, so Maven \
keeps resolving {} ({why})",
c.group_id,
c.artifact_id,
c.suffixed_version(),
c.version
),
})
}

/// [`wired_checked`], and no declaration of `c`'s g:a is left at a version
/// that may still read the base: a `${...}` the checkout does not define
/// or that an inheriting pom or a profile moves (#513, #459). Such a
Expand Down Expand Up @@ -5376,6 +5435,51 @@ mod tests {
assert!(unwired(&after));
}

/// #513: a plan that leaves every declaration and root alone wires
/// nothing, so `vendor` refuses it with the reasons instead of
/// counting the patch as applied.
#[test]
fn a_plan_that_wires_nothing_is_refused() {
let parent = "<parent><groupId>com.corp</groupId><artifactId>corp-parent</artifactId>\
<version>3</version><relativePath/></parent>";
let mut files = external_reactor(parent, "");
let a = text(&files, "a/pom.xml").replace(
"</artifactId></dependency>",
"</artifactId><version>${ct.version}</version></dependency>",
);
files.insert("a/pom.xml".into(), a.into_bytes());
let plan = run(&files).unwrap();
let read = |p: &str| files.get(p).cloned();
let r = refuse_unwired(&read, &lister(&files), &patch().coords(), &plan).unwrap_err();
assert_eq!(r.code, SHAPE_UNSUPPORTED);
assert_eq!(refusal_reason(&r), "nothing_wired");
assert!(
r.detail.contains("reason: property_unresolved: "),
"{}",
r.detail
);
assert!(!r.detail.contains("maven_f_outside_root"), "{}", r.detail);
assert!(!r.detail.contains("maven_mirror_of_all"), "{}", r.detail);
}

/// The same check passes a plan that pins the root or rewrites a
/// declaration, and an already vendored reactor whose re-plan writes
/// nothing.
#[test]
fn a_wired_plan_is_not_refused() {
let files = fs(&[
("pom.xml", ROOT),
("a/pom.xml", &module("a", &dep("1.10.0"))),
("b/pom.xml", &module("b", "")),
]);
let (plan, after) = vendor(&files);
let read = |p: &str| files.get(p).cloned();
refuse_unwired(&read, &lister(&files), &patch().coords(), &plan).unwrap();
let again = run(&after).unwrap();
let read = |p: &str| after.get(p).cloned();
refuse_unwired(&read, &lister(&after), &patch().coords(), &again).unwrap();
}

/// #513 case 2: Maven 4.1.0 infers an empty `<parent/>` from
/// `../pom.xml`, so the root's property resolves and is rewritten.
#[test]
Expand Down
21 changes: 19 additions & 2 deletions crates/socket-patch-core/src/vendor/maven_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,10 +779,20 @@ async fn vendor_maven_jvm(
);
}
// A declared classifier is sourced only for a real patch.
if let Err(e) = plan {
if !e.detail.starts_with("reason: classifier_unavailable:") {
match plan {
Err(e) if !e.detail.starts_with("reason: classifier_unavailable:") => {
return refused(e.code, e.detail);
}
Ok(plan) if matches!(shape, Shape::MavenReactor | Shape::Mixed) => {
let read = |rel: &str| reader.read(rel);
let list = |dir: &str| reader.list(dir);
if let Err(e) =
super::jvm::maven_reactor::refuse_unwired(&read, &list, &patch.coords(), &plan)
{
return refused(e.code, e.detail);
}
}
_ => {}
}
return done(
synthesized_result(purl, &display_path, Vec::new(), true, None),
Expand Down Expand Up @@ -976,6 +986,13 @@ async fn vendor_maven_jvm(
Ok(plan) => plan,
Err(refusal) => return refused(refusal.code, refusal.detail),
};
if matches!(shape, Shape::MavenReactor | Shape::Mixed) {
if let Err(refusal) =
super::jvm::maven_reactor::refuse_unwired(&read, &list, &patch.coords(), &plan)
{
return refused(refusal.code, refusal.detail);
}
}
if gradle
&& reader
.read(super::jvm::gradle::VERIFICATION_REL)
Expand Down
Loading