[MSHADE-323] make shade plugin compatible with the flatten plugin by gzm55 · Pull Request #129 · apache/maven-shade-plugin

gzm55 · 2022-04-06T00:46:36Z

The runtime injected pom will be set via MavenProject::setFile orMavenProject::setPomFile, including this plugin and the flatten plugin. So to chain these kind of plugins, when generating the d-r-p pom, this pr first tries to parse a model directly from MavenProject::getFile and fallback to a clone of MavenProject::getOriginalModel if no pom file set.

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

Make sure there is a JIRA issue filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
Each commit in the pull request should have a meaningful subject line and body.
Format the pull request title like [MSHADE-XXX] - Fixes bug in ApproximateQuantiles,
where you replace MSHADE-XXX with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Run mvn clean verify to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
You have run the integration tests successfully (mvn -Prun-its clean verify).

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.

I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.

The runtime injected pom will be set via `MavenProject::setFile` or `MavenProject::setPomFile`, including this plugin and the flatten plugin. So to chain these kind of plugins, when generating the d-r-p pom, this pr first try to parse a model directly from `MavenProject::getFile` and fallback to a clone of `MavenProject::getOriginalModel` if not set the pom file.

rmannibucau · 2022-04-06T06:48:36Z

src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java

        }

-        Model model = project.getOriginalModel();
+        final Model model = project.getFile() == null


looks like an issue any plugin consuming the pom will get so it should likely be fixed in flatten plugin using setOriginalModel to be a global fix and not a local one.

hi @rmannibucau , the in Maven, what is originalModel defined to be, a model of the original pom or a model of the current pom file? If defined as the latter one, the global solution of updating originalModel could be done in MavenProject::setPomFile or MavenProject::setFile.

My guess is that it was the really original file but since maven is a chain (per module) it is in practise the current pom file - otherwise it can't really be used by any plugins and some do.
So guess the choice is:

keep original untouched but means no plugin should use it except help/debug ones (so fix is to always read getFile())

ensure flatten plugin updates original model to let downstream plugins consumes the post processing model

Personally I think the option 2 is what makes the most sense for the ecosystem (inter-plugin operability).

ok, this pr is the first choice, which prefer a practice single source to get current raw model.

@gzm55 this pr does not impl 1 since it should never use original model in case 1 but always file to enable to chain plugins.

it is ok to not fall back to original model when getFile() is null, throwing an exception instead.

Hmm, I just checked flatten code and it only works if updatePomFile is set to true no? So long story short, if you want it to work reliably you must ensure originalModel is set before shade plugin is executed. From a design point of view it is the responsability of flatten plugin to do that since it is the one hacking the user pom but an intermediary plugin setting it from a file when flatten plugin dumped its value would work too but I don't think there is any bug in shade there.

it seems better to have a convention or rule that the original model should be equal to the pom file. the shade plugin also breaking this convention, it reverts the parent.relativePath after write the model to the file.

shade plugins reset it cause it just needs to change it while dumping the pom (that said good point it breaks thread safety in concurrent builds).
agree shade plugin breaks this rules and happy to get it fixed, it is just not that important cause shade plugin is generally in the end of the chain (so dowstream consumers list is empty) and it also handles itself the pom rewriting but you are right.
long story short: I don't think we should re-read from the filesystem any pom model anytime in maven build otherwise the design itself is broken so I'm in favor of trying to not use the reader more than once per file.

hope it makes sense.

src/it/projects/MSHADE-323-compatible-with-flatten-pom/pom.xml

src/it/projects/MSHADE-323-compatible-with-flatten-pom/verify.groovy

src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java

Based on the discussion in maven-shade-plugin#129 apache/maven-shade-plugin#129 (comment) It seems that the flatten plugin should update the `originalModel` property after we flatten the pom.

jira-importer · 2025-06-14T16:52:58Z

Resolve #323

jira-importer · 2025-06-14T17:54:07Z

Resolve #323

altaiezior · 2026-01-21T22:45:54Z

@olamy @slawekjaranowski can we check if this can be deployed in the next version?

altaiezior · 2026-03-08T10:43:18Z

@cstamas would be help get this fix in the next release? This is an older but simpler fix.

So my use-case here for this fix is to move the dependency-reduced-pom.xml to the target folder, the reason for this that I don't want my search results to include the values from this file.

Target folder is excluded by default in IntelliJ and hence this helps

cstamas · 2026-03-08T12:49:08Z

TBH, I don't quite the problem here. PR is about "chaining flatten plugin and this plugin", and you mention IDE and search... Can someone enlighten me more?

