Re: Inheritance behaviour for MNG-5951 attributes?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Inheritance behaviour for MNG-5951 attributes?

Hervé BOUTEMY
Hi Mark,

Seems you found one more thing that I forgot when merging MNG-6059:
inheritance of these child.inherit.append.path configurations...

I created MNG-6505 [1] and added the fix to MNG-6059 branch.

Can you check that it works as you expect? (notice: with the new
child.x.y.inherit.append.path names...)


Thank you for the tests and reports: this really helps a lot to improve
quality.

Regards,

Hervé

[1] https://issues.apache.org/jira/browse/MNG-6505

Le samedi 3 novembre 2018, 19:11:26 CET Mark Raynsford a écrit :

> Let's say I have the following:
>
> <project>
>   <modelVersion>4.0.0</modelVersion>
>   <groupId>com.example</groupId>
>   <artifactId>a</artifactId>
>   <version>1.0.0</version>
>   <packaging>pom</packaging>
>   <scm child.inherit.append.path="false">
>     <url>https://example.com/a</url>
>     <connection>scm:git:https://example.com/a</connection>
>     <developerConnection>scm:git:https://example.com/a</developerConnection>
> </scm>
> </project>
>
> And then:
>
> <project>
>   <modelVersion>4.0.0</modelVersion>
>   <parent>
>     <groupId>com.example</groupId>
>     <artifactId>a</artifactId>
>     <version>1.0.0</version>
>   </parent>
>   <groupId>com.example</groupId>
>   <artifactId>b</artifactId>
>   <version>1.0.0</version>
>   <packaging>pom</packaging>
>   <scm>
>     <url>https://example.com/b</url>
>     <connection>scm:git:https://example.com/b</connection>
>     <developerConnection>scm:git:https://example.com/b</developerConnection>
> </scm>
> </project>
>
> And then:
>
> <project>
>   <modelVersion>4.0.0</modelVersion>
>   <parent>
>     <groupId>com.example</groupId>
>     <artifactId>b</artifactId>
>     <version>1.0.0</version>
>   </parent>
>   <groupId>com.example</groupId>
>   <artifactId>c</artifactId>
>   <version>1.0.0</version>
>   <packaging>jar</packaging>
> </project>
>
> So that's com.example:a:1.0.0 → com.example:b:1.0.0 →
> com.example:c:1.0.0.
>
> Would you expect com.example:c:1.0.0 to have
> child.inherit.append.path="true" for the (inherited) <scm> element? It
> wasn't clear exactly what semantics were intended to be. What I *think*
> is happening right now is that the <scm> element in com.example:b:1.0.0
> is assigned a value of child.inherit.append.path="true", because "true"
> is the default if something isn't specified and it overrides the value
> specified in com.example:a:1.0.0.





---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Inheritance behaviour for MNG-5951 attributes?

Mark Raynsford-3
On 2018-11-04T13:34:39 +0100
Hervé BOUTEMY <[hidden email]> wrote:

> Hi Mark,
>
> Seems you found one more thing that I forgot when merging MNG-6059:
> inheritance of these child.inherit.append.path configurations...
>
> I created MNG-6505 [1] and added the fix to MNG-6059 branch.
>
> Can you check that it works as you expect? (notice: with the new
> child.x.y.inherit.append.path names...)

Can do! Which code should I be building? Not sure if this work is
happening on a branch somewhere.

> Thank you for the tests and reports: this really helps a lot to improve
> quality.

You're welcome!

--
Mark Raynsford | http://www.io7m.com


