[
https://issues.apache.org/jira/browse/MSHADE-339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17005051#comment-17005051
]
Peter De Maeyer edited comment on MSHADE-339 at 12/30/19 9:31 AM:
------------------------------------------------------------------
{quote}
IMHO if we consider that the MavenProjectHelper parameter is a type (which it
actually is given the implementation), we should attach with "test-jar" type
and no classifier, since the implementation will fill with the default
classifier https://maven.apache.org/ref/3.6.3/maven-core/artifact-handlers.html
{quote}
I agree with you there, but if we do that consistently in the implementation of
{{ShadeMojo}}, that's a much bigger change than I was willing to do in the
context of this issue. I was planning to create a separate issue for that at
some point.
Everywhere in the implementation {{ShadeMojo}} where
{{projectHelper.attachArtifact}} is used, we should pass {{null}} as
classifier. In fact, the only reason to ever pass a non-null classifier is if
you intend to use a _different_ one than the default. Classifiers and extension
should always be gotten from the {{ArtifactHandler}} instead of the hardcoded
ones that are currently used. If you do that, it would be easier to prove that
"jar" is the wrong type, and it should really be "test-jar".
{quote}
if we consider we want to use it more as an extension (which is IMHO more
logical given
https://maven.apache.org/shared/maven-artifact-transfer/comparison.html), we
should use "jar" extension and "tests" classifier
{quote}
I disagree with you there.
You shouldn't use type "as an extension" nor "as a classifier", because type,
extension and classifier are 3 different things.
Let me elaborate:
There are 3 relevant types in the context of {{ShadeMojo}}: "jar", "test-jar"
and "java-source".
The type you pass to {{(Default)MavenProjectHelper.attachArtifact}} is used to
look up an {{ArtifactHandler}}.
The {{ArtifactHandler}} defines the extension and the default classifier.
By using "jar" instead of "test-jar", you in fact have the wrong
{{ArtifactHandler}}.
It works by accident because "jar" happens to have the same extension as
"test-jar", and because the {{ArtifactHandler}} isn't really used much in the
rest of the code because the classifier "tests" is passed in to override it,
but that doesn't make it right.
was (Author: peterdm):
{quote}
IMHO if we consider that the MavenProjectHelper parameter is a type (which it
actually is given the implementation), we should attach with "test-jar" type
and no classifier, since the implementation will fill with the default
classifier https://maven.apache.org/ref/3.6.3/maven-core/artifact-handlers.html
{quote}
I agree with you there, but if we do that consistently in the implementation of
{{ShadeMojo}}, that's a much bigger change than I was willing to do in the
context of this issue. I was planning to create a separate issue for that at
some point.
# Everywhere in the implementation {{ShadeMojo}} where
{{projectHelper.attachArtifact}} is used, we should pass {{null}} as
classifier. In fact, the only reason to ever pass a non-null classifier is if
you intend to use a _different_ one than the default. Classifiers and extension
should always be gotten from the {{ArtifactHandler}} instead of the hardcoded
ones that are currently used. If you do that, it would be easier to prove that
"jar" is the wrong type, and it should really be "test-jar".
{quote}
if we consider we want to use it more as an extension (which is IMHO more
logical given
https://maven.apache.org/shared/maven-artifact-transfer/comparison.html), we
should use "jar" extension and "tests" classifier
{quote}
I disagree with you there.
You shouldn't use type "as an extension" nor "as a classifier", because type,
extension and classifier are 3 different things.
Let me elaborate:
There are 3 relevant types in the context of {{ShadeMojo}}: "jar", "test-jar"
and "java-source".
The type you pass to {{(Default)MavenProjectHelper.attachArtifact}} is used to
look up an {{ArtifactHandler}}.
The {{ArtifactHandler}} defines the extension and the default classifier.
By using "jar" instead of "test-jar", you in fact have the wrong
{{ArtifactHandler}}.
It works by accident because "jar" happens to have the same extension as
"test-jar", and because the {{ArtifactHandler}} isn't really used much in the
rest of the code because the classifier "tests" is passed in to override it,
but that doesn't make it right.
> Shaded test jar has wrong type "jar"
> ------------------------------------
>
> Key: MSHADE-339
> URL: https://issues.apache.org/jira/browse/MSHADE-339
> Project: Maven Shade Plugin
> Issue Type: Bug
> Affects Versions: 2.2, 3.2.2
> Reporter: Peter De Maeyer
> Priority: Minor
> Fix For: 3.2.2
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The shaded test jar has the wrong type "jar".
> It should be "test-jar".
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