rmannibucau · 2026-03-08T14:24:42Z

@altaiezior still needs a fix cause this PR is broken cause it rereads the pom from the filesystem which doesn't respect maven model chain at all so it can't be merged like that

altaiezior · 2026-03-08T15:15:23Z

@rmannibucau I have not previously worked on the issue and I am not sure if the original contributor is around. Would it be easy to get this fixed with the help of Gemini / AI? or does it require changes in the flatten maven plugin?

rmannibucau · 2026-03-09T06:50:19Z

@altaiezior not sure about AI but the model can be injected and we have a writer in the codebase so shouldn't be that hard.
My 2 cts would be that since maven 4 has flattening feature built-in it can be worth testing it and working with maven 4 more than maven 3 base now - but API changed, this is why I think we can just play the status quo to only update maven 4 plugin code base once setup.

altaiezior · 2026-03-09T07:28:56Z

TBH, I don't quite the problem here. PR is about "chaining flatten plugin and this plugin", and you mention IDE and search... Can someone enlighten me more?

Yes you are write, actually my comment was meant for #128, I did mistakenly added here.

altaiezior · 2026-03-09T09:45:31Z

Actually I re-validated that the latest version of flatten-maven-plugin and maven-shade-plugin are working together.

I was using flatten-maven-plugin to use the ci-friendly version and earlier I was facing the issue where the dependency--reduced-pom.xml was still having the ${revision} tag so I had to run the flatten plugin after the maven-shade-plugin.

But I did a couple of changes with my original configuration of the pom.xml and now everything seems to be working fine. So I am not sure if the original issue was fixed and is this required anymore.

altaiezior · 2026-03-09T09:46:28Z

@altaiezior not sure about AI but the model can be injected and we have a writer in the codebase so shouldn't be that hard. My 2 cts would be that since maven 4 has flattening feature built-in it can be worth testing it and working with maven 4 more than maven 3 base now - but API changed, this is why I think we can just play the status quo to only update maven 4 plugin code base once setup.

Yes I am also testing with maven-4 and there has been no issues with maven-4 as we don't use flatten plugin with the same.

jglick · 2026-03-09T13:31:58Z

Regarding testing with Maven 4, beware apache/maven#11370

altaiezior · 2026-03-09T16:53:27Z

I re-checked it with maven 4, actually the dependency reduced pom is being considered as the build pom and not the consumer pom so the final pom being installed is not dependency reduced and hence it is not working with it.

I was going through the discussion here apache/maven#1437 which helps me understand why changing the model is important.

In short, the consumer poms are generated from the model and not the file. So if we are changing and creating a new pom file by any plugin, then one needs to also change the model or else the consumer pom will not be updated.

altaiezior · 2026-03-09T17:40:59Z

@rmannibucau I think this issue can be closed as flatten-maven-plugin now implements setting originalModel which can then be consumed by maven-shade-plugin

mojohaus/flatten-maven-plugin@f85967a

when updatePomFile property is set

rmannibucau reviewed Apr 6, 2022

View reviewed changes

jglick reviewed Apr 6, 2022

View reviewed changes

gzm55 added 2 commits April 6, 2022 20:16

fix description and style

a0f8848

set ModelReader to parse pom file

221c760

clayreimann mentioned this pull request May 31, 2022

Fix #100 Set project.originalModel after flattening mojohaus/flatten-maven-plugin#281

Merged

gzm55 mentioned this pull request Jul 5, 2022

flatten-vs-shade incompatibility mojohaus/flatten-maven-plugin#100

Closed

Conversation

gzm55 commented Apr 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gzm55 Apr 6, 2022 • edited by gnodet Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jira-importer commented Jun 14, 2025

Uh oh!

jira-importer commented Jun 14, 2025

Uh oh!

altaiezior commented Jan 21, 2026

Uh oh!

altaiezior commented Mar 8, 2026

Uh oh!

cstamas commented Mar 8, 2026

Uh oh!

rmannibucau commented Mar 8, 2026

Uh oh!

altaiezior commented Mar 8, 2026

Uh oh!

rmannibucau commented Mar 9, 2026

Uh oh!

altaiezior commented Mar 9, 2026

Uh oh!

altaiezior commented Mar 9, 2026

Uh oh!

altaiezior commented Mar 9, 2026

Uh oh!

jglick commented Mar 9, 2026

Uh oh!

altaiezior commented Mar 9, 2026

Uh oh!

altaiezior commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gzm55 commented Apr 6, 2022 •

edited

Loading

gzm55 Apr 6, 2022 •

edited by gnodet

Loading