Skip to content

Build against Maven 4.0.0-rc-6 - #315

Closed
slachiewicz wants to merge 3 commits into
apache:masterfrom
slachiewicz:maven-4.0.0-rc-6
Closed

Build against Maven 4.0.0-rc-6#315
slachiewicz wants to merge 3 commits into
apache:masterfrom
slachiewicz:maven-4.0.0-rc-6

Conversation

@slachiewicz

@slachiewicz slachiewicz commented Aug 4, 2026

Copy link
Copy Markdown
Member

This plugin was still on 4.0.0-beta-3, the furthest behind of the 4-native plugins, and it is a migration rather than a version bump.

Dependencies. maven-api-meta and maven-api-impl no longer exist; they become maven-api-annotations and org.apache.maven:maven-testing respectively, matching what maven-jar-plugin already does. maven-archiver moved to org.apache.maven.shared.

Source roots. ProjectManager.getCompileSourceRoots(project, scope) and getResources(project, scope) are gone; resources and source roots are unified into SourceRoot:

before after
getCompileSourceRoots(p, MAIN) getEnabledSourceRoots(p, MAIN, Language.JAVA_FAMILY).map(SourceRoot::directory)
getResources(p, MAIN) getEnabledSourceRoots(p, MAIN, Language.RESOURCES)
Resource.getDirectory() SourceRoot.directory()
Resource.getIncludes() / getExcludes() SourceRoot.includes() / excludes()
Resource.getTargetPath() SourceRoot.targetPath()
session.createArtifact(...) session.createProducedArtifact(...)

attachArtifact now takes a ProducedArtifact. The maven-shared-archive-resources special case moves onto the same SourceRoot stream. Include/exclude and target-path handling are preserved, so which files land in the sources jar should not change.

site.xml. The <skin> workaround is removed — its own comment said it was only needed on beta-3 and is redundant from rc-5 onwards.

Verification. mvn test: 10 tests, 0 failures. mvn verify -Prun-its: all 23 ITs pass.

Getting there needed a second commit. MSOURCES-140 was failing, and its assertion turned out to be unrunnable: verify.groovy:22 is a Groovy slashy-string regex containing the literal text " + File.separator + " — Java concatenation written inside a regex literal, which a slashy string does not evaluate. It cannot match on any platform. It went unnoticed because invoker.properties requires 4.0.0-beta-4+, so the IT was skipped on the beta-3 this plugin used to build against. The mojo does emit exactly what it looks for:

[INFO] Artifact ...:jar-no-fork:jar:sources:1.0-SNAPSHOT already attached to
target/jar-no-fork-1.0-SNAPSHOT-sources.jar: ignoring same re-attach (same artifact, same file)

Matching the separator as a character class makes it pass. Kept as its own commit so it can be dropped or split out if you would rather take it separately.

For reference, unmodified master cannot run its ITs under rc-6 at all — every mojo invocation dies with NoSuchMethodError: ProjectManager.getCompileSourceRoots, so 22 ITs go from failing to passing here.

Two judgement calls worth a reviewer's eye:

  • createArchiver() looked up maven-shared-archive-resources through project.getBuild().getResources(), which is deprecated at rc-6. It now goes through ProjectManager, which is a superset — it also sees roots registered at runtime by maven-remote-resources-plugin, the very plugin that produces those resources. No test covers this path either way; it is a two-line revert if you prefer a literal translation.
  • getEnabledSourceRoots requires a Language; JAVA_FAMILY is used for sources, matching maven-compiler-plugin. It also filters on enabled(), which the old call did not. Both look like the intent of the new API, but they are real semantic differences.

SourceRoot.stringFiltering() is deliberately not wired up: the beta-3 code never read Resource.getFiltering(), and honouring it now would change file contents in the jar.

Part of apache/maven#12676.

The plugin still compiled against the 4.0.0-beta-3 API jars, but every
mojo blew up at runtime on rc-6 with

  NoSuchMethodError: ProjectManager.getCompileSourceRoots(Project, ProjectScope)

Dependencies:
  org.apache.maven:maven-api-meta                        -> org.apache.maven:maven-api-annotations
  org.apache.maven:maven-archiver                        -> org.apache.maven.shared:maven-archiver (4.0.0-beta-5)
  o.a.m.plugin-testing:maven-plugin-testing-harness      -> org.apache.maven:maven-testing
  org.apache.maven:maven-api-impl (test)                 -> covered by maven-testing
  org.apache.maven:maven-core (test)                     -> covered by maven-testing
  com.google.inject:guice (test)                         -> dropped, comes in via maven-testing
  org.apache.maven:maven-impl (test)                     -> added, tests use DefaultSourceRoot

