Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent duplicate generation of sources and javadocs artefacts during a release build #596

Merged
merged 1 commit into from
Aug 4, 2021
Merged

Prevent duplicate generation of sources and javadocs artefacts during a release build #596

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 37 additions & 5 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,6 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId>
<version>${maven-release-plugin.version}</version>
</plugin>
<plugin>
<groupId>com.mycila</groupId>
<artifactId>license-maven-plugin</artifactId>
Expand Down Expand Up @@ -532,6 +527,16 @@
</defaultProperties>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId>
<version>${maven-release-plugin.version}</version>
<configuration>
<!-- Use our own profile instead of the default "release-profile" -->
<releaseProfiles>release</releaseProfiles>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
Expand Down Expand Up @@ -580,6 +585,33 @@
<maven.javadoc.failOnWarnings>false</maven.javadoc.failOnWarnings>
</properties>
</profile>

<!-- Profile activated during release:prepare and release:perform instead
of the legacy release-profile that is otherwise activated during a release
build by the maven-release-plugin.

Reasons are:
- the "sources" artifact is produced by the shade plugin. The one produced by
the legacy profile must be discarded.
- javadoc is already produced by normal builds.

Note: maven-deploy-plugin:3.0.0 does not support the "updateReleaseInfo" configuration
parameter anymore. This profile can be removed after upgrading.
Comment on lines +598 to +599
Copy link
Collaborator

@philsttr philsttr Aug 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm. logstash-logback-encoder does not use the maven-deploy-plugin. It uses the nexus-staging-maven-plugin instead, which overrides the deploy goals.

So, I'm a little confused as to why the maven-deploy-plugin is configured here.

I also just noticed you added maven-deploy-plugin stuff in 85ee44b, but that shouldn't be needed either. (?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The release-profile is inherited from Maven Super POM (see https://maven.apache.org/pom.html, Super POM, line 115). This profile is activated by the release plugin and has the Deploy Plugin in its section.

Changes here are meant to remove the javadoc/sources stuff while keeping the rest "untouched".

Note that the "release-profile" will be removed in future Maven versions. Also, if you are 100% certain the deploy plugin is not used at all - then we can simply get rid of this profile and configure the release plugin to not active any at all (which will be simpler).

Copy link
Collaborator

Choose a reason for hiding this comment

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

if you are 100% certain the deploy plugin is not used at all - then we can simply get rid of this profile and configure the release plugin to not active any at all (which will be simpler).

My understanding of the nexus-staging-maven-plugin is that it replaces all the functionality of the maven-deploy-plugin...

From its documentation, it "automatically replaces the deploy goal invocation of the Maven Deploy plugin in the deploy Maven life cycle phase with the deploy goal invocation of the Nexus staging Maven plugin."

Was there a problem with the source/javadoc artifacts with previous logstash-logback-encoder versions? Or is this change just an optimization to the build process?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

First of all I changed the build to produce/validate Javadoc during non-release builds as well. The idea was to detect issues before the actual release.

Secondly, as described in #590, 2 sources artefacts were produced and attached to the build. Both were deployed with the second overriding the first one. This is not good and likely to cause issues at some point in the future for the following reasons:

  • ordering is not "explicit" - I mean it is not clear in the pom which one should be kept. A small unrelated change may break it (it is fragile)
  • deploying twice the same artifact (that is same GAV) may cause issue depending on how the repository is configured (allows redeploy or not) - not an issue atm tho
  • there is long lasting issue in Maven around duplicates artefacts attached to the build (should both be kept or the last replaces the first, etc). AFAIK the current behaviour is likely to be changed in future Maven versions... At the end, the rule is: "don't rely on duplicates" ;-)

We are using the nexus-staging-maven-plugin as you mentioned. It looks like we could simply get rid of the release profile... I'll give it a try.

-->
<profile>
<id>release</id>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<configuration>
<updateReleaseInfo>true</updateReleaseInfo>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>

<distributionManagement>
Expand Down