rmannibucau commented on code in PR #24:
URL:
https://github.com/apache/maven-source-plugin/pull/24#discussion_r1541465699
##########
src/main/java/org/apache/maven/plugins/source/AbstractSourceJarMojo.java:
##########
@@ -303,15 +304,24 @@ protected void packageSources(List<MavenProject>
theProjects) throws MojoExecuti
}
if (attach) {
+ boolean requiresAttach = true;
for (Artifact attachedArtifact :
project.getAttachedArtifacts()) {
- if (isAlreadyAttached(attachedArtifact, project,
getClassifier())) {
- getLog().error("We have duplicated artifacts
attached.");
- throw new MojoExecutionException("Presumably you have
configured maven-source-plugin "
- + "to execute twice in your build. You have to
configure a classifier "
- + "for at least one of them.");
+ Artifact previouslyAttached =
getPreviouslyAttached(attachedArtifact, project, getClassifier());
+ if (previouslyAttached != null) {
+ if (!outputFile.equals(previouslyAttached.getFile())) {
Review Comment:
Not really, I understand this PR but you introduce a regression which is
that you can attach twice the same artifact due to two executions so this "fix"
is actually a regression.
`requiresAttach` is never needed until the build is broken - ie you build
twice the source artifact which means you had a broken one until it is exactly
the same content (why I originally proposed to test it since it must never
happen in sane builds so cost is supposed to be 0).
`requiresAttach` is more a `ignoreAlreadyAttachedArtifact` but looses the
"attached artifact was not wrongly used" by another mojo case (ie you enable to
process a source jar which is not the one deployed).
The fix for the two tickets is quite easy and is on build side (it is
already fixed on maven side):
* disable attach-sources phase which was embedded in maven 3.9 (or use maven
4 which does not have it anymore)
* use the same attach-sources than in 3.9 (jar-no-fork instead of jar)
So think we don't need to loose the check which was introduced in the plugin.
To make it more obvious I think
https://issues.apache.org/jira/browse/MSOURCES-121 is right and shouldn't be
reverted by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