API mapping, old -> new:
  ProjectManager.getCompileSourceRoots(p, scope)
      -> getEnabledSourceRoots(p, scope, Language.JAVA_FAMILY).map(SourceRoot::directory)
  ProjectManager.getResources(p, scope)
      -> getEnabledSourceRoots(p, scope, Language.RESOURCES)
  org.apache.maven.api.model.Resource (as the getResources() abstraction type)
      -> org.apache.maven.api.SourceRoot
         getDirectory()  -> directory()      (already a Path, no Paths.get needed)
         getIncludes()   -> includes()
         getExcludes()   -> excludes()
         getTargetPath() -> targetPath()     (Optional<Path>)
  Project.getBuild().getResources(), used to locate maven-shared-archive-resources
      -> getEnabledSourceRoots(project, MAIN, Language.RESOURCES).map(SourceRoot::directory)
         The model getter is deprecated on rc-6 and would miss roots registered at
         runtime by maven-remote-resources-plugin; the ProjectManager view is a
         superset of it.
  ProjectManager.attachArtifact(Project, Artifact, Path) now takes a ProducedArtifact
      -> Session.createArtifact(...) -> Session.createProducedArtifact(...)
  org.apache.maven.archiver.*       -> org.apache.maven.shared.archiver.*
  org.apache.maven.internal.impl.InternalSession  -> org.apache.maven.impl.InternalSession
  org.apache.maven.api.plugin.testing.*           -> org.apache.maven.testing.plugin.*
         The old annotations still exist as shims but MojoExtension no longer honours
         them, so @Basedir/@MojoParameter were silently ignored.

Which files end up in the sources jar is unchanged: the resource includes,
excludes and targetPath are read off SourceRoot instead of the model Resource,
and excludeResources still short-circuits to an empty list.

The mock ProjectManager in the mojo tests now answers getEnabledSourceRoots and
falls back to src/main/java resp. src/test/java, because rc-6 no longer puts a
default <sourceDirectory> in the model.

src/site/site.xml: dropped the <skin> workaround, redundant from rc-5 on as its
own comment says.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The regex is a Groovy slashy literal containing a stray Java concatenation:

  /... already attached to target" + File.separator + "jar-no-fork-...jar .../

Slashy strings do not evaluate that, so `" + File.separator + "` stays in
the pattern verbatim and it cannot match on any platform. It went unnoticed
because invoker.properties requires 4.0.0-beta-4+, so the IT was skipped on
the beta-3 this plugin used to build against.

The mojo does emit exactly what the IT is looking for:

  [INFO] Artifact ...:jar-no-fork:jar:sources:1.0-SNAPSHOT already attached
  to target/jar-no-fork-1.0-SNAPSHOT-sources.jar: ignoring same re-attach
  (same artifact, same file)

With the separator matched as a character class the IT passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@desruisseaux desruisseaux left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not yet familiar with the internal of the source plugin, but for the parts that I recognize it looks fine to me.

@slachiewicz
slachiewicz requested a review from sparsick August 4, 2026 12:39
@sparsick

sparsick commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@slachiewicz for this case a PR is already opened (see #268 )

The pom moved to 4.0.0-rc-6 but the workflow still asked for 4.0.0-beta-3,
so CI built the migrated code against the version it was migrated away
from and failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@slachiewicz

Copy link
Copy Markdown
Member Author

Closing in favour of #268, which I had not spotted — it is the same upgrade, opened in January and refreshed today, and it should have precedence.

For the record, the two overlap almost entirely: mavenVersion to rc-6, maven-api-metamaven-api-annotations, maven-archiver moving to org.apache.maven.shared, the ProjectManager/SourceRoot migration, the workflow pin, and the MSOURCES-140 assertion. #268 also bumps the parent POM 47 → 49 and the plugin-plugin / plugin-testing versions, which mine did not.

Two small things here are not in #268, if they are wanted:

  1. src/site/site.xml still carries a <skin> workaround whose own comment says it is only needed on beta-3 and is redundant from rc-5 onwards. Removing it is a clean follow-up now that the build is on rc-6.
  2. AbstractSourcePluginTestCase still imports org.apache.maven.api.plugin.testing.MojoExtension.getBasedir. That compiles, because the deprecated shims are still there, but MojoExtension in rc-6 only honours the relocated annotations — see maven-testing: deprecated org.apache.maven.api.plugin.testing annotations compile but are silently ignored maven#12678, where the same thing silently broke maven-jar-plugin. Worth moving to org.apache.maven.testing.plugin while touching these files.

Happy to send either as a separate PR against master once #268 lands.

@slachiewicz slachiewicz closed this Aug 4, 2026
@ascheman ascheman mentioned this pull request Aug 4, 2026
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants