myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leonardo Uribe <lu4...@gmail.com>
Subject Re: Use maven-shade-plugin to prevent duplicate code
Date Mon, 08 Nov 2010 23:11:05 GMT
Hi

2010/11/8 David Jencks <david_jencks@yahoo.com>

> If you don't need a "shaded" source jar, have you guys experimented with
> the bundle plugin for generating this jar?
>
>
I have never tried it. Maybe someone else? But in this case have a "shaded"
source jar could be important.

regards,

Leonardo Uribe


> thanks
> david jencks
>
> On Nov 8, 2010, at 2:15 PM, Leonardo Uribe wrote:
>
> Hi
>
> No, I understand well, but maybe we are talking about many possible
> enhancements at the same time.
>
> I know how shade plugin works, after all, I did the necessary changes to
> use
> it on implee6. That's the reason why I know well the problems involved. I
> tried
> to use it in myfaces-test but the conclusion was myfaces builder plugin
> unpack
> goal is better in that specific context. I also tried to do the same with
> shared,
> but again I found all previous problems. The code you have on the branch is
> similar to the attempt I did, but I investigated much more about it.
>
> I'll do a brief resume of the discussion. In that way we'll be sure we are
> taking
> of the same thing.
>
> Q: What do we want to do?
>
> The central point is how to refactor or change the way shared works, so we
> don't
> need to to a full build each time a change is done.
>
> Q: Which are the problems when using maven shade plugin?
>
> 1. Source jar file is not updated, so IDEs will have problems with it.
> 2. Maven shade plugin does not play well with bundle plugin.
> 3. Some shared_impl classes are public, so we can't change its package
> name.
>     without break backwards compatibility.
>
> Q: Could shared be a submodule on myfaces core project instead a separate
> project?
>
> Only if we can release shared in a independent way. In my opinion it is
> better let it
> as is.
>
> Q: Mojarra will provide an artifact with api and impl bundle together,
> could we do the same?
>
> Yes, but for that artifact we need to generate a new manifest.mf file. By
> problem 2, the only option
> is use unpack goal. Configure that one will be tricky. I'll create a
> submodule for this one.
>
> Q: So, what alternative do we have at this point?
>
> 1. Keep public shared classes with package shared_impl. Maybe the only one
> is DelegateServlet.
> 2. Refactor impl module to use "org.apache.myfaces.shared" instead
> "org.apache.myfaces.shared_impl".
> 3. Use shade plugin and ignore the problem 2 (we can do it since those
> packages will not be exported).
> 4. Check manifest.mf and source jar to see if everything is ok.
> 5. Leave shared project as is. We don't need to do any changes there.
>
> That's what I'm thinking. Does that sounds good? Do you see any
> incongruence or
> misunderstanding? It that so, just let me know.
>
> Suggestions are welcome.
>
> regards,
>
> Leonardo Uribe
>
> 2010/11/8 Jakob Korherr <jakob.korherr@gmail.com>
>
>> Hi
>>
>> Leo, you are getting this all wrong. Please take a look at the
>> shade-branch, which I created, and then we can continue this
>> discussion.
>>
>> Thanks.
>>
>> Regards,
>> Jakob
>>
>> 2010/11/8 Leonardo Uribe <lu4242@gmail.com>:
>> > Hi
>> >
>> > 2010/11/8 Gerhard <gerhard.petracek@gmail.com>
>> >>
>> >> hi,
>> >> i didn't talk about copying the code to the impl. module. it would be a
>> >> normal module (similar to the shared module) which gets shadded into
>> the
>> >> impl. module.
>> >> actually both approaches are very similar. so you have the same
>> advantages
>> >> (compared to the shared module) and it's easier to handle during the
>> >> development process.
>> >
>> > Ok, but if that so, the advantage of the current configuration is we can
>> > release
>> > shared code without release myfaces core. If we put shared code as a
>> > submodule
>> > of myfaces core and we need to release tomahawk or orchestra or other
>> > project
>> > that uses shared code we'll need to release core first.
>> >
>> > regards,
>> >
>> > Leonardo Uribe
>> >
>> >>
>> >> regards,
>> >> gerhard
>> >>
>> >> http://www.irian.at
>> >>
>> >> Your JSF powerhouse -
>> >> JSF Consulting, Development and
>> >> Courses in English and German
>> >>
>> >> Professional Support for Apache MyFaces
>> >>
>> >>
>> >>
>> >> 2010/11/8 Leonardo Uribe <lu4242@gmail.com>
>> >>>
>> >>> Hi
>> >>>
>> >>> 2010/11/8 Gerhard <gerhard.petracek@gmail.com>
>> >>>>
>> >>>> again - i agree with jakob!
>> >>>> such an >additional< all-in-one dist won't change the situation
for
>> osgi
>> >>>> users. (for now) they just have to stick with the current jar files.
>> >>>> @shared:
>> >>>> the classes of the shared module would be in the impl. module, if
we
>> >>>> don’t (have to) share them with other myfaces sub-projects.
>> >>>
>> >>> The advantage of have shared in a separate module is we ensure all
>> shared
>> >>> code only depends
>> >>> of jsf api. If we put that code inside myfaces impl, we have the risk
>> of
>> >>> mix code and then
>> >>> well see ClassNotFoundException or things like that when libraries
>> like
>> >>> tomahawk or
>> >>> in the future portlet bridge are used with mojarra.
>> >>>
>> >>> regards,
>> >>>
>> >>> Leonardo Uribe
>> >>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> regards,
>> >>>> gerhard
>> >>>>
>> >>>> http://www.irian.at
>> >>>>
>> >>>> Your JSF powerhouse -
>> >>>> JSF Consulting, Development and
>> >>>> Courses in English and German
>> >>>>
>> >>>> Professional Support for Apache MyFaces
>> >>>>
>> >>>>
>> >>>>
>> >>>> 2010/11/8 Jakob Korherr <jakob.korherr@gmail.com>
>> >>>>>
>> >>>>> Hi,
>> >>>>>
>> >>>>> IMHO shared code ist just as private as myfaces-impl code. Not
more,
>> >>>>> not less.
>> >>>>>
>> >>>>> Adding the all-in-one-jar is not a change, but an improvement.
It is
>> >>>>> just an additional (non-OSGi-ready) distribution form of MyFaces
>> code
>> >>>>> and thus does not affect the problems we're having with
>> myfaces-shared
>> >>>>> and OSGi.
>> >>>>>
>> >>>>> Regards,
>> >>>>> Jakob
>> >>>>>
>> >>>>> 2010/11/8 Leonardo Uribe <lu4242@gmail.com>:
>> >>>>> > Hi
>> >>>>> >
>> >>>>> > 2010/11/8 Jakob Korherr <jakob.korherr@gmail.com>
>> >>>>> >>
>> >>>>> >> Hi,
>> >>>>> >>
>> >>>>> >> Last week I created a branch (see [1]) to test the
shade module
>> >>>>> >> integration of shared and also implee6 for MyFaces
core.
>> >>>>> >> Coincidentally, Leonardo committed a similar solution
to MyFaces
>> >>>>> >> core
>> >>>>> >> trunk, however only for the implee6 integration.
>> >>>>> >>
>> >>>>> >> The branch at [1] uses the shade plugin to include
shared and
>> >>>>> >> implee6.
>> >>>>> >> For shared it uses a dependency to myfaces-shared-core
(NOT
>> >>>>> >> shared-impl), which will then be shaded to org.apache.myfaces.*
>> >>>>> >> (without the shared-package) - however this is only
a prototype.
>> To
>> >>>>> >> make this work I had to rename all imports in myfaces-impl
from
>> >>>>> >> "shared_impl" to "shared". Everything works pretty
well expect
>> for
>> >>>>> >> the
>> >>>>> >> OSGi-issues mentioned by Leonardo.
>> >>>>> >>
>> >>>>> >> Using this branch you are able to work on MyFaces shared
classes
>> in
>> >>>>> >> the context of MyFaces core and not having to do a
whole maven
>> build
>> >>>>> >> when testing it, because your IDE uses shared directly
as a
>> >>>>> >> dependency. Thus it really is an improvement to what
we have now
>> and
>> >>>>> >> we should try to fix the OSGi issues in some way to
really make
>> this
>> >>>>> >> work. I already did some work in this direction and
I think that
>> a
>> >>>>> >> ResourceTransformer implementation for shade that creates
the
>> >>>>> >> Manifest
>> >>>>> >> file for OSGi is the way to go, but we certainly have
to discuss
>> >>>>> >> this
>> >>>>> >> (maybe also with the bundle-plugin team). WDYT Leo?
>> >>>>> >>
>> >>>>> >
>> >>>>> > Well, before try to do something like that (a ResourceTransformer
>> >>>>> > implementation)
>> >>>>> > it is good to ask if it is really necessary to do that.
On a
>> previous
>> >>>>> > mail I
>> >>>>> > said that
>> >>>>> > "shared" code should be private, so there should not be
used for
>> >>>>> > users
>> >>>>> > outside
>> >>>>> > myfaces impl. There are exceptions (DelegateServlet), so
we have
>> to
>> >>>>> > identify
>> >>>>> > first
>> >>>>> > which code could not be relocated. The effect on maven
bundle
>> plugin
>> >>>>> > is
>> >>>>> > shared packages are excluded from Export-Package header,
but as
>> long
>> >>>>> > as
>> >>>>> > users
>> >>>>> > don't have code importing shared_impl package it is ok
to ignore
>> this
>> >>>>> > side
>> >>>>> > effect.
>> >>>>> >
>> >>>>> >>
>> >>>>> >> However, please take a look at the branch at [1] and
try to use
>> it
>> >>>>> >> in
>> >>>>> >> your IDE - I think it's really great! (... and furthermore
I
>> think
>> >>>>> >> it's much easier to understand for every myfaces-developer).
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> I also totally agree with Gerhard that we should provide
this
>> >>>>> >> all-in-one jar, even if it may cause problems in OSGi,
because
>> our
>> >>>>> >> OSGi users will most certainly know that. It's really
easy to do
>> >>>>> >> this
>> >>>>> >> using the shade plugin and it provides a very convenient
way for
>> >>>>> >> developers to use MyFaces (especially when they're
not using
>> maven).
>> >>>>> >> As Gerhard mentioned, Mojarra will do the same and
furthermore
>> other
>> >>>>> >> projects like e.g. Weld also provide this all-in-one
solution
>> (-->
>> >>>>> >> weld-servlet).
>> >>>>> >>
>> >>>>> >
>> >>>>> > I disagree. Our first priority should be myfaces usage
in
>> different
>> >>>>> > environments,
>> >>>>> > and then enhance IDE support. Only if the two previous
objections
>> can
>> >>>>> > be
>> >>>>> > solved,
>> >>>>> > the change can be made.
>> >>>>> >
>> >>>>> > regards,
>> >>>>> >
>> >>>>> > Leonardo Uribe
>> >>>>> >
>> >>>>> >>
>> >>>>> >> Regards,
>> >>>>> >> Jakob
>> >>>>> >>
>> >>>>> >> [1]
>> >>>>> >>
>> >>>>> >>
>> http://svn.apache.org/repos/asf/myfaces/core/branches/2_0_3_snapshot_shade_test/
>> >>>>> >>
>> >>>>> >> 2010/11/8 Leonardo Uribe <lu4242@gmail.com>:
>> >>>>> >> > Hi
>> >>>>> >> >
>> >>>>> >> > 2010/11/8 Gerhard <gerhard.petracek@gmail.com>
>> >>>>> >> >>
>> >>>>> >> >> hi,
>> >>>>> >> >> @ide-support:
>> >>>>> >> >> since you get an additional all-in-one sources
jar file, it
>> >>>>> >> >> should
>> >>>>> >> >> work.
>> >>>>> >> >> i've created external codi examples which
use the all-in-one
>> jar
>> >>>>> >> >> of
>> >>>>> >> >> codi
>> >>>>> >> >> and the ide support works perfectly.
>> >>>>> >> >
>> >>>>> >> > Yes, that's true (I checked that code) but in
shared you need
>> to
>> >>>>> >> > change
>> >>>>> >> > the
>> >>>>> >> > package name to org.apache.myfaces.shared_impl.
>> >>>>> >> >
>> >>>>> >> > Really that package renaming is questionable.
Why? It exists
>> since
>> >>>>> >> > 1.1.x
>> >>>>> >> > but
>> >>>>> >> > I don't know why this is necessary. In theory,
the code inside
>> >>>>> >> > shared
>> >>>>> >> > should
>> >>>>> >> > be "private", but the truth is we have one class
that could be
>> >>>>> >> > consumed
>> >>>>> >> > by
>> >>>>> >> > users:
>> >>>>> >> >
>> >>>>> >> >
>> org.apache.myfaces.shared_impl.webapp.webxml.DelegatedFacesServlet.
>> >>>>> >> > That is the main reason why I moved the code proposed
on
>> >>>>> >> > https://issues.apache.org/jira/browse/MYFACES-2944
to
>> myfaces-impl
>> >>>>> >> > package.
>> >>>>> >> >
>> >>>>> >> >>
>> >>>>> >> >> @osgi:
>> >>>>> >> >> if there are restrictions, we should improve
the shade plugin.
>> >>>>> >> >> (for now: osgi users just can't use this optional
all-in-one
>> jar
>> >>>>> >> >> file -
>> >>>>> >> >> if
>> >>>>> >> >> we document it, it shouldn't be a problem.)
>> >>>>> >> >
>> >>>>> >> > There is a discussion of this issue here:
>> >>>>> >> >
>> >>>>> >> > https://issues.apache.org/jira/browse/FELIX-1184
>> >>>>> >> >
>> >>>>> >> > It was reported here too:
>> >>>>> >> >
>> >>>>> >> > http://jira.codehaus.org/browse/MSHADE-51
>> >>>>> >> >
>> >>>>> >> > The issue in maven is here:
>> >>>>> >> >
>> >>>>> >> > http://jira.codehaus.org/browse/MNG-2258
>> >>>>> >> >
>> >>>>> >> > Unfortunately, the only hack I can see to make
this work
>> correctly
>> >>>>> >> > is
>> >>>>> >> > create
>> >>>>> >> > a plugin with a maven lifecycle extension, and
do that is very
>> >>>>> >> > nasty,
>> >>>>> >> > because we need to create a plugin just to do
that.
>> >>>>> >> >
>> >>>>> >> >>
>> >>>>> >> >> @use-case:
>> >>>>> >> >> we should really get rid of the shared module.
>> >>>>> >> >
>> >>>>> >> > I agree. First we need a more explicit plan to
do it.
>> Suggestions
>> >>>>> >> > are
>> >>>>> >> > welcome.
>> >>>>> >> >
>> >>>>> >> > regards,
>> >>>>> >> >
>> >>>>> >> > Leonardo Uribe
>> >>>>> >> >
>> >>>>> >> >>
>> >>>>> >> >> regards,
>> >>>>> >> >> gerhard
>> >>>>> >> >> http://www.irian.at
>> >>>>> >> >>
>> >>>>> >> >> Your JSF powerhouse -
>> >>>>> >> >> JSF Consulting, Development and
>> >>>>> >> >> Courses in English and German
>> >>>>> >> >>
>> >>>>> >> >> Professional Support for Apache MyFaces
>> >>>>> >> >>
>> >>>>> >> >>
>> >>>>> >> >>
>> >>>>> >> >> 2010/11/8 Leonardo Uribe <lu4242@gmail.com>
>> >>>>> >> >>>
>> >>>>> >> >>> Hi
>> >>>>> >> >>>
>> >>>>> >> >>> Unfortunately, maven-shade-plugin has
some unwanted side
>> >>>>> >> >>> effects.
>> >>>>> >> >>>
>> >>>>> >> >>> - The source jar file is not updated too,
so if we "shade"
>> >>>>> >> >>> shared, the
>> >>>>> >> >>> sources are not updated. In theory, the
source jar is used by
>> >>>>> >> >>> IDEs
>> >>>>> >> >>> like
>> >>>>> >> >>> Eclipse or Netbeans to show the source
file of a .class.
>> >>>>> >> >>> - It does not play well with osgi bundle
plugin (the one that
>> >>>>> >> >>> create
>> >>>>> >> >>> manifest.mf). The problem is the manifest
is generated before
>> >>>>> >> >>> "shade",
>> >>>>> >> >>> and
>> >>>>> >> >>> we need the later. Really that one is
a problem related to
>> maven
>> >>>>> >> >>> itself.
>> >>>>> >> >>>
>> >>>>> >> >>> The only valid use case I found where
maven-shade-plugin fits
>> >>>>> >> >>> well is
>> >>>>> >> >>> with implee6 module, but anyway it was
required to do some
>> hacks
>> >>>>> >> >>> to
>> >>>>> >> >>> make
>> >>>>> >> >>> bundle plugin works well.
>> >>>>> >> >>>
>> >>>>> >> >>> regards,
>> >>>>> >> >>>
>> >>>>> >> >>> Leonardo Uribe
>> >>>>> >> >>
>> >>>>> >> >
>> >>>>> >> >
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> --
>> >>>>> >> Jakob Korherr
>> >>>>> >>
>> >>>>> >> blog: http://www.jakobk.com
>> >>>>> >> twitter: http://twitter.com/jakobkorherr
>> >>>>> >> work: http://www.irian.at
>> >>>>> >
>> >>>>> >
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> --
>> >>>>> Jakob Korherr
>> >>>>>
>> >>>>> blog: http://www.jakobk.com
>> >>>>> twitter: http://twitter.com/jakobkorherr
>> >>>>> work: http://www.irian.at
>> >>>>
>> >>>
>> >>
>> >
>> >
>>
>>
>>
>> --
>> Jakob Korherr
>>
>> blog: http://www.jakobk.com
>> twitter: http://twitter.com/jakobkorherr
>> work: http://www.irian.at
>>
>
>
>

Mime
View raw message