Sitelet https://github.com/apache/maven/pull/12997
Skip to content

Fix #12991: skip shade-plugin upgrade when custom ResourceTransformers are present - #12997

Merged
gnodet merged 3 commits into
masterfrom
fix-apache-maven-12991-mvnup-shade-plugin-upgrad
Sep 2, 2026
Merged

Fix #12991: skip shade-plugin upgrade when custom ResourceTransformers are present#12997
gnodet merged 3 commits into
masterfrom
fix-apache-maven-12991-mvnup-shade-plugin-upgrad

Conversation

@gnodet

@gnodet gnodet commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Detects custom (non-standard) ResourceTransformer implementations in maven-shade-plugin configuration before upgrading the plugin version
  • Skips the upgrade and emits a warning when custom transformers are found, preventing silent build breakage
  • Covers both direct POM declarations and effective model analysis (inherited plugins)

Problem

mvnup upgrades maven-shade-plugin from old versions (e.g. 1.3.3) to 3.5.0 for Maven 4 compatibility. However, projects using custom ResourceTransformer implementations (e.g. BeansXmlTransformer in myfaces-extcdi) may depend on transitive dependencies like org.jdom:jdom that were available in old shade-plugin versions but removed in newer ones. The upgrade silently breaks these projects.

Solution

Added detection logic (following the existing Quarkus plugin skip pattern) that inspects <configuration>/<transformers>/<transformer implementation="..."> elements in both top-level and per-execution configurations. Transformers whose implementation class does not start with org.apache.maven.plugins.shade.resource. are considered custom. When found, the upgrade is skipped with a warning message advising manual upgrade.

Test plan

  • 11 new tests in PluginUpgradeShadeTest covering all scenarios
  • Custom transformer in execution config → skip upgrade
  • Custom transformer in top-level config → skip upgrade
  • Standard transformers only → upgrade proceeds
  • No transformers → upgrade proceeds
  • Mixed standard + custom → skip upgrade
  • Property-based version with custom transformers → skip upgrade
  • Without explicit groupId + custom transformers → skip upgrade
  • Custom transformers in pluginManagement → skip upgrade
  • Unit tests for findCustomTransformerClasses method
  • All 42 existing PluginUpgradeStrategyTest tests pass
  • All 10 existing PluginUpgradeQuarkusTest tests pass

🤖 Generated with Claude Code

@gnodet
gnodet marked this pull request as ready for review September 1, 2026 11:07
@gnodet gnodet added the bug Something isn't working label Sep 1, 2026
@gnodet gnodet added this to the 4.0.0-rc-7 milestone Sep 1, 2026
gnodet
gnodet commented Sep 1, 2026
View reviewed changes

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well-implemented bug fix that follows the established Quarkus plugin skip pattern. Custom ResourceTransformer detection is correct and conservatively prevents build breakage.

Two areas could be improved:

  1. Missing log on effective-model path (medium): When hasCustomTransformersInAnyPom() returns true and shade-plugin is silently removed from pluginUpgrades in analyzePluginsUsingEffectiveModels(), no log message is emitted. The direct-upgrade path correctly warns via context.warning("Skipping maven-shade-plugin upgrade..."), but the effective-model path (for shade-plugin inherited from a remote parent) is silent. Adding a context.warning() after pluginUpgrades.remove(shadePluginKey) would close this feedback gap.

  2. Project-wide exclusion in multi-module builds (medium): hasCustomTransformersInAnyPom() checks ALL POMs in the project. In a multi-module build where only one module uses custom transformers, shade-plugin upgrade is skipped for every module via the effective-model path. This is safe but overly conservative — a per-module check would allow upgrading shade-plugin in modules that only use standard transformers.

Strengths:

  • Thorough test coverage (11 tests covering all combinations)
  • Clean single-commit structure with proper issue reference
  • Mirrors the established Quarkus plugin skip pattern correctly

🔀 Backport Status

⚠️ This bug fix targets master but no backport to maven-4.0.x was found. The maven-4.0.x branch has the shade-plugin upgrade entry but lacks the custom transformer detection. Branches maven-3.9.x and maven-3.10.x do not contain mvnup.

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

Comment thread ...aven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
}

for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Path originalPomPath = entry.getKey();

@gnodet gnodet Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When shade-plugin is removed here because custom transformers were detected, no log message is emitted. The direct-upgrade path in upgradePluginVersion() correctly warns the user, but this effective-model path is silent. Consider adding:

context.warning("Skipping maven-shade-plugin in effective-model analysis: "
        + "custom ResourceTransformer(s) found in project POMs");

gnodet and others added 3 commits September 2, 2026 12:35
@gnodet @claude
Fix #12991: skip shade-plugin upgrade when custom ResourceTransformer…
f2ce89d 
…s are present

mvnup now detects custom (non-standard) ResourceTransformer implementation
classes in maven-shade-plugin configuration and skips the version upgrade
when they are found. Custom transformers may depend on transitive
dependencies (e.g. org.jdom:jdom) that are no longer present in newer
shade-plugin versions, silently breaking the build.

The check inspects both top-level <configuration> and per-execution
<configuration> blocks for <transformer> elements whose implementation
attribute does not start with org.apache.maven.plugins.shade.resource.
When custom transformers are detected, a warning is emitted advising
manual upgrade after verifying compatibility.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet @claude
Address review: per-module shade check and warning on effective-model…
c86635f 
… path

- Add context.warning() when shade-plugin is skipped in effective-model
  analysis, closing the feedback gap with the direct-upgrade path
- Replace project-wide hasCustomTransformersInAnyPom() with per-module
  hasCustomTransformersInPomOrParents() that walks the local parent
  chain, so unrelated modules in a multi-module build can still have
  their shade-plugin upgraded

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet @claude
fix: pass context to buildEffectiveModel after upstream signature change
282d69f 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet
gnodet force-pushed the fix-apache-maven-12991-mvnup-shade-plugin-upgrad branch from 2ba3672 to 282d69f Compare September 2, 2026 10:38
@gnodet gnodet self-assigned this Sep 2, 2026
@gnodet gnodet modified the milestones: 4.0.0-rc-7, 4.1.0 Sep 2, 2026
@gnodet
gnodet merged commit 7584841 into master Sep 2, 2026
23 checks passed
@gnodet
gnodet deleted the fix-apache-maven-12991-mvnup-shade-plugin-upgrad branch September 2, 2026 11:18
@gnodet gnodet mentioned this pull request Sep 2, 2026
Merged
1 of 2 tasks
gnodet added a commit that referenced this pull request Sep 2, 2026
@gnodet @claude
Fix #12991: skip shade-plugin upgrade when custom ResourceTransformer…
335ad9a 
…s are present

Backport of #12997 to maven-4.0.x.

mvnup detects custom ResourceTransformer classes in shade-plugin configuration
and skips the upgrade when found. Per-module check in multi-module builds —
unaffected modules can still be upgraded. Warning logged with the list of
custom transformers found.

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

Reviewers

@rmannibucau rmannibucau rmannibucau approved these changes

Assignees

@gnodet gnodet

Labels

bug Something isn't working

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@gnodet @rmannibucau