Fix IT failures on Maven 4 (transporter: NoTransporterException for file:// protocol)#309
Fix IT failures on Maven 4 (transporter: NoTransporterException for file:// protocol)#309elharo wants to merge 4 commits into
Conversation
…gin version Maven 4's default resolver does not include a file:// protocol transporter, causing all integration tests to fail with NoTransporterException when resolving artifacts from the local repository. The fix adds -Dmaven.resolver.transport=wagon to the invoker plugin configuration, forcing Maven to use the Wagon-based transport which natively supports file:// URLs. Additionally updates the hardcoded maven-deploy-plugin version in the reproducible IT from 2.8.2 to 3.1.4, which is compatible with Maven 4. Fixes apache#261
|
trigger ci |
|
trigger ci |
elharo
added
build
There was a problem hiding this comment.
Pull request overview
This PR aims to restore the project’s Maven Invoker-based integration tests on Maven 4 by ensuring the IT Maven executions can resolve artifacts from a file://-backed repository and by updating an IT to use a Maven 4–compatible deploy plugin.
Changes:
- Force IT Maven executions (via
maven-invoker-plugin) to use the Wagon transporter sofile://repositories work on Maven 4. - Update the
reproducibleIT to usemaven-deploy-plugin3.1.4 (instead of 2.8.2) for Maven 4 compatibility. - Additional changes included in this PR (site skin config + IT goal list tweak) that are not currently reflected in the PR description.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pom.xml |
Adds Invoker mavenOpts to force Wagon transport during IT runs (addresses Maven 4 file:// transporter failure). |
src/it/reproducible/pom.xml |
Bumps maven-deploy-plugin to 3.1.4 for Maven 4 compatibility in the reproducible IT. |
src/it/reproducible/invoker.properties |
Adjusts IT invoked goals by adding jar:jar (rationale should be captured in PR description). |
src/site/site.xml |
Adds an explicit Maven Site skin/version (appears unrelated to the stated IT/Maven 4 issue). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <skin> | ||
| <groupId>org.apache.maven.skins</groupId> | ||
| <artifactId>maven-fluido-skin</artifactId> | ||
| <version>2.1.0</version> | ||
| </skin> | ||
| <body> |
| # under the License. | ||
|
|
||
| invoker.goals=remote-resources:process source:jar deploy:deploy | ||
| invoker.goals=remote-resources:process jar:jar source:jar deploy:deploy |
|
@ascheman I'M in a hurry due vacation preparations. Please have a look. I have in mind that we had this local repository issue in other plugins too. I can also imagine that lifting the plugin to RC5/ (6 aka SNAPSHOT) first, may solve some issue. According upgrading: Sandra has started (but paused) working on it: #268 |
ascheman
left a comment
ascheman
left a comment
There was a problem hiding this comment.
Thanks for tackling the reproducible IT, @elharo — adding jar:jar to the goals and bumping the deploy-plugin are sensible IT-plumbing fixes.
I wanted to flag one concern before this lands, based on testing against current Maven 4. I ran this PR's branch in a clean container (fresh ~/.m2, JDK 21) against both 4.0.0-rc-5 and the maven-4.0.x SNAPSHOT (rc-6 candidate). In both cases the reproducible IT fails before the transport/deploy changes are even reached:
[ERROR] Failed to execute goal ...maven-source-plugin:4.0.0-beta-2-SNAPSHOT:jar (default-cli)
on project reproducible:
java.lang.NoSuchMethodError:
org.apache.maven.api.services.ProjectManager.getCompileSourceRoots(Project, ProjectScope)
ProjectManager.getCompileSourceRoots(Project, ProjectScope) was removed before rc-4 (replaced by getSourceRoots(...) / getEnabledSourceRoots(Project, ProjectScope, Language)), while this plugin still builds against 4.0.0-beta-3 and calls the old method in SourceJarNoForkMojo / TestSourceJarNoForkMojo. So the IT here can only pass on beta-3-and-older Maven — which is presumably why CI is green — and fails on every released RC.
That's my hesitation about merging as-is: a fix whose IT passes only on a pre-rc Maven, and fails on rc-4/rc-5/rc-6, isn't really validating Maven 4 compatibility.
The API migration is exactly what #268 (@sparsick) does — it moves those mojos to getEnabledSourceRoots(...). Could we sequence this so the IT plumbing is validated on top of the migrated code against a current/upcoming RC? Concretely, either:
- fold the IT-plumbing parts of this PR (the
jar:jargoal and the deploy-plugin2.8.2 -> 3.1.4bump) into #268, so they're verified against the final API, or - rebase this PR once the plugin builds against a current RC and re-confirm the IT is green there.
One thing worth re-checking during that rework: whether -Dmaven.resolver.transport=wagon is still needed once deploy-plugin is on 3.1.4 — in a separate test a plain file:// deploy worked with the default transport on current 4.0.x, so the wagon override may be droppable.
Happy to help verify on the RC once the plugin is migrated. (Non-binding review.)
Updating to RC5 will end in failing tests (see also my PR #268). Therefore, I paused the work until RC6 is released. After RC6 is released, I will continue |
Yes, we should wait for RC-6 and rework 309 with the latest API changes accordingly (this can already be prepared with RC-5/4.0.x-SNAPSHOT). |
elharo
left a comment
elharo
left a comment
There was a problem hiding this comment.
I don't think we should wait for RC6. Right now the repo is borked and all work is blocked. This is the minimal change we need to get the CI passing. Details can be revisited after RC6 if that's needed.
And what does the green CI help in the end? No-one with a recent M4 installation will be able to use it as it is using an API from a beta-version which was changed meanwhile. If someone is really, really on any older version than RC-4, we should heavily recommend to switch to RC-4 or RC-5 (which has small issues). Meanwhile the fix could be aligned with the new API as described. |
|
A green CI allows new and existing PRs to move forward. The current state blocks all work. This is not about what's deployed in production (which shouldn't be RC-anything) but rather about whether any work can proceed on this plugin. |
| <goal>install</goal> | ||
| </goals> | ||
| <addTestClassPath>true</addTestClassPath> | ||
| <mavenOpts>-Dmaven.resolver.transport=wagon</mavenOpts> |
There was a problem hiding this comment.
I would be careful with such change .... it turn on old transport which is not used by default
In real use users use default transport so problem can be only hidden here and we test something different which is normally used
There was a problem hiding this comment.
This change is necessary to make the existing tests pass because they use file:// URLs the default transport doesn't support. Perhaps file an issue to fix those tests. However, in the meantime I think it is absolutely essential to get the tests passing. No progress on anything can be made until we do that, and this is the minimal and safest change I could contrive that accomplishes that. Once the CI is green, we can carefully move forward with other fixes without fear of accidentally breaking more things.
The alternative would be to roll everything back to the last known good commit.
There was a problem hiding this comment.
Well, I suggest to focus on the root cause (-> #310) to make CI green again and leave the transport fix (and clarification of #309 (comment)) for the time beyond merging #268 (which could be prepared nonetheless).
|
#310 helps but it doesn't seem to be sufficient. I'm still seeing CI failures on some JDKs (not all) that don't have anything do with fluido or skins or site: |
Seems to be green now. Do you have a concrete build job reference where it still broke (perhaps a flake)? |
No. They still have the |
|
seems to be working now. I think I pulled master a little too soon and missed #312 |
Fixes #261
Problem
All 24 integration tests fail on Maven 4 due to
NoTransporterException: Unsupported transport protocol file. Maven 4's default resolver does not include afile://protocol transporter, so the@localRepositoryUrl@(resolved to afile://URL) insrc/it/settings.xmlcauses all ITs to fail at dependency resolution.The
reproducibleIT also had an additional failure: itsassert deployDir.exists()assertion fails becausedeploy:deployused a hardcodedmaven-deploy-plugin:2.8.2which is not compatible with Maven 4.Fix
pom.xml: Added<mavenOpts>-Dmaven.resolver.transport=wagon</mavenOpts>to themaven-invoker-pluginconfiguration in therun-itsprofile. This forces Maven to use the Wagon-based transport, which natively supportsfile://URLs.src/it/reproducible/pom.xml: Updatedmaven-deploy-pluginversion from2.8.2to3.1.4, which is compatible with Maven 4.