[BEAM-2242] Ensure that jars are shaded correctly by running the jar plugin before the shade plugin #3034
Conversation
This required listing the maven jar plugin earlier within the file so that the jar/test jar ran before shade within the package phase. I also update the maven shade plugin version in archetypes to pickup the fixes in the service file transformer and dropped the usage of finalName in the shade plugin so all artifacts are named the same way.
|
R: @davorbonaci |
|
retest this please |
|
The maven-jar-plugin is not bound everywhere, its only bound in the modules which have maven-shade-plugin and its also excluded in sdks/java/maven-archetypes parent |
…a8 since it was being performed incorrectly
|
R: @kennknowles @davorbonaci passes on my desktop now |
| <!-- Ensure that the Maven jar plugin runs before the Maven | ||
| shade plugin by listing the plugin higher within the file. --> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> |
There was a problem hiding this comment.
If we moved the overrides on the shade plugin to pluginManagement then would the ordering from the parent automatically fix all the submodules?
There was a problem hiding this comment.
It might, but maven docs only say that the execution order of plugins is based first on phase order and then within a single phase it depends on the order in which they are defined in plugns. It doesn't say anything about pluginManagement affecting order. It also says that the order is inherited from parent modules but it is unclear as to what happens if I declare conflicting orders within a child module or just redefine the same plugin within a child module.
There was a problem hiding this comment.
I mean that the fact that we configured the shade plugin in the plugins area is a bug. It cause the order to be changed.
There was a problem hiding this comment.
Your right, filed https://issues.apache.org/jira/browse/BEAM-2252
There was a problem hiding this comment.
Despite my comment about possible alternatives, doing the expedient thing is better right now. I like how surgical this change is.
Plus, even the best possible config is not going to be particularly clean, and I'd even argue that the inheritance just muddles readability while coupling things that don't need to be.
lukecwik
changed the title
| <!-- | ||
| Configures `mvn package` to produce a bundled jar ("fat jar") for runners | ||
| that require this for job submission to a cluster. | ||
| --> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-shade-plugin</artifactId> | ||
| <version>2.4.1</version> | ||
| <version>3.0.0</version> |
There was a problem hiding this comment.
for later: why is this here at all?
Should be managed in root pom.xml
There was a problem hiding this comment.
This is the generated pom.xml which we should be able to improve by having a tool set the versions based upon the ones defined within the parent pom
There was a problem hiding this comment.
We might want to get rid of the shading or keep it, filed https://issues.apache.org/jira/browse/BEAM-2251
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify.<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
This required listing the maven jar plugin earlier within the file so that the jar/test jar ran before shade within the package phase.
I also update the maven shade plugin version in archetypes to pickup the fixes in the service file transformer and dropped the usage of finalName in the shade plugin so all artifacts are named the same way.