Skip to content

Preserve dependency scope in plugin artifacts - fix #12497#12503

Open
slawekjaranowski wants to merge 1 commit into
apache:maven-3.10.xfrom
slawekjaranowski:fix-12497
Open

Preserve dependency scope in plugin artifacts - fix #12497#12503
slawekjaranowski wants to merge 1 commit into
apache:maven-3.10.xfrom
slawekjaranowski:fix-12497

Conversation

@slawekjaranowski

Copy link
Copy Markdown
Member

What changed:

  • Added RepositoryUtils.toArtifact(ArtifactResult) to convert resolved artifacts and copy dependency metadata from the resolution request.
  • Set Maven Artifact scope and optional flag from the resolved dependency node instead of converting only the raw Aether artifact.
  • Updated DefaultMavenPluginManager to use the new conversion path for plugin realm and extension artifact resolution.

Why:

  • PluginDescriptor.getArtifacts() returned artifacts with null scope in Maven 3.10.0-rc-1.
  • Restoring scope/optional metadata brings behavior back in line with 3.9.x and avoids regressions in plugins that depend on artifact scope.

fix #12497

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Your pull request should address just one issue, without pulling in other changes.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.
    Note that commits might be squashed by a maintainer on merge.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
    This may not always be possible but is a best-practice.
  • Run mvn verify to make sure basic checks pass.
    A more thorough check will be performed on your pull request automatically.
  • You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@slawekjaranowski
slawekjaranowski requested a review from cstamas July 18, 2026 12:58
@slawekjaranowski slawekjaranowski added this to the 3.10.0 milestone Jul 18, 2026
@slawekjaranowski slawekjaranowski added the bug Something isn't working label Jul 18, 2026
What changed:
- Added `RepositoryUtils.toArtifact(ArtifactResult)` to convert resolved artifacts and copy dependency metadata from the resolution request.
- Set Maven `Artifact` scope and optional flag from the resolved dependency node instead of converting only the raw Aether artifact.
- Updated `DefaultMavenPluginManager` to use the new conversion path for plugin realm and extension artifact resolution.

Why:
- `PluginDescriptor.getArtifacts()` returned artifacts with `null` scope in Maven 3.10.0-rc-1.
- Restoring scope/optional metadata brings behavior back in line with 3.9.x and avoids regressions in plugins that depend on artifact scope.

fix apache#12497

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review — PR #12503

Verdict: ✅ APPROVE

Summary: This PR correctly fixes a regression in 3.10.0-rc-1 where PluginDescriptor.getArtifacts() returned artifacts with null scope. The new RepositoryUtils.toArtifact(ArtifactResult) overload navigates the ArtifactResult → ArtifactRequest → DependencyNode → Dependency chain to recover scope and optional metadata. The approach is minimal, null-safe via Optional, and follows existing codebase patterns.

Findings

Important — CI Spotless failure
RepositoryUtils.java line 177: Spotless rejects the space before the closing )} ); should be });. All three build jobs fail at spotless:check. Running mvn spotless:apply -B in maven-core will fix it.

// Current (fails Spotless):
                } );
// Fix:
                });

Suggestion — Javadoc
The new method only has @since 3.10.0. A brief description would help future readers:

/**
 * Converts a resolved {@link ArtifactResult} to a Maven Artifact,
 * preserving dependency scope and optional flag from the resolution
 * request's dependency node.
 *
 * @since 3.10.0
 */

Suggestion — Test coverage
No test for the new toArtifact(ArtifactResult) overload. A test verifying scope/optional preservation through the ArtifactResult → DependencyNode → Dependency chain would help prevent this regression from resurfacing.

What I Appreciate

  • Follows existing patterns precisely — the private toArtifact(Dependency) already does setScope()/setOptional(), and this recovers the same info from the ArtifactResult path.
  • Null-safety via Optional.ofNullable(...).map(...).map(...).ifPresent(...) — if any link is absent, the artifact retains its prior state gracefully.
  • The method reference change in DefaultMavenPluginManager is clean and unambiguous.
  • Backward compatible — the root plugin artifact with null scope gets a no-op setScope(null).

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet

gnodet added a commit to gnodet/maven that referenced this pull request Jul 18, 2026
Review posted for apache#12503 (preserve dependency scope in plugin
artifacts). APPROVED with Spotless formatting note.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PluginDescriptor.getArtifacts() returns Artifact instances with null scope

3 participants