-
Notifications
You must be signed in to change notification settings - Fork 408
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.
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
Conversation
Note: maven-deploy-plugin:3.0.0 does not support the "updateReleaseInfo" configuration | ||
parameter anymore. This profile can be removed after upgrading. |
There was a problem hiding this comment.
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. (?)
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
closes #590