karaf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Estermann <ester...@apache.org>
Subject Re: FeaturesProcessorImpl improvement of bundle override
Date Wed, 06 Mar 2019 14:36:29 GMT
Thanks guys for your great feedback! As requested I created everything we need to make it official:

The issue https://issues.apache.org/jira/browse/KARAF-6183
And the PR https://github.com/apache/karaf/pull/772

Cheers
Daniel

On 2019/03/06 06:32:18, Grzegorz Grzybek <gr.grzybek@gmail.com> wrote: 
> Hello
> 
> Looks like I was anticipating such problem by adding "TOCHECK: last rule
> wins - no break!!!" :)
> The diff looks good - if all existing tests pass, I think we can merge it.
> Could you please create KARAF jira issue for this?
> 
> best regards
> Grzegorz Grzybek
> 
> wt., 5 mar 2019 o 18:27 Jean-Baptiste Onofré <jb@nanthrax.net> napisał(a):
> 
> > Thanks for sharing Daniel.
> >
> > I gonna take a look.
> >
> > Regards
> > JB
> >
> > On 05/03/2019 18:19, Daniel Estermann wrote:
> > > Hi everybody,
> > >
> > > we have the following override.properties:
> > >
> > >
> > mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT;range="[0,99999)"
> > >
> > mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration;range="[0,99999)"
> > >
> > > which are supposed to do the following replacements:
> > >
> > > portal-backend-sdk/2.68.3 -> portal-backend-sdk/2.69.0-SNAPSHOT
> > >
> > > and
> > >
> > > portal-backend-sdk/2.68.3/jar/karaf-migration ->
> > > portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration
> > >
> > > But the method
> > >
> > org.apache.karaf.features.internal.service.FeaturesProcessorImpl.staticOverrideBundle(Bundle)
> > > <
> > https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesProcessorImpl.java#L225
> > >
> > > replaces portal-backend-sdk/2.68.3/jar/karaf-migration with
> > > mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT, i.e. a
> > > classified artifact gets replaced with an artifact without classifier.
> > This
> > > happens because the implementation of
> > > org.apache.karaf.features.LocationPattern
> > > <
> > https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/LocationPattern.java#L151
> > >
> > > allows matching of classified URI by a non-classified pattern. The
> > > LocationPatternTest indicates that this is an intentional behavior: see
> > > line 112
> > > <
> > https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L112
> > >,
> > > 115
> > > <
> > https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L115
> > >,
> > > 116
> > > <
> > https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L116
> > >
> > > .
> > >
> > > I can understand why LocationPattern is implemented like that. But then
> > my
> > > guess is that FeaturesProcessorImpl is incorrect. By the way an
> > interesting
> > > comment there: *TOCHECK: last rule wins - no break!!!* Last rule doesn't
> > > work for us so looks like now the time has come to check it. I think what
> > > is crucial there, is that we shouldn't rely on the order of overrides,
> > but
> > > rather on their quality. I.e. not the first/last match is appropriate,
> > but
> > > the best one. I propose to collect candidates to match and then determine
> > > the best one using simply the length as criterion.
> > >
> > > My proposal is already implemented in our fork and tested on my local
> > > system (for now). Here you can see it:
> > >
> > https://github.com/seeburger-ag/karaf/commit/940911e8a8ccdb97d5cebf976e41747f1e7716a4
> > >
> > > I would appreciate to receive any feedback on this.
> > >
> > > Regards,
> > > Daniel
> > >
> >
> 

Mime
View raw message