You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Brett Porter <br...@apache.org> on 2008/12/12 07:02:18 UTC
opinions on MSHADE-42
Hi,
I was looking at applying the patch for this one, but wasn't happy
with it and looked for a better solution, which I'm also not happy
with :)
Basically, in the reactor target/classes is always used as the
classpath for projects referring to dependencies inside the reactor
because of the way it is set in MavenProject. I think it would be
worth us reversing the logic in MavenProject (if the artifact file is
not null, use that, if it is, use the active project) - any thoughts
why that would be a bad idea?
As for shade, to make it work in the reactor, there's 3 options:
- unpack the final JAR into the classesDirectory (the original patch)
- replace the outputDirectory with the JAR (cleaner and more concise,
but effectively makes the output directory read-only from the package
phase, might have unexpected side effects if someone modifies the
output afterwards, though that would seem unwise anyway)
- don't fix it, require people to use <shadedArtifactAttached>true</
shadedArtifactAttached> instead and depend on the one with the
classifier.
Opinions?
Cheers,
Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: opinions on MSHADE-42
Posted by Jason van Zyl <jv...@sonatype.com>.
>
>
> This isn't the entire reactor, but the getClasspathElements()
> method. The only situation I can think of this breaking is:
> * someone trying to pull apart the classpath looking for directories
> and doing something special with them (which would be a horrible
> abuse of the method's contract)
> * someone modifying the outputDirectory after packaging and
> expecting those changes to stick (a more realistic scenario but
> still one that seems unwise to support)
>
> It would seem more important to support the case of modifications to
> the final artifact that are not reflected in the target/classes
> directory (essentially what shade does, but others may as well). Not
> terribly important, but this could still be a change that suits a
> 2.1+ release IMO.
>
Yes, any change like is beyond the scope of 2.0.x.
>>
>>
>>>
>>>
>>> As for shade, to make it work in the reactor, there's 3 options:
>>
>> Trying to use the shade plugin from the reactor is a misuse of the
>> plugin. I specifically made it to work with JARs and it reads
>> entries from JARs.
>
> I'm not quite sure what you mean here. The shade plugin is replacing
> the main artifact with the one it produced, but not reflecting that
> change in the reactor so others are not using. This is the default
> mode for the shade plugin, not some obscure use case.
This was not the primary use case of the shade plugin i.e. replacing
the primarily artifact. The internal logic in 2.0.x is too confused to
try and deal with this case. The shade plugin was specifically made to
create final output after everything else was done. Really a JAR
assembly with the ability to internally rewrite some class references.
>
>
>>
>>
>> I would suggest treating the über JAR like an assembly where you
>> have a separate project.
>
> That's what I've recommended in the mean time.
>
> Thanks,
> Brett
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
Thanks,
Jason
----------------------------------------------------------
Jason van Zyl
Founder, Apache Maven
jason at sonatype dot com
----------------------------------------------------------
happiness is like a butterfly: the more you chase it, the more it will
elude you, but if you turn your attention to other things, it will come
and sit softly on your shoulder ...
-- Thoreau
Re: opinions on MSHADE-42
Posted by Brett Porter <br...@apache.org>.
On 12/12/2008, at 5:47 PM, Jason van Zyl wrote:
>
>
> Sent from my iPhone
>
> On Dec 12, 2008, at 7:02 AM, Brett Porter <br...@apache.org> wrote:
>
>> Hi,
>>
>> I was looking at applying the patch for this one, but wasn't happy
>> with it and looked for a better solution, which I'm also not happy
>> with :)
>>
>> Basically, in the reactor target/classes is always used as the
>> classpath for projects referring to dependencies inside the reactor
>> because of the way it is set in MavenProject. I think it would be
>> worth us reversing the logic in MavenProject (if the artifact file
>> is not null, use that, if it is, use the active project) - any
>> thoughts why that would be a bad idea?
>
> The reactor, if only defined by what it has always been, means
> resolving from the stated build output directory. You can't change
> that fundamental contract at this point. How people work from the
> reactor and/or walk into directories will yield a random mixture of
> artifact and build output directories which I can only see causing
> problems.
This isn't the entire reactor, but the getClasspathElements() method.
The only situation I can think of this breaking is:
* someone trying to pull apart the classpath looking for directories
and doing something special with them (which would be a horrible abuse
of the method's contract)
* someone modifying the outputDirectory after packaging and expecting
those changes to stick (a more realistic scenario but still one that
seems unwise to support)
It would seem more important to support the case of modifications to
the final artifact that are not reflected in the target/classes
directory (essentially what shade does, but others may as well). Not
terribly important, but this could still be a change that suits a 2.1+
release IMO.
>
>
>>
>>
>> As for shade, to make it work in the reactor, there's 3 options:
>
> Trying to use the shade plugin from the reactor is a misuse of the
> plugin. I specifically made it to work with JARs and it reads
> entries from JARs.
I'm not quite sure what you mean here. The shade plugin is replacing
the main artifact with the one it produced, but not reflecting that
change in the reactor so others are not using. This is the default
mode for the shade plugin, not some obscure use case.
>
>
> I would suggest treating the über JAR like an assembly where you
> have a separate project.
That's what I've recommended in the mean time.
Thanks,
Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: opinions on MSHADE-42
Posted by Jason van Zyl <jv...@sonatype.com>.
Sent from my iPhone
On Dec 12, 2008, at 7:02 AM, Brett Porter <br...@apache.org> wrote:
> Hi,
>
> I was looking at applying the patch for this one, but wasn't happy
> with it and looked for a better solution, which I'm also not happy
> with :)
>
> Basically, in the reactor target/classes is always used as the
> classpath for projects referring to dependencies inside the reactor
> because of the way it is set in MavenProject. I think it would be
> worth us reversing the logic in MavenProject (if the artifact file
> is not null, use that, if it is, use the active project) - any
> thoughts why that would be a bad idea?
The reactor, if only defined by what it has always been, means
resolving from the stated build output directory. You can't change
that fundamental contract at this point. How people work from the
reactor and/or walk into directories will yield a random mixture of
artifact and build output directories which I can only see causing
problems.
>
>
> As for shade, to make it work in the reactor, there's 3 options:
Trying to use the shade plugin from the reactor is a misuse of the
plugin. I specifically made it to work with JARs and it reads entries
from JARs.
I would suggest treating the über JAR like an assembly where you have
a separate project.
>
> - unpack the final JAR into the classesDirectory (the original patch)
> - replace the outputDirectory with the JAR (cleaner and more
> concise, but effectively makes the output directory read-only from
> the package phase, might have unexpected side effects if someone
> modifies the output afterwards, though that would seem unwise anyway)
> - don't fix it, require people to use <shadedArtifactAttached>true</
> shadedArtifactAttached> instead and depend on the one with the
> classifier.
>
> Opinions?
>
> Cheers,
> Brett
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org