From 25c8a4f0b80c1bff5d2d4d56dfdc6d26005dfddf Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Thu, 12 Nov 2026 10:37:15 +0100 Subject: [PATCH 1/2] fix(manifest): record every resolved Maven dependency edge Maven facts only had the edges that survive conflict resolution, so an edge onto a dependency Maven kept elsewhere (a direct dependency, or a mediated version) was missing, unlike Gradle and sbt facts. The Maven extension now records each node's children before conflict resolution and points every edge at the node Maven kept. Roots follow Gradle's runtime and test classpaths: the test root is the module's whole resolution, the compile root the nodes Maven kept with a prod scope. A dependency Maven widens to compile under a test parent is therefore prod, matching Maven's runtime classpath. A module whose conflict resolution fails has no resolved graph, so it is reported unscannable and contributes no dependencies. --- CHANGELOG.md | 2 + .../socket/SocketFactsRecordsEngine.java | 184 +++++++---- .../scripts/test/maven-compat/.gitignore | 3 + .../test/maven-compat/assert-records.py | 47 ++- .../test/maven-compat/project/app/pom.xml | 30 +- .../test/maven-compat/project/mid/pom.xml | 11 + .../smoke-test-unsolvable-conflict.sh | 56 ++++ .../scripts/test/maven-compat/smoke-test.sh | 13 +- .../maven-compat/unsolvable-conflict/pom.xml | 42 +++ .../manifest/scripts/test/run-compat.sh | 1 + test/maven-compat-smoke-assertions.test.mts | 298 ++++++++++-------- 11 files changed, 485 insertions(+), 202 deletions(-) create mode 100755 src/commands/manifest/scripts/test/maven-compat/smoke-test-unsolvable-conflict.sh create mode 100644 src/commands/manifest/scripts/test/maven-compat/unsolvable-conflict/pom.xml diff --git a/CHANGELOG.md b/CHANGELOG.md index b7002ef4ec..2e0007ac55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed - When `dotnet restore` fails in one .NET project, the projects it may have skipped as up to date are no longer reported as not restored. +- Maven Socket facts now include every dependency edge Maven resolves, as Gradle and sbt facts do, so dependency chains show each path to a package. A dependency Maven puts on the runtime classpath is no longer marked as a dev dependency because Maven kept it under a test or provided dependency. +- With `--ignore-unresolved`, a Maven module with unresolvable version conflicts no longer adds a partial dependency list. ## [1.14.1](https://github.com/SocketDev/socket-cli/releases/tag/v1.14.1) - 2026-11-06 diff --git a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java index 93c8ad272f..60d06ecc33 100644 --- a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java +++ b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java @@ -9,9 +9,13 @@ import org.apache.maven.project.DependencyResolutionResult; import org.apache.maven.project.MavenProject; import org.apache.maven.project.ProjectDependenciesResolver; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.RepositoryException; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.RequestTrace; import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.collection.DependencyGraphTransformationContext; +import org.eclipse.aether.collection.DependencyGraphTransformer; import org.eclipse.aether.graph.Dependency; import org.eclipse.aether.graph.DependencyFilter; import org.eclipse.aether.graph.DependencyNode; @@ -46,8 +50,9 @@ /** * Emits each reactor module's resolved dependency graph as line-protocol records for the TS assembler - * (same contract as the Gradle/SBT scripts; no JSON/hashing here). Per module: a prod root - * (compile/runtime/system) and a dev root (test/provided). A reactor module becomes a component only + * (same contract as the Gradle/SBT scripts; no JSON/hashing here). Per module: a test root with the + * whole resolution and a compile root with the nodes kept with a prod scope (compile/runtime/system), + * both with every resolved edge. A reactor module becomes a component only * where another depends on it, by its bare {@code groupId:artifactId:version} id. * *

