maven-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peter De Maeyer (Jira)" <j...@apache.org>
Subject [jira] [Comment Edited] (MSHADE-339) Shaded test jar has wrong type "jar"
Date Mon, 30 Dec 2019 09:32:00 GMT

    [ 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)

Mime
View raw message