attachment0 (235 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Inheritance behaviour for MNG-5951 attributes?

Hervé BOUTEMY
you can build https://github.com/apache/maven/tree/MNG-6059 with quick build
instruction at the end of README

Regards,

Hervé

Le dimanche 4 novembre 2018, 13:48:51 CET Mark Raynsford a écrit :

> On 2018-11-04T13:34:39 +0100
>
> Hervé BOUTEMY <[hidden email]> wrote:
> > Hi Mark,
> >
> > Seems you found one more thing that I forgot when merging MNG-6059:
> > inheritance of these child.inherit.append.path configurations...
> >
> > I created MNG-6505 [1] and added the fix to MNG-6059 branch.
> >
> > Can you check that it works as you expect? (notice: with the new
> > child.x.y.inherit.append.path names...)
>
> Can do! Which code should I be building? Not sure if this work is
> happening on a branch somewhere.
>
> > Thank you for the tests and reports: this really helps a lot to improve
> > quality.
>
> You're welcome!





---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Inheritance behaviour for MNG-5951 attributes?

Mark Raynsford-3
In reply to this post by Hervé BOUTEMY
'Ello.

On 2018-11-06T00:03:53 +0100
Hervé BOUTEMY <[hidden email]> wrote:

> Hi Mark,
>
> even if this is somewhat a corner case (while overriding everything in b, you
> can override the attribute also), it is strange...

I'm not sure it's a corner case, exactly. It reflects the real-life
situation I'm in: Consider a tree where 'a' is the root, and there are
a large number of projects (at the depth of 'b') that all inherit from
'a'. In my case "large" means "more than seventy" so, as you can
imagine, I'm not enthusiastic about going through all of those projects
and adding an attribute just to to fix a bug in Maven. :) In my case,
there are over 400 modules at the depth of 'c' spread across all of the
'b'-level projects.

> I can reproduce the issue in an inheritance unit test (no-append-urls in
> maven-model-builder), but still need to investigate why it does not work as
> intended by ModelMerger.mergeSite(...): you can easily check by removing
> "name" field in b, you'll see that merge for other fields does not work

Good to know it's reproducible in tests.

--
Mark Raynsford | http://www.io7m.com


attachment0 (235 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Inheritance behaviour for MNG-5951 attributes?

Hervé BOUTEMY
ok, I investigated more in details and added a little trick in [1]:
child.site.url.inherit.append.path is inherited independantly from id/name/url

this will manage your edge case, which I can understand is expected

do you think it is ok?

Regards,

Hervé

[1] https://gitbox.apache.org/repos/asf?p=maven.git;a=commit;h=11312d5b275260fbd2b3ed3294da3ccff5413d42

Le mercredi 7 novembre 2018, 00:03:19 CET Hervé BOUTEMY a écrit :

> Le mardi 6 novembre 2018, 10:35:25 CET Mark Raynsford a écrit :
> > 'Ello.
> >
> > On 2018-11-06T00:03:53 +0100
> >
> > Hervé BOUTEMY <[hidden email]> wrote:
> > > Hi Mark,
> > >
> > > even if this is somewhat a corner case (while overriding everything in
> > > b,
> > > you can override the attribute also), it is strange...
> >
> > I'm not sure it's a corner case, exactly. It reflects the real-life
> > situation I'm in: Consider a tree where 'a' is the root, and there are
> > a large number of projects (at the depth of 'b') that all inherit from
> > 'a'. In my case "large" means "more than seventy" so, as you can
> > imagine, I'm not enthusiastic about going through all of those projects
> > and adding an attribute just to to fix a bug in Maven. :) In my case,
> > there are over 400 modules at the depth of 'c' spread across all of the
> > 'b'-level projects.
>
> it's a corner case just because in b, you define
> distributionManagement.site.id, name and url, but not
> @child.site.url.inherit.append.path: just define this attribute while at it
> (instead of counting on inheritance) and it will work as expected. Like
> proposed in https://github.com/io7m/mng-6059-examples/pull/1
> > > I can reproduce the issue in an inheritance unit test (no-append-urls in
> > > maven-model-builder), but still need to investigate why it does not work
> > > as
> > > intended by ModelMerger.mergeSite(...): you can easily check by removing
> > > "name" field in b, you'll see that merge for other fields does not work
> >
> > Good to know it's reproducible in tests.
>
> in fact, you can also reproduce by defining in b:
>   <distributionManagement>
>     <site>
>       <name>name from B</name>
>     </site>
>   </distributionManagement>
>
> If you look at effective pom, you'll see that id and url are unexpectedly
> not inherited...
>
> You discovered an edge case with the new attribute that already exists with
> elements: I still didn't have time yet to investigate on the cause, but
> will do
>
> Regards,
>
> Hervé
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]





---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]