Resolution goes through {@link ProjectDependenciesResolver} — the same component Maven's own @@ -79,6 +84,8 @@ public static final class Options { private static final String ARTIFACT_PROPERTY_TYPE = "type"; // ConflictResolver.NODE_DATA_WINNER, inlined so nothing here needs maven-resolver-util. private static final String NODE_DATA_CONFLICT_WINNER = "conflict.winner"; + // TransformationContextKeys.CONFLICT_IDS, inlined for the same reason. + private static final String CONTEXT_CONFLICT_IDS = "conflictIds"; private final ProjectDependenciesResolver dependenciesResolver; private final RepositorySystem repositorySystem; @@ -161,10 +168,8 @@ public void run(MavenSession session, List reactor, File rootDir, if (SocketSupport.isExcludedDir(excludeBase, module.getBasedir().toPath(), excludes)) continue; Map nodes = new LinkedHashMap<>(); Set directIds = new HashSet<>(); - // Which direct dependencies are prod-scoped: seeds the prod/dev root split (see emitModuleRoots). - Set directProdIds = new HashSet<>(); - collectModule(session, module, passingScopes, reactorGavs, populateGavs, opts, nodes, directIds, directProdIds, failures, unscannable); - rootIdx = emitModuleRoots(lines, rootIdx, projectKey(module), nodes, directIds, directProdIds); + collectModule(session, module, passingScopes, reactorGavs, populateGavs, opts, nodes, directIds, failures, unscannable); + rootIdx = emitModuleRoots(lines, rootIdx, projectKey(module), nodes, directIds); } for (Failure f : failures) rec(lines, "failure", f.coord, f.detail, f.config, f.project); @@ -190,7 +195,6 @@ private void collectModule( Options opts, Map nodes, Set directIds, - Set directProdIds, Set failures, Set unscannable) { String moduleCoord = module.getGroupId() + ":" + module.getArtifactId() + ":" + module.getVersion(); @@ -198,9 +202,11 @@ private void collectModule( DependencyResolutionException thrown = null; DependencyFilter resolutionFilter = materializationFilter(opts, passingScopes, reactorGavs, populateGavs); observer.reset(); + EdgeRecorder recorder = new EdgeRecorder(session.getRepositorySession().getDependencyGraphTransformer()); + DefaultRepositorySystemSession recording = new DefaultRepositorySystemSession(session.getRepositorySession()); + recording.setDependencyGraphTransformer(recorder); try { - DependencyResolutionRequest request = - new DefaultDependencyResolutionRequest(module, session.getRepositorySession()); + DependencyResolutionRequest request = new DefaultDependencyResolutionRequest(module, recording); request.setResolutionFilter(resolutionFilter); result = dependenciesResolver.resolve(request); } catch (DependencyResolutionException e) { @@ -215,7 +221,8 @@ private void collectModule( } } boolean reported = false; - DependencyNode graph = result.getDependencyGraph(); + // Conflict resolution threw: the result holds only the collected candidates, not a resolved graph. + DependencyNode graph = recorder.resolved() ? result.getDependencyGraph() : null; List collectionFailures = new ArrayList<>(); for (Exception e : result.getCollectionErrors()) { FailedRequest failed = failedRequest(e); @@ -297,17 +304,11 @@ private void collectModule( log.warn("[socket-facts] could not resolve dependencies for " + moduleCoord + ": " + rootMessage(thrown)); } if (graph == null) return; + Map> edges = recorder.resolvedEdges(graph); Set visited = new HashSet<>(); - for (DependencyNode child : graph.getChildren()) { - String id = visit(child, passingScopes, reactorGavs, unresolved, opts, nodes, visited); - if (id != null) { - directIds.add(id); - // A direct dependency's effective scope is its declared scope (Aether's scope selector keeps - // a depth-1 item's scope), so it reliably seeds the prod/dev split. The node's own `prod` flag - // does NOT: conflict resolution can keep a node under a dev parent while its effective scope - // is prod (or vice versa). - if (isProd(scopeOf(child.getDependency()))) directProdIds.add(id); - } + for (DependencyNode child : edges.get(graph)) { + String id = visit(child, edges, passingScopes, reactorGavs, unresolved, opts, nodes, visited); + if (id != null) directIds.add(id); } for (String id : unfetched) { Node node = nodes.get(id); @@ -395,6 +396,7 @@ private void reportUnfetched( private String visit( DependencyNode dn, + Map> edges, Set passingScopes, Set reactorGavs, Set unresolved, @@ -404,9 +406,6 @@ private String visit( Dependency dep = dn.getDependency(); Artifact artifact = dn.getArtifact(); if (dep == null || artifact == null) return null; - // A verbose collect — Maven's -X turns one on — keeps conflict-losing nodes in the graph, tagged - // with the winner they lost to. Only the winner is on the classpath Maven would build. - if (dn.getData().get(NODE_DATA_CONFLICT_WINNER) != null) return null; String scope = scopeOf(dep); if (!passingScopes.contains(scope)) return null; @@ -419,14 +418,17 @@ private String visit( String key = externalId(artifact); String id = internal ? SocketSupport.bareId(artifact.getGroupId(), artifact.getArtifactId(), version) : key; - // One walk per artifact: a shared subtree is already recorded. Keyed by coordinate, as a sibling's jar - // and test-jar share a node but Maven can keep the sibling's dependencies under either. - if (!visited.add(key)) return id; - Node node = internal ? upsert(nodes, id, artifact.getGroupId(), artifact.getArtifactId(), "", "", version, gav) : upsert(nodes, id, artifact.getGroupId(), artifact.getArtifactId(), type == null ? "" : type, classifier == null ? "" : classifier, version, ""); + // A sibling's jar and test-jar share a node, each kept with its own scope. + if (isProd(scope)) node.prod = true; + + // One walk per artifact: a shared subtree is already recorded. Keyed by coordinate, as a sibling's jar + // and test-jar share a node but Maven can keep the sibling's dependencies under either. + if (!visited.add(key)) return id; + // Maven wrote each accepted node's resolved file back onto the node; a reactor module reports its // own dirs through its `project` record instead of a `file` record. if (!internal && opts.withFiles) { @@ -434,9 +436,10 @@ private String visit( if (file != null) node.files.add(file); } - for (DependencyNode child : dn.getChildren()) { - String childId = visit(child, passingScopes, reactorGavs, unresolved, opts, nodes, visited); - if (childId != null) node.children.add(childId); + for (DependencyNode child : edges.get(dn)) { + String childId = visit(child, edges, passingScopes, reactorGavs, unresolved, opts, nodes, visited); + // A cycle back onto the node's own artifact, or a sibling's jar and test-jar, which share an id. + if (childId != null && !childId.equals(id)) node.children.add(childId); } return id; } @@ -478,45 +481,21 @@ private static Node upsert( // ---- emission ---- - // Split a module's resolved nodes into a prod root and a dev root. Membership is decided by which - // direct dependency a node is REACHABLE from, not by the node's own effective scope: Maven's - // conflict resolution can retain a node under a dev parent while its effective scope is prod (the - // retained path came from a test/provided dep, but a removed deeper path had a compile scope, so - // Aether's scope selector unions them to compile). Partitioning by scope would put that node in the - // prod root while its only parent sits in the dev root, and emitRoot drops the cross-root edge — - // leaving the node unreachable and non-direct (the "orphaned component" defect). Reachability also - // keeps edges intra-root, because a node reached from a prod direct dependency stays prod. - // Empty roots are skipped. + // Shaped like Gradle's runtime and test classpaths. An edge from a prod node onto a dev one, such as + // onto a dependency the module declares `provided`, lives in the test root only. private int emitModuleRoots( - List lines, int rootIdx, String projectKey, Map nodes, - Set directIds, Set directProdIds) { - Set prodReachable = reachable(nodes, directProdIds); + List lines, int rootIdx, String projectKey, Map nodes, Set directIds) { Map prod = new LinkedHashMap<>(); - Map dev = new LinkedHashMap<>(); - for (Node n : nodes.values()) (prodReachable.contains(n.id) ? prod : dev).put(n.id, n); + for (Node n : nodes.values()) if (n.prod) prod.put(n.id, n); if (!prod.isEmpty()) { rootIdx = emitRoot(lines, rootIdx, projectKey, "compile", true, prod, directIds); } - if (!dev.isEmpty()) { - rootIdx = emitRoot(lines, rootIdx, projectKey, "test", false, dev, directIds); + if (!nodes.isEmpty()) { + rootIdx = emitRoot(lines, rootIdx, projectKey, "test", false, nodes, directIds); } return rootIdx; } - // Every node id reachable from `seeds` by following retained child edges. - private static Set reachable(Map nodes, Set seeds) { - Set seen = new HashSet<>(); - Deque stack = new ArrayDeque<>(seeds); - while (!stack.isEmpty()) { - String id = stack.pop(); - if (!seen.add(id)) continue; - Node node = nodes.get(id); - if (node == null) continue; - for (String child : node.children) stack.push(child); - } - return seen; - } - private int emitRoot( List lines, int rootIdx, String projectKey, String config, boolean prod, Map nodeMap, Set directIds) { String rootId = Integer.toString(rootIdx); @@ -873,6 +852,90 @@ public int hashCode() { } } + // Records each node's children before conflict resolution removes the losers: every resolved edge, + // including one onto a dependency Maven kept elsewhere, possibly at another version. + private static final class EdgeRecorder implements DependencyGraphTransformer { + private final DependencyGraphTransformer delegate; + private Map> collected; + private Map conflictIds; + private boolean resolved; + + EdgeRecorder(DependencyGraphTransformer delegate) { + this.delegate = delegate; + } + + @Override + public DependencyNode transformGraph(DependencyNode root, DependencyGraphTransformationContext context) + throws RepositoryException { + // Resolver 2 may collect again when a DependencyCollectionChecker asks; the last run is the result. + resolved = false; + Map> children = new IdentityHashMap<>(); + // The collector gives nodes of one artifact and context one shared child list. + Map, List> copies = new IdentityHashMap<>(); + Deque todo = new ArrayDeque<>(); + todo.push(root); + while (!todo.isEmpty()) { + DependencyNode n = todo.pop(); + if (children.containsKey(n)) continue; + List copy = copies.get(n.getChildren()); + if (copy == null) { + copy = new ArrayList<>(n.getChildren()); + copies.put(n.getChildren(), copy); + todo.addAll(copy); + } + children.put(n, copy); + } + DependencyNode transformed = delegate.transformGraph(root, context); + collected = children; + conflictIds = (Map) context.get(CONTEXT_CONFLICT_IDS); + resolved = true; + return transformed; + } + + /** Whether Maven's conflict resolution completed, so the resolution result holds its graph. */ + boolean resolved() { + return resolved; + } + + /** + * The root and each node Maven kept, mapped to its children by every resolved edge, a child that + * lost its conflict replaced by the node kept for its conflict id. Releases the recorded graph. + */ + Map> resolvedEdges(DependencyNode graph) { + if (conflictIds == null) throw new IllegalStateException("Maven's graph transformer assigned no conflict ids"); + Set classpath = onClasspath(graph); + Map kept = new HashMap<>(); + for (DependencyNode n : classpath) { + if (n == graph) continue; + DependencyNode other = kept.put(conflictId(n), n); + if (other != null) { + throw new IllegalStateException("Maven kept two nodes for one conflict: " + other + " and " + n); + } + } + Map> edges = new IdentityHashMap<>(); + for (DependencyNode parent : classpath) { + List children = collected.get(parent); + if (children == null) throw new IllegalStateException(parent + " was kept but never collected"); + List resolved = new ArrayList<>(); + for (DependencyNode child : children) { + // None kept: the resolved graph has no such dependency, so neither has the edge. + DependencyNode winner = kept.get(conflictId(child)); + if (winner != null) resolved.add(winner); + } + edges.put(parent, resolved); + } + collected = null; + conflictIds = null; + return edges; + } + + private Object conflictId(DependencyNode n) { + Object id = conflictIds.get(n); + if (id == null) throw new IllegalStateException("Maven's graph transformer assigned " + n + " no conflict id"); + return id; + } + } + private static final class Node { final String id; final String groupId; @@ -884,6 +947,7 @@ private static final class Node { final String project; final TreeSet children = new TreeSet<>(); final TreeSet files = new TreeSet<>(); + boolean prod; boolean unfetched; Node(String id, String groupId, String artifactId, String type, String classifier, String version, String project) { diff --git a/src/commands/manifest/scripts/test/maven-compat/.gitignore b/src/commands/manifest/scripts/test/maven-compat/.gitignore index 9867fa6a1e..eca4326139 100644 --- a/src/commands/manifest/scripts/test/maven-compat/.gitignore +++ b/src/commands/manifest/scripts/test/maven-compat/.gitignore @@ -16,3 +16,6 @@ graph-failure/target/ descriptor-failure/records.tsv descriptor-failure/target/ descriptor-failure/*/target/ +unsolvable-conflict/records.tsv +unsolvable-conflict/target/ +unsolvable-conflict/localrepo/ diff --git a/src/commands/manifest/scripts/test/maven-compat/assert-records.py b/src/commands/manifest/scripts/test/maven-compat/assert-records.py index 5638ca82c2..80bbbcdbe9 100755 --- a/src/commands/manifest/scripts/test/maven-compat/assert-records.py +++ b/src/commands/manifest/scripts/test/maven-compat/assert-records.py @@ -6,14 +6,20 @@ import sys rows = [l.rstrip('\n').split('\t') for l in open(sys.argv[1]) if l.strip()] tool = None -roots, nodes, files, direct = {}, {}, {}, {} +roots, root_projects, nodes, files, direct, edges = {}, {}, {}, {}, {}, {} for r in rows: if r[0] == 'meta': tool = r[1] - elif r[0] == 'root': roots[r[1]] = (r[4] == '1') # rootId -> prod + elif r[0] == 'root': + # rootId to prod and to projectKey. + roots[r[1]] = (r[4] == '1') + root_projects[r[1]] = r[2] elif r[0] == 'node': nodes.setdefault(r[2], set()).add(r[1]) # coordId -> {rootId} if r[8] == '1': direct.setdefault(r[2], set()).add(r[1]) # coordId -> {rootId where direct} elif r[0] == 'file': files.setdefault(r[2], set()).add(r[3]) # coordId -> {path} + elif r[0] == 'edge': + # (parent, child) to the roots that hold the edge. + edges.setdefault((r[2], r[3]), set()).add(r[1]) errors = [] @@ -61,15 +67,42 @@ def has_jar(cid): return any(p.endswith('.jar') for p in files.get(cid, ())) elif not in_prod(lib): errors.append("internal module demo:lib not in app's prod root") elif not direct.get(lib): errors.append("internal module demo:lib not marked direct") -# Scope conflict: Maven keeps demo.ext:shared under the test parent while its effective -# scope is compile, so it must land in the dev root. Reachability (checked by -# ../assert-reachability.py) additionally proves it kept its parent edge. +def edge_roots(parent, child): + return edges.get((parent, child), set()) + +# Scope conflict: Maven keeps demo.ext:shared under the test parent but selects compile, the +# scope conflict-prod's path gives it, so it is on the runtime classpath. Its edge from +# conflict-prod, onto the copy Maven kept under the test parent, keeps it reachable in the prod root. shared = coord('demo.ext:shared:jar:') +conflict_prod = coord('demo.ext:conflict-prod:jar:') if not shared: errors.append("missing scope-conflict dep demo.ext:shared") -elif in_prod(shared): errors.append("scope-conflict demo.ext:shared wrongly in a prod root") +elif not in_prod(shared): errors.append("scope-conflict demo.ext:shared not in a prod root") +elif not any(roots.get(rid) for rid in edge_roots(conflict_prod, shared)): + errors.append("edge demo.ext:conflict-prod -> demo.ext:shared missing from a prod root") + +# An edge onto a dependency Maven kept elsewhere: dup-io's dup-util is the direct one. +dup_io, dup_util = coord('demo.ext:dup-io:jar:'), coord('demo.ext:dup-util:jar:') +if not dup_io or not dup_util: errors.append("missing demo.ext:dup-io or demo.ext:dup-util") +elif not edge_roots(dup_io, dup_util): errors.append("edge demo.ext:dup-io -> demo.ext:dup-util missing") + +# A mediated edge: med-io asks for another med-util than mid declares, and points at mid's. One +# med-util coordinate in all, so the version med-io asked for was never emitted. +med_io, med_util = coord('demo.ext:med-io:jar:'), coord('demo.ext:med-util:jar:') +if not med_io or not med_util: errors.append("missing demo.ext:med-io or demo.ext:med-util") +elif not any(root_projects.get(rid) == 'demo:mid:1.0' for rid in edge_roots(med_io, med_util)): + errors.append("edge demo.ext:med-io -> demo.ext:med-util missing from mid") + +# An edge across the prod/dev split: container-lib (compile) onto container-api, which app +# declares provided. The edge is kept and container-api stays dev. +container_lib, container_api = coord('demo.ext:container-lib:jar:'), coord('demo.ext:container-api:jar:') +if not container_lib or not container_api: errors.append("missing demo.ext:container-lib or demo.ext:container-api") +elif in_prod(container_api): errors.append("provided demo.ext:container-api wrongly in a prod root") +elif not in_prod(container_lib): errors.append("demo.ext:container-lib not in a prod root") +elif not edge_roots(container_lib, container_api): + errors.append("edge demo.ext:container-lib -> demo.ext:container-api missing") if errors: print("FAIL:") for e in errors: print(" -", e) sys.exit(1) -print(f"PASS: tool=maven; {ext} prod+jar; harness/harness-core dev; internal demo:lib (bare id, direct); {shared} dev") +print(f"PASS: tool=maven; {ext} prod+jar; harness/harness-core dev; internal demo:lib (bare id, direct); {shared} prod; edges onto kept dependencies") diff --git a/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml b/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml index 10d2d6fbe4..b414ec39a6 100644 --- a/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml +++ b/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml @@ -31,10 +31,9 @@ test + dep is declared first, Maven keeps shared under the test parent but selects compile, the + scope conflict-prod's path gives it, so shared is prod and reached through + conflict-prod's edge. --> demo.ext conflict-test @@ -46,6 +45,29 @@ conflict-prod 1.0 + + + demo.ext + dup-util + 1.0 + + + demo.ext + dup-io + 1.0 + + + + demo.ext + container-api + 1.0 + provided + + + demo.ext + container-lib + 1.0 + demo lib diff --git a/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml b/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml index 8feda9593c..e48fd27f01 100644 --- a/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml +++ b/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml @@ -13,5 +13,16 @@ lib 1.0 + + + demo.ext + med-util + 1.0 + + + demo.ext + med-io + 1.0 + diff --git a/src/commands/manifest/scripts/test/maven-compat/smoke-test-unsolvable-conflict.sh b/src/commands/manifest/scripts/test/maven-compat/smoke-test-unsolvable-conflict.sh new file mode 100755 index 0000000000..ce8f69f681 --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/smoke-test-unsolvable-conflict.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# Regression guard: when Maven's conflict resolution throws (hard version ranges no version +# satisfies), Maven resolves no graph for the module. It must be reported unscannable and none of its +# collected dependencies emitted or fetched, however their scopes were left mid-resolution. +# +# Usage: smoke-test-unsolvable-conflict.sh +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +MVN="${1:?usage: smoke-test-unsolvable-conflict.sh }" +JAR="${2:?usage: smoke-test-unsolvable-conflict.sh }" +PROJECT="$HERE/unsolvable-conflict" +RECORDS="$PROJECT/records.tsv" +STUBS="$PROJECT/localrepo" +# shellcheck source=SCRIPTDIR/../compat-cache.sh +. "$HERE/../compat-cache.sh" +M2="$SOCKET_COMPAT_CACHE/m2" + +bash "$HERE/../make-stub-repo.sh" "$STUBS" \ + 'demo.unsolvable:left:1.0+demo.unsolvable:shared:[1.0]' \ + 'demo.unsolvable:right:1.0+demo.unsolvable:shared:[2.0]' \ + 'demo.unsolvable:harness:1.0+demo.unsolvable:harness-core:1.0' \ + 'demo.unsolvable:harness-core:1.0' \ + 'demo.unsolvable:shared:1.0' \ + 'demo.unsolvable:shared:2.0' +rm -rf "$M2/demo/unsolvable" "$RECORDS" + +echo "+ $("$MVN" -v 2>/dev/null | head -1) (unsolvable conflict)" +( cd "$PROJECT" && "$MVN" --batch-mode -q \ + "-Dmaven.ext.class.path=$JAR" \ + -Dcoana.task=socket-facts \ + -Dsocket.withFiles=true \ + "-Dmaven.repo.local=$M2" \ + "-Dstub.repo.url=file://$STUBS" \ + "-Dsocket.recordsFile=$RECORDS" \ + validate ) + +python3 - "$RECORDS" "$M2/demo/unsolvable" <<'PY' +import os, sys +rows = [l.rstrip('\n').split('\t') for l in open(sys.argv[1]) if l.strip()] +module = 'demo:unsolvable-conflict:1.0' +errors = [] +if not any(r[0] == 'unscannable' and r[3] == module for r in rows): + errors.append(f"{module} must be reported unscannable; records={rows}") +nodes = sorted(r[2] for r in rows if r[0] == 'node') +if nodes: + errors.append(f"Maven resolved no graph, so nothing may be emitted; nodes={nodes}") +jars = sorted(f for _, _, fs in os.walk(sys.argv[2]) for f in fs if f.endswith('.jar')) +if jars: + errors.append(f"Maven resolved no graph, so nothing may be fetched; jars={jars}") +if errors: + print("FAIL:") + for e in errors: + print(" -", e) + sys.exit(1) +print(f"PASS (unsolvable conflict): {module} unscannable, nothing emitted or fetched") +PY diff --git a/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh b/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh index be8dac3f40..af17904d4c 100755 --- a/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh +++ b/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh @@ -6,7 +6,9 @@ # its transitive demo.ext:harness-core (only in a non-prod root -> the assembler's dev flag); # - emits the internal reactor module demo:lib by its bare groupId:artifactId:version id (so the # inter-module edge lines up with its `project` record); -# - resolves a scope conflict so the retained node stays reachable from its parent root; +# - marks a node prod by the scope Maven kept it with, and keeps it reachable in the prod root; +# - records every resolved edge, pointing at the node Maven kept: onto a direct dependency, onto a +# mediated version, and from a prod node onto a provided one; # - every component is reachable from a direct dependency of its own root (no orphaned components); # - keeps the dependencies Maven leaves only under a sibling module's test-jar (app reaches # demo.ext:libdep under lib's test-jar node, after lib's jar node through mid); @@ -39,7 +41,14 @@ bash "$HERE/../make-stub-repo.sh" "$STUBS" \ 'demo.ext:conflict-test:1.0+demo.ext:shared:1.0' \ 'demo.ext:conflict-prod:1.0+demo.ext:shared:1.0' \ 'demo.ext:shared:1.0' \ - 'demo.ext:libdep:1.0' + 'demo.ext:libdep:1.0' \ + 'demo.ext:dup-util:1.0' \ + 'demo.ext:dup-io:1.0+demo.ext:dup-util:1.0' \ + 'demo.ext:med-util:1.0' \ + 'demo.ext:med-util:2.0' \ + 'demo.ext:med-io:1.0+demo.ext:med-util:2.0' \ + 'demo.ext:container-api:1.0' \ + 'demo.ext:container-lib:1.0+demo.ext:container-api:1.0' # Maven's own plugin closure stays cached between runs; the stubs never do, so every run has to # resolve them from the repo just generated. rm -rf "$M2/demo" "$RECORDS" "$PROJECT"/*/target "$PROJECT"/target diff --git a/src/commands/manifest/scripts/test/maven-compat/unsolvable-conflict/pom.xml b/src/commands/manifest/scripts/test/maven-compat/unsolvable-conflict/pom.xml new file mode 100644 index 0000000000..a884bbe4f0 --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/unsolvable-conflict/pom.xml @@ -0,0 +1,42 @@ + + + 4.0.0 + demo + unsolvable-conflict + 1.0 + + file://${maven.multiModuleProjectDirectory}/localrepo + + + + socket-stubs + ${stub.repo.url} + + true + ignore + + + false + + + + + + + demo.unsolvable + left + 1.0 + + + demo.unsolvable + right + 1.0 + + + demo.unsolvable + harness + 1.0 + test + + + diff --git a/src/commands/manifest/scripts/test/run-compat.sh b/src/commands/manifest/scripts/test/run-compat.sh index 1e7d9ac241..3e503016ee 100755 --- a/src/commands/manifest/scripts/test/run-compat.sh +++ b/src/commands/manifest/scripts/test/run-compat.sh @@ -117,6 +117,7 @@ run_maven() { bash "$HERE/maven-compat/smoke-test-duplicate-failure.sh" "$dir/bin/mvn" "$jar" bash "$HERE/maven-compat/smoke-test-graph-failure.sh" "$dir/bin/mvn" "$jar" bash "$HERE/maven-compat/smoke-test-descriptor-failure.sh" "$dir/bin/mvn" "$jar" + bash "$HERE/maven-compat/smoke-test-unsolvable-conflict.sh" "$dir/bin/mvn" "$jar" done } diff --git a/test/maven-compat-smoke-assertions.test.mts b/test/maven-compat-smoke-assertions.test.mts index 189ed48455..c8cca14053 100644 --- a/test/maven-compat-smoke-assertions.test.mts +++ b/test/maven-compat-smoke-assertions.test.mts @@ -5,8 +5,11 @@ * their bare groupId:artifactId:version id, distinct from the typed * groupId:artifactId:type:version form used for external artifacts. The lookup * that checks this has to reject the typed form, or a regression that stopped - * emitting the bare id would still pass. The fixture itself needs Maven and a - * JDK, so these run its assertions over synthetic records instead. + * emitting the bare id would still pass. The fixture also checks that the + * extension records every resolved edge and marks a node prod by the scope + * Maven kept it with. + * The fixture itself needs Maven and a JDK, so these run its assertions over + * synthetic records instead. * * Implementation: src/commands/manifest/scripts/test/maven-compat/assert-records.py */ @@ -26,12 +29,13 @@ const ASSERT_RECORDS_PATH = path.join( '../src/commands/manifest/scripts/test/maven-compat/assert-records.py', ) -const PROD_ROOT = 'app' -const TEST_ROOT = 'testroot' - -/** A `root` row: the assertions read the id from field 2 and prod-ness from field 5. */ -function rootRow(rootId: string, options: { prod: boolean }): string { - return ['root', rootId, '', '', options.prod ? '1' : '0'].join('\t') +/** A `root` row: the assertions read the id (field 2), project (field 3) and prod-ness (field 5). */ +function rootRow( + rootId: string, + projectKey: string, + options: { prod: boolean }, +): string { + return ['root', rootId, projectKey, '', options.prod ? '1' : '0'].join('\t') } /** A `node` row: root id in field 2, coordinate in field 3, directness in field 9. */ @@ -63,78 +67,107 @@ function edgeRow(rootId: string, parent: string, child: string): string { return ['edge', rootId, parent, child].join('\t') } -/** - * A records file the assertions accept, apart from the internal module's - * coordinate, which each test supplies. Every non-direct node carries a parent - * edge in its own root (the reachability invariant now asserted). - */ -function buildRecords(internalModuleCoord: string): string { - return [ - ['meta', 'maven'].join('\t'), - rootRow(PROD_ROOT, { prod: true }), - rootRow(TEST_ROOT, { prod: false }), - // External prod dependency, materialized as a jar. - nodeRow(PROD_ROOT, 'demo.ext:tool:jar:1.0', { direct: true }), - fileRow(PROD_ROOT, 'demo.ext:tool:jar:1.0', '/repo/tool-1.0.jar'), - // Test-scope dependency and its transitive, kept out of the prod root. - nodeRow(TEST_ROOT, 'demo.ext:harness:jar:1.0', { direct: true }), - fileRow(TEST_ROOT, 'demo.ext:harness:jar:1.0', '/repo/harness-1.0.jar'), - edgeRow( - TEST_ROOT, - 'demo.ext:harness:jar:1.0', - 'demo.ext:harness-core:jar:1.0', - ), - nodeRow(TEST_ROOT, 'demo.ext:harness-core:jar:1.0', { direct: false }), - // Scope-conflict shape: shared's effective scope is prod, but Maven kept it under a - // test parent, so it belongs in the dev root and must keep that parent edge. - nodeRow(TEST_ROOT, 'demo.ext:conflict-test:jar:1.0', { direct: true }), - edgeRow( - TEST_ROOT, - 'demo.ext:conflict-test:jar:1.0', - 'demo.ext:shared:jar:1.0', - ), - nodeRow(TEST_ROOT, 'demo.ext:shared:jar:1.0', { direct: false }), - // The internal reactor module under test. - nodeRow(PROD_ROOT, internalModuleCoord, { direct: true }), - '', - ].join('\n') +const TOOL = 'demo.ext:tool:jar:1.0' +const HARNESS = 'demo.ext:harness:jar:1.0' +const HARNESS_CORE = 'demo.ext:harness-core:jar:1.0' +const SHARED = 'demo.ext:shared:jar:1.0' +const CONFLICT_PROD = 'demo.ext:conflict-prod:jar:1.0' +const CONFLICT_TEST = 'demo.ext:conflict-test:jar:1.0' +const DUP_IO = 'demo.ext:dup-io:jar:1.0' +const DUP_UTIL = 'demo.ext:dup-util:jar:1.0' +const MED_IO = 'demo.ext:med-io:jar:1.0' +const MED_UTIL = 'demo.ext:med-util:jar:1.0' +const CONTAINER_LIB = 'demo.ext:container-lib:jar:1.0' +const CONTAINER_API = 'demo.ext:container-api:jar:1.0' + +const DEV_NODES = new Set([CONFLICT_TEST, CONTAINER_API, HARNESS, HARNESS_CORE]) + +const EDGES = { + conflictProdShared: { module: 'app', parent: CONFLICT_PROD, child: SHARED }, + conflictTestShared: { module: 'app', parent: CONFLICT_TEST, child: SHARED }, + containerLibApi: { + module: 'app', + parent: CONTAINER_LIB, + child: CONTAINER_API, + }, + dupIoUtil: { module: 'app', parent: DUP_IO, child: DUP_UTIL }, + harnessCore: { module: 'app', parent: HARNESS, child: HARNESS_CORE }, + medIoUtil: { module: 'mid', parent: MED_IO, child: MED_UTIL }, +} + +type RecordsOptions = { + dev?: string[] + internalModuleCoord?: string + omitEdges?: Array + prod?: string[] } /** - * The buggy pre-fix shape: `demo.ext:shared`'s effective scope is prod, so a - * scope-based split put it in the prod root while its only parent stayed in the - * test root. `withEdge=false` drops the prod-root parent edge, producing exactly - * the unreachable, non-direct component the validation flags. + * Records in the extension's root layout: per module, a compile root with the + * nodes Maven kept with a prod scope and a test root with the whole + * resolution. `dev`, `prod` and `omitEdges` alter it into outputs the + * assertions must reject. */ -function scopeConflictRecords(sharedRoot: string, withEdge: boolean): string { - return [ - ['meta', 'maven'].join('\t'), - rootRow(PROD_ROOT, { prod: true }), - rootRow(TEST_ROOT, { prod: false }), - nodeRow(PROD_ROOT, 'demo.ext:tool:jar:1.0', { direct: true }), - fileRow(PROD_ROOT, 'demo.ext:tool:jar:1.0', '/repo/tool-1.0.jar'), - nodeRow(TEST_ROOT, 'demo.ext:harness:jar:1.0', { direct: true }), - fileRow(TEST_ROOT, 'demo.ext:harness:jar:1.0', '/repo/harness-1.0.jar'), - edgeRow( - TEST_ROOT, - 'demo.ext:harness:jar:1.0', - 'demo.ext:harness-core:jar:1.0', - ), - nodeRow(TEST_ROOT, 'demo.ext:harness-core:jar:1.0', { direct: false }), - nodeRow(TEST_ROOT, 'demo.ext:conflict-test:jar:1.0', { direct: true }), - ...(withEdge - ? [ - edgeRow( - TEST_ROOT, - 'demo.ext:conflict-test:jar:1.0', - 'demo.ext:shared:jar:1.0', - ), - ] - : []), - nodeRow(sharedRoot, 'demo.ext:shared:jar:1.0', { direct: false }), - nodeRow(PROD_ROOT, 'demo:lib:1.0', { direct: true }), - '', - ].join('\n') +function buildRecords({ + dev = [], + internalModuleCoord = 'demo:lib:1.0', + omitEdges = [], + prod = [], +}: RecordsOptions = {}): string { + // Coordinate and directness of each module's nodes. + const modules: Array<[string, Array<[string, boolean]>]> = [ + [ + 'app', + [ + [TOOL, true], + [HARNESS, true], + [HARNESS_CORE, false], + [CONFLICT_TEST, true], + [CONFLICT_PROD, true], + [SHARED, false], + [DUP_UTIL, true], + [DUP_IO, true], + [CONTAINER_API, true], + [CONTAINER_LIB, true], + [internalModuleCoord, true], + ], + ], + [ + 'mid', + [ + [MED_UTIL, true], + [MED_IO, true], + ], + ], + ] + const isProd = (coord: string) => + prod.includes(coord) || (!DEV_NODES.has(coord) && !dev.includes(coord)) + const rows = [['meta', 'maven'].join('\t')] + for (const { 0: module, 1: nodes } of modules) { + for (const prodRoot of [true, false]) { + const rootId = `${module}-${prodRoot ? 'compile' : 'test'}` + const inRoot = (coord: string) => !prodRoot || isProd(coord) + rows.push(rootRow(rootId, `demo:${module}:1.0`, { prod: prodRoot })) + for (const { 0: coord, 1: direct } of nodes) { + if (inRoot(coord)) { + rows.push(nodeRow(rootId, coord, { direct })) + } + } + for (const { 0: name, 1: edge } of Object.entries(EDGES)) { + if ( + edge.module === module && + !omitEdges.includes(name as keyof typeof EDGES) && + inRoot(edge.parent) && + inRoot(edge.child) + ) { + rows.push(edgeRow(rootId, edge.parent, edge.child)) + } + } + } + } + rows.push(fileRow('app-compile', TOOL, '/repo/tool-1.0.jar')) + rows.push(fileRow('app-test', HARNESS, '/repo/harness-1.0.jar')) + return `${rows.join('\n')}\n` } describe('maven-compat assert-records.py', () => { @@ -148,21 +181,12 @@ describe('maven-compat assert-records.py', () => { rmSync(workDir, { force: true, recursive: true }) }) - /** Run the fixture's assertions over a records file naming `internalModuleCoord`. */ - function assertRecords(internalModuleCoord: string): { - status: number | null - output: string - } { - return assertRecordsText(buildRecords(internalModuleCoord)) - } - - /** Run the fixture's assertions over a raw records file. */ - function assertRecordsText(text: string): { + function assertRecords(options?: RecordsOptions): { status: number | null output: string } { const recordsPath = path.join(workDir, 'records.tsv') - writeFileSync(recordsPath, text) + writeFileSync(recordsPath, buildRecords(options)) const result = spawnSync('python3', [ASSERT_RECORDS_PATH, recordsPath], { cwd: workDir, @@ -171,54 +195,70 @@ describe('maven-compat assert-records.py', () => { return { output: `${result.stdout}${result.stderr}`, status: result.status } } - it('accepts an internal module emitted under its bare id', () => { - const { output, status } = assertRecords('demo:lib:1.0') - - expect(output).toContain('PASS') - expect(status).toBe(0) - }) - - it('rejects an internal module emitted only under a typed id', () => { - // 'demo:lib:' is a prefix of 'demo:lib:jar:1.0', so a lookup that matched - // on the prefix alone would report bare-id coverage it does not have. - const { output, status } = assertRecords('demo:lib:jar:1.0') - - expect(output).toContain( - 'internal module demo:lib not emitted by its bare id', - ) - expect(status).not.toBe(0) - }) - - it('rejects an internal module that is missing entirely', () => { - const { output, status } = assertRecords('demo:unrelated:1.0') - - expect(output).toContain( - 'internal module demo:lib not emitted by its bare id', - ) - expect(status).not.toBe(0) - }) - - it('accepts a scope-conflict node kept in its parent root', () => { - // Fixed extension output: shared's effective scope is prod, but the retained - // parent is test, so it must land in the dev root with the edge that reaches it. - const { output, status } = assertRecordsText( - scopeConflictRecords(TEST_ROOT, true), - ) + it('accepts the extension output', () => { + const { output, status } = assertRecords() expect(output).toContain('PASS') expect(status).toBe(0) }) - it('rejects a scope-conflict node stranded in the prod root', () => { - // Pre-fix output: shared sorted into the prod root by its effective scope. Reachability (the - // shared helper) would also flag it; here we assert the Maven-specific root classification. - const { output, status } = assertRecordsText( - scopeConflictRecords(PROD_ROOT, false), - ) + it.for([ + { + // 'demo:lib:' is a prefix of 'demo:lib:jar:1.0', so a lookup that matched + // on the prefix alone would report bare-id coverage it does not have. + name: 'an internal module emitted only under a typed id', + options: { internalModuleCoord: 'demo:lib:jar:1.0' }, + message: 'internal module demo:lib not emitted by its bare id', + }, + { + name: 'an internal module that is missing entirely', + options: { internalModuleCoord: 'demo:unrelated:1.0' }, + message: 'internal module demo:lib not emitted by its bare id', + }, + { + name: 'a compile-scoped node kept under a test parent left dev', + options: { dev: [SHARED] }, + message: 'scope-conflict demo.ext:shared not in a prod root', + }, + { + name: 'a missing edge onto a node kept under another parent', + options: { omitEdges: ['conflictProdShared'] }, + message: + 'edge demo.ext:conflict-prod -> demo.ext:shared missing from a prod root', + }, + { + name: 'a missing edge onto a direct dependency', + options: { omitEdges: ['dupIoUtil'] }, + message: 'edge demo.ext:dup-io -> demo.ext:dup-util missing', + }, + { + name: 'a missing mediated edge', + options: { omitEdges: ['medIoUtil'] }, + message: 'edge demo.ext:med-io -> demo.ext:med-util missing from mid', + }, + { + name: 'a missing edge from a prod node onto a provided one', + options: { omitEdges: ['containerLibApi'] }, + message: 'edge demo.ext:container-lib -> demo.ext:container-api missing', + }, + { + name: 'a provided dependency in a prod root', + options: { prod: [CONTAINER_API] }, + message: 'provided demo.ext:container-api wrongly in a prod root', + }, + { + name: 'a compile dependency of a provided one left dev', + options: { dev: [CONTAINER_LIB] }, + message: 'demo.ext:container-lib not in a prod root', + }, + ] satisfies Array<{ + name: string + options: RecordsOptions + message: string + }>)('rejects $name', ({ message, options }) => { + const { output, status } = assertRecords(options) - expect(output).toContain( - 'scope-conflict demo.ext:shared wrongly in a prod root', - ) + expect(output).toContain(message) expect(status).not.toBe(0) }) }) From a1a4242c5029ab2c27bc2d9b16dc1208805a94d9 Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Thu, 12 Nov 2026 11:38:14 +0100 Subject: [PATCH 2/2] fix(manifest): fail closed when Maven's graph skips the edge recorder Build the resolved edges inside the transformer, so a broken assumption about Maven's graph becomes a collection error that marks only that module unscannable. A module whose graph never passed through the recorder is now reported instead of emitting nothing silently. Walk each artifact once again before touching its node, leave self-edges to the assembler, and exclude libdep from mid's lib dependency so the fixture catches a walk keyed by the shared jar/test-jar id. --- .../socket/SocketFactsRecordsEngine.java | 108 ++++++++++-------- .../test/maven-compat/assert-records.py | 50 ++++---- .../test/maven-compat/project/app/pom.xml | 5 +- .../test/maven-compat/project/mid/pom.xml | 6 + .../scripts/test/maven-compat/smoke-test.sh | 4 +- test/maven-compat-smoke-assertions.test.mts | 38 ++++-- 6 files changed, 126 insertions(+), 85 deletions(-) diff --git a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java index 60d06ecc33..14e098e1e9 100644 --- a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java +++ b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java @@ -52,7 +52,7 @@ * Emits each reactor module's resolved dependency graph as line-protocol records for the TS assembler * (same contract as the Gradle/SBT scripts; no JSON/hashing here). Per module: a test root with the * whole resolution and a compile root with the nodes kept with a prod scope (compile/runtime/system), - * both with every resolved edge. A reactor module becomes a component only + * each with the resolved edges between its own nodes. A reactor module becomes a component only * where another depends on it, by its bare {@code groupId:artifactId:version} id. * *

Resolution goes through {@link ProjectDependenciesResolver} — the same component Maven's own @@ -215,14 +215,16 @@ private void collectModule( thrown = e; result = e.getResult(); if (result == null) { - unscannable.add(new Unscannable(rootMessage(e), moduleCoord)); - log.warn("[socket-facts] could not resolve dependencies for " + moduleCoord + ": " + rootMessage(e)); + reportUnscannable(unscannable, moduleCoord, rootMessage(e)); return; } } boolean reported = false; - // Conflict resolution threw: the result holds only the collected candidates, not a resolved graph. - DependencyNode graph = recorder.resolved() ? result.getDependencyGraph() : null; + // Null when the transformer threw (the result then holds only the collected candidates) or the + // recorder never saw the graph Maven returned. + Map> edges = recorder.edges(); + DependencyNode returned = result.getDependencyGraph(); + DependencyNode graph = edges != null && edges.containsKey(returned) ? returned : null; List collectionFailures = new ArrayList<>(); for (Exception e : result.getCollectionErrors()) { FailedRequest failed = failedRequest(e); @@ -258,11 +260,10 @@ private void collectModule( if (scopes != null) reported |= addFailures(failures, e.getKey(), e.getValue(), scopes, passingScopes, moduleCoord); } Set placed = new HashSet<>(); - Set onClasspath = null; + Set onClasspath = graph == null ? Collections.emptySet() : edges.keySet(); for (FailedRequest failed : collectionFailures) { String coord = gav(failed.artifact); if (scopesByGav.containsKey(coord)) continue; - if (onClasspath == null) onClasspath = onClasspath(graph); String scope = absentScope(session, module, failed, onClasspath); placed.add(coord); if (scope == null) { @@ -300,11 +301,14 @@ private void collectModule( // Fail closed: a throw whose result named nothing must still surface, or an unresolved dependency // would silently leave the reachability analysis blind to whatever that artifact contains. if (thrown != null && !reported) { - unscannable.add(new Unscannable(rootMessage(thrown), moduleCoord)); - log.warn("[socket-facts] could not resolve dependencies for " + moduleCoord + ": " + rootMessage(thrown)); + reportUnscannable(unscannable, moduleCoord, rootMessage(thrown)); + } + // A transformer that threw is a collection error, reported above. Otherwise Maven's graph bypassed + // the recorder, and the module's whole graph would vanish unreported. + if (graph == null && !recorder.threw()) { + reportUnscannable(unscannable, moduleCoord, "dependencies resolved without the socket-facts graph transformer"); } if (graph == null) return; - Map> edges = recorder.resolvedEdges(graph); Set visited = new HashSet<>(); for (DependencyNode child : edges.get(graph)) { String id = visit(child, edges, passingScopes, reactorGavs, unresolved, opts, nodes, visited); @@ -386,6 +390,11 @@ public boolean accept(DependencyNode node, List parents) { } } + private void reportUnscannable(Set unscannable, String moduleCoord, String reason) { + unscannable.add(new Unscannable(reason, moduleCoord)); + log.warn("[socket-facts] could not resolve dependencies for " + moduleCoord + ": " + reason); + } + private void reportUnfetched( Dependency dep, List errors, String moduleCoord, Set failures, Set unfetched) { String id = externalId(dep.getArtifact()); @@ -418,6 +427,10 @@ private String visit( String key = externalId(artifact); String id = internal ? SocketSupport.bareId(artifact.getGroupId(), artifact.getArtifactId(), version) : key; + // One walk per artifact: a shared subtree is already recorded. Keyed by coordinate, as a sibling's jar + // and test-jar share a node but can collect different children, e.g. when one declaration has exclusions. + if (!visited.add(key)) return id; + Node node = internal ? upsert(nodes, id, artifact.getGroupId(), artifact.getArtifactId(), "", "", version, gav) : upsert(nodes, id, artifact.getGroupId(), artifact.getArtifactId(), @@ -425,10 +438,6 @@ private String visit( // A sibling's jar and test-jar share a node, each kept with its own scope. if (isProd(scope)) node.prod = true; - // One walk per artifact: a shared subtree is already recorded. Keyed by coordinate, as a sibling's jar - // and test-jar share a node but Maven can keep the sibling's dependencies under either. - if (!visited.add(key)) return id; - // Maven wrote each accepted node's resolved file back onto the node; a reactor module reports its // own dirs through its `project` record instead of a `file` record. if (!internal && opts.withFiles) { @@ -438,8 +447,7 @@ private String visit( for (DependencyNode child : edges.get(dn)) { String childId = visit(child, edges, passingScopes, reactorGavs, unresolved, opts, nodes, visited); - // A cycle back onto the node's own artifact, or a sibling's jar and test-jar, which share an id. - if (childId != null && !childId.equals(id)) node.children.add(childId); + if (childId != null) node.children.add(childId); } return id; } @@ -856,19 +864,43 @@ public int hashCode() { // including one onto a dependency Maven kept elsewhere, possibly at another version. private static final class EdgeRecorder implements DependencyGraphTransformer { private final DependencyGraphTransformer delegate; - private Map> collected; - private Map conflictIds; - private boolean resolved; + private Map> edges; + private boolean threw; EdgeRecorder(DependencyGraphTransformer delegate) { this.delegate = delegate; } + /** + * The root and each node Maven kept, mapped to its children by every resolved edge, a child that + * lost its conflict replaced by the node kept for its conflict id. Null unless the last + * call to the transformer, recording included, completed. + */ + Map> edges() { + return edges; + } + + boolean threw() { + return threw; + } + + // A RepositoryException is a collection error, so a broken assumption fails this module only. @Override public DependencyNode transformGraph(DependencyNode root, DependencyGraphTransformationContext context) throws RepositoryException { // Resolver 2 may collect again when a DependencyCollectionChecker asks; the last run is the result. - resolved = false; + edges = null; + threw = true; + Map> collected = snapshot(root); + DependencyNode transformed = delegate.transformGraph(root, context); + Map conflictIds = (Map) context.get(CONTEXT_CONFLICT_IDS); + if (conflictIds == null) throw new RepositoryException("Maven's graph transformer assigned no conflict ids"); + edges = resolvedEdges(transformed, collected, conflictIds); + threw = false; + return transformed; + } + + private static Map> snapshot(DependencyNode root) { Map> children = new IdentityHashMap<>(); // The collector gives nodes of one artifact and context one shared child list. Map, List> copies = new IdentityHashMap<>(); @@ -885,53 +917,37 @@ public DependencyNode transformGraph(DependencyNode root, DependencyGraphTransfo } children.put(n, copy); } - DependencyNode transformed = delegate.transformGraph(root, context); - collected = children; - conflictIds = (Map) context.get(CONTEXT_CONFLICT_IDS); - resolved = true; - return transformed; + return children; } - /** Whether Maven's conflict resolution completed, so the resolution result holds its graph. */ - boolean resolved() { - return resolved; - } - - /** - * The root and each node Maven kept, mapped to its children by every resolved edge, a child that - * lost its conflict replaced by the node kept for its conflict id. Releases the recorded graph. - */ - Map> resolvedEdges(DependencyNode graph) { - if (conflictIds == null) throw new IllegalStateException("Maven's graph transformer assigned no conflict ids"); + private static Map> resolvedEdges( + DependencyNode graph, Map> collected, Map conflictIds) + throws RepositoryException { Set classpath = onClasspath(graph); Map kept = new HashMap<>(); for (DependencyNode n : classpath) { if (n == graph) continue; - DependencyNode other = kept.put(conflictId(n), n); - if (other != null) { - throw new IllegalStateException("Maven kept two nodes for one conflict: " + other + " and " + n); - } + DependencyNode other = kept.put(conflictId(conflictIds, n), n); + if (other != null) throw new RepositoryException("Maven kept two nodes for one conflict: " + other + " and " + n); } Map> edges = new IdentityHashMap<>(); for (DependencyNode parent : classpath) { List children = collected.get(parent); - if (children == null) throw new IllegalStateException(parent + " was kept but never collected"); + if (children == null) throw new RepositoryException(parent + " was kept but never collected"); List resolved = new ArrayList<>(); for (DependencyNode child : children) { // None kept: the resolved graph has no such dependency, so neither has the edge. - DependencyNode winner = kept.get(conflictId(child)); + DependencyNode winner = kept.get(conflictId(conflictIds, child)); if (winner != null) resolved.add(winner); } edges.put(parent, resolved); } - collected = null; - conflictIds = null; return edges; } - private Object conflictId(DependencyNode n) { + private static Object conflictId(Map conflictIds, DependencyNode n) throws RepositoryException { Object id = conflictIds.get(n); - if (id == null) throw new IllegalStateException("Maven's graph transformer assigned " + n + " no conflict id"); + if (id == null) throw new RepositoryException("Maven's graph transformer assigned " + n + " no conflict id"); return id; } } diff --git a/src/commands/manifest/scripts/test/maven-compat/assert-records.py b/src/commands/manifest/scripts/test/maven-compat/assert-records.py index 80bbbcdbe9..66202e5e72 100755 --- a/src/commands/manifest/scripts/test/maven-compat/assert-records.py +++ b/src/commands/manifest/scripts/test/maven-compat/assert-records.py @@ -70,39 +70,45 @@ def has_jar(cid): return any(p.endswith('.jar') for p in files.get(cid, ())) def edge_roots(parent, child): return edges.get((parent, child), set()) +def edge_pair(parent, child): + """Both coordinates, or None after reporting the missing ones.""" + found = (coord(parent), coord(child)) + missing = [p for p, c in zip((parent, child), found) if not c] + if missing: + errors.append(f"missing {', '.join(m.rstrip(':') for m in missing)}") + return None + return found + # Scope conflict: Maven keeps demo.ext:shared under the test parent but selects compile, the # scope conflict-prod's path gives it, so it is on the runtime classpath. Its edge from # conflict-prod, onto the copy Maven kept under the test parent, keeps it reachable in the prod root. -shared = coord('demo.ext:shared:jar:') -conflict_prod = coord('demo.ext:conflict-prod:jar:') -if not shared: errors.append("missing scope-conflict dep demo.ext:shared") -elif not in_prod(shared): errors.append("scope-conflict demo.ext:shared not in a prod root") -elif not any(roots.get(rid) for rid in edge_roots(conflict_prod, shared)): - errors.append("edge demo.ext:conflict-prod -> demo.ext:shared missing from a prod root") +pair = edge_pair('demo.ext:conflict-prod:jar:', 'demo.ext:shared:jar:') +if pair: + if not in_prod(pair[1]): errors.append("scope-conflict demo.ext:shared not in a prod root") + elif not any(roots.get(rid) for rid in edge_roots(*pair)): + errors.append("edge demo.ext:conflict-prod -> demo.ext:shared missing from a prod root") # An edge onto a dependency Maven kept elsewhere: dup-io's dup-util is the direct one. -dup_io, dup_util = coord('demo.ext:dup-io:jar:'), coord('demo.ext:dup-util:jar:') -if not dup_io or not dup_util: errors.append("missing demo.ext:dup-io or demo.ext:dup-util") -elif not edge_roots(dup_io, dup_util): errors.append("edge demo.ext:dup-io -> demo.ext:dup-util missing") - -# A mediated edge: med-io asks for another med-util than mid declares, and points at mid's. One -# med-util coordinate in all, so the version med-io asked for was never emitted. -med_io, med_util = coord('demo.ext:med-io:jar:'), coord('demo.ext:med-util:jar:') -if not med_io or not med_util: errors.append("missing demo.ext:med-io or demo.ext:med-util") -elif not any(root_projects.get(rid) == 'demo:mid:1.0' for rid in edge_roots(med_io, med_util)): +pair = edge_pair('demo.ext:dup-io:jar:', 'demo.ext:dup-util:jar:') +if pair and not edge_roots(*pair): errors.append("edge demo.ext:dup-io -> demo.ext:dup-util missing") + +# A mediated edge: med-io asks for another med-util than mid declares, and points at mid's. +# coord() fails if the med-util version med-io asked for is also emitted. +pair = edge_pair('demo.ext:med-io:jar:', 'demo.ext:med-util:jar:') +if pair and not any(root_projects.get(rid) == 'demo:mid:1.0' for rid in edge_roots(*pair)): errors.append("edge demo.ext:med-io -> demo.ext:med-util missing from mid") # An edge across the prod/dev split: container-lib (compile) onto container-api, which app # declares provided. The edge is kept and container-api stays dev. -container_lib, container_api = coord('demo.ext:container-lib:jar:'), coord('demo.ext:container-api:jar:') -if not container_lib or not container_api: errors.append("missing demo.ext:container-lib or demo.ext:container-api") -elif in_prod(container_api): errors.append("provided demo.ext:container-api wrongly in a prod root") -elif not in_prod(container_lib): errors.append("demo.ext:container-lib not in a prod root") -elif not edge_roots(container_lib, container_api): - errors.append("edge demo.ext:container-lib -> demo.ext:container-api missing") +pair = edge_pair('demo.ext:container-lib:jar:', 'demo.ext:container-api:jar:') +if pair: + if in_prod(pair[1]): errors.append("provided demo.ext:container-api wrongly in a prod root") + elif not in_prod(pair[0]): errors.append("demo.ext:container-lib not in a prod root") + elif not edge_roots(*pair): + errors.append("edge demo.ext:container-lib -> demo.ext:container-api missing") if errors: print("FAIL:") for e in errors: print(" -", e) sys.exit(1) -print(f"PASS: tool=maven; {ext} prod+jar; harness/harness-core dev; internal demo:lib (bare id, direct); {shared} prod; edges onto kept dependencies") +print(f"PASS: tool=maven; {ext} prod+jar; harness/harness-core dev; internal demo:lib (bare id, direct); demo.ext:shared prod; edges onto kept dependencies") diff --git a/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml b/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml index b414ec39a6..cd2250b4a8 100644 --- a/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml +++ b/src/commands/manifest/scripts/test/maven-compat/project/app/pom.xml @@ -9,9 +9,8 @@ app + through mid, which excludes demo.ext:libdep, so only lib's test-jar further down brings + libdep, and the walk reaches the jar first. --> demo mid diff --git a/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml b/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml index e48fd27f01..677893ccfb 100644 --- a/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml +++ b/src/commands/manifest/scripts/test/maven-compat/project/mid/pom.xml @@ -12,6 +12,12 @@ demo lib 1.0 + + + demo.ext + libdep + + diff --git a/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh b/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh index af17904d4c..0f868cca68 100755 --- a/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh +++ b/src/commands/manifest/scripts/test/maven-compat/smoke-test.sh @@ -10,8 +10,8 @@ # - records every resolved edge, pointing at the node Maven kept: onto a direct dependency, onto a # mediated version, and from a prod node onto a provided one; # - every component is reachable from a direct dependency of its own root (no orphaned components); -# - keeps the dependencies Maven leaves only under a sibling module's test-jar (app reaches -# demo.ext:libdep under lib's test-jar node, after lib's jar node through mid); +# - keeps a dependency only a sibling module's test-jar brings (mid excludes demo.ext:libdep from +# lib, and app reaches lib's jar through mid before lib's test-jar); # - materializes resolved external jars under -Dsocket.withFiles; # - scopes that materialization to -Dsocket.populateFilesFor (a newline-delimited GAV file). # diff --git a/test/maven-compat-smoke-assertions.test.mts b/test/maven-compat-smoke-assertions.test.mts index c8cca14053..62517cbaa6 100644 --- a/test/maven-compat-smoke-assertions.test.mts +++ b/test/maven-compat-smoke-assertions.test.mts @@ -82,6 +82,8 @@ const CONTAINER_API = 'demo.ext:container-api:jar:1.0' const DEV_NODES = new Set([CONFLICT_TEST, CONTAINER_API, HARNESS, HARNESS_CORE]) +type Edge = { child: string; module: string; parent: string } + const EDGES = { conflictProdShared: { module: 'app', parent: CONFLICT_PROD, child: SHARED }, conflictTestShared: { module: 'app', parent: CONFLICT_TEST, child: SHARED }, @@ -93,25 +95,31 @@ const EDGES = { dupIoUtil: { module: 'app', parent: DUP_IO, child: DUP_UTIL }, harnessCore: { module: 'app', parent: HARNESS, child: HARNESS_CORE }, medIoUtil: { module: 'mid', parent: MED_IO, child: MED_UTIL }, +} satisfies Record + +function rootIdOf(module: string, prodRoot: boolean): string { + return `${module}-${prodRoot ? 'compile' : 'test'}` } type RecordsOptions = { dev?: string[] internalModuleCoord?: string - omitEdges?: Array + omitEdges?: Edge[] + omitNodes?: string[] prod?: string[] } /** * Records in the extension's root layout: per module, a compile root with the * nodes Maven kept with a prod scope and a test root with the whole - * resolution. `dev`, `prod` and `omitEdges` alter it into outputs the + * resolution. `dev`, `prod`, `omitEdges` and `omitNodes` alter it into outputs the * assertions must reject. */ function buildRecords({ dev = [], internalModuleCoord = 'demo:lib:1.0', omitEdges = [], + omitNodes = [], prod = [], }: RecordsOptions = {}): string { // Coordinate and directness of each module's nodes. @@ -145,18 +153,19 @@ function buildRecords({ const rows = [['meta', 'maven'].join('\t')] for (const { 0: module, 1: nodes } of modules) { for (const prodRoot of [true, false]) { - const rootId = `${module}-${prodRoot ? 'compile' : 'test'}` - const inRoot = (coord: string) => !prodRoot || isProd(coord) + const rootId = rootIdOf(module, prodRoot) + const inRoot = (coord: string) => + !omitNodes.includes(coord) && (!prodRoot || isProd(coord)) rows.push(rootRow(rootId, `demo:${module}:1.0`, { prod: prodRoot })) for (const { 0: coord, 1: direct } of nodes) { if (inRoot(coord)) { rows.push(nodeRow(rootId, coord, { direct })) } } - for (const { 0: name, 1: edge } of Object.entries(EDGES)) { + for (const edge of Object.values(EDGES)) { if ( edge.module === module && - !omitEdges.includes(name as keyof typeof EDGES) && + !omitEdges.includes(edge) && inRoot(edge.parent) && inRoot(edge.child) ) { @@ -165,8 +174,8 @@ function buildRecords({ } } } - rows.push(fileRow('app-compile', TOOL, '/repo/tool-1.0.jar')) - rows.push(fileRow('app-test', HARNESS, '/repo/harness-1.0.jar')) + rows.push(fileRow(rootIdOf('app', true), TOOL, '/repo/tool-1.0.jar')) + rows.push(fileRow(rootIdOf('app', false), HARNESS, '/repo/harness-1.0.jar')) return `${rows.join('\n')}\n` } @@ -222,25 +231,30 @@ describe('maven-compat assert-records.py', () => { }, { name: 'a missing edge onto a node kept under another parent', - options: { omitEdges: ['conflictProdShared'] }, + options: { omitEdges: [EDGES.conflictProdShared] }, message: 'edge demo.ext:conflict-prod -> demo.ext:shared missing from a prod root', }, { name: 'a missing edge onto a direct dependency', - options: { omitEdges: ['dupIoUtil'] }, + options: { omitEdges: [EDGES.dupIoUtil] }, message: 'edge demo.ext:dup-io -> demo.ext:dup-util missing', }, { name: 'a missing mediated edge', - options: { omitEdges: ['medIoUtil'] }, + options: { omitEdges: [EDGES.medIoUtil] }, message: 'edge demo.ext:med-io -> demo.ext:med-util missing from mid', }, { name: 'a missing edge from a prod node onto a provided one', - options: { omitEdges: ['containerLibApi'] }, + options: { omitEdges: [EDGES.containerLibApi] }, message: 'edge demo.ext:container-lib -> demo.ext:container-api missing', }, + { + name: 'a missing end of an edge', + options: { omitNodes: [CONFLICT_PROD] }, + message: 'missing demo.ext:conflict-prod:jar', + }, { name: 'a provided dependency in a prod root', options: { prod: [CONTAINER_API] },