You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by nicolas de loof <ni...@apache.org> on 2009/05/04 14:33:50 UTC

DefaultMavenProjectBuilder possible bug ?

Hi guys,
Trying to update the 2.2.x branch to use generics java5 syntax in
collections for better type safety I fall into this :

DefaultMavenProjectBuilder.normalizeToArtifactRepositories() method converts
a List that can contain either Repository or ArtifactRepository to a list of
ArtifactRepository. A new ArrayList is created and returned to contain those
normalised objects.

Beeing a private method the only call is from public
Model findModelFromRepository( ... )  BUT this method does not use the
returned result :

        normalizeToArtifactRepositories( remoteArtifactRepositories,
projectId ); // Missing "List repositories = ..."

Two options there :
- this dead code,
- there is a valid use case to call findModelFromRepository with a
List<Repository>, if true we have a bug here.

Nicolas

Re: DefaultMavenProjectBuilder possible bug ?

Posted by John Casey <jd...@commonjava.org>.
Yes, IF the xwiki build works without that code in place. I guess I'll 
fire up that build today to see.

Benjamin Bentmann wrote:
> nicolas de loof wrote:
> 
>> ------------------------------------------------------------------------
>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 
>> 1 line
>>
>> porting revId 616610 of trunk back to 2.0.x branch
> 
> The commits mentioned suggest that your analysis was right and the code 
> in the 2.x branches is buggy. While in the original r616610 the return 
> value of normalizeToArtifactRepositories() is used, it's not in r616830 
> and the method has no side effects either.
> 
> This naturally leads to the question, if the code in 2.x isn't effective 
> anyway and nothing requires it in practice, should we should get rid of it?
> 
> 
> Benjamin
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: DefaultMavenProjectBuilder possible bug ?

Posted by John Casey <jd...@commonjava.org>.
Yes

Benjamin Bentmann wrote:
> nicolas de loof wrote:
> 
>> ------------------------------------------------------------------------
>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 
>> 1 line
>>
>> porting revId 616610 of trunk back to 2.0.x branch
> 
> The commits mentioned suggest that your analysis was right and the code 
> in the 2.x branches is buggy. While in the original r616610 the return 
> value of normalizeToArtifactRepositories() is used, it's not in r616830 
> and the method has no side effects either.
> 
> This naturally leads to the question, if the code in 2.x isn't effective 
> anyway and nothing requires it in practice, should we should get rid of it?
> 
> 
> Benjamin
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: DefaultMavenProjectBuilder possible bug ?

Posted by Brian Fox <br...@infinity.nu>.
I agree that this (lack of generics) can be a problem, but concur that it's
not the most pressing thing, particularly with 3.x looming ahead. Fixing
bugs in 2.x would serve the users far more and once 3.x stabilizes, then we
can add generics if they aren't there yet.

On Sun, May 10, 2009 at 3:48 PM, nicolas de loof
<ni...@gmail.com>wrote:

> I just was not aware John was creating a RC when I committed into core. My
> current issue with core is that from a plugin developer point of view it is
> really hard to use as too many untyped List are used, and there is no
> Javadoc to explain what is expected. buildFromRepository() is typically one
> of those API method that is difficult to use but required when a plugin has
> to use Artifact.
> My goal is not to apply generics just for the beauty of using generics, but
> to have a strong-typed API on MavenProject and other common classes.
>
>
>
> 2009/5/10 Jason van Zyl <jv...@sonatype.com>
>
> >
> > On 10-May-09, at 8:05 AM, nicolas de loof wrote:
> >
> >  I've started my second attempt to generify the API, using call hierarchy
> >> to
> >> check how collections are used in code.
> >>
> >
> > Nicolas this is not the way to learn the code and help.
> >
> > Pick a particular problem and learn a part of the code base. Littering
> > changes all over the place to code you don't know is not a good idea.
> > Changing everything to generics is not the most pressing need and you
> seem
> > to be causing more problems then necessary. It's fine that you want to
> help
> > but try and focus your efforts until you understand more of the code
> base.
> > You don't need to put generics everywhere to understand a particular set
> of
> > code. Also, changing code while John is trying to release the code is
> also
> > not very helpful.
> >
> > You don't need to trample over everything to exercise your commit privs
> on
> > the core. I would suggest you ask John about what specific needs have to
> be
> > addressed for the next release and pick on of the problems to work on
> > instead of the approach you're taking. Then a specific issues will be
> fixed
> > and you do that a bunch of times and you'll gain far more understanding
> of
> > how the system works.
> >
> >
> >  I still don't know how to handle this buildFromRepository method :
> >> Considering the current remote-resource-plugin releases will wail with a
> >> ClassCastException I'd like to remove the unused normalize method and
> >> define
> >> a strong typed API - the SNAPSHOT code of remote-resource plugin is
> ready
> >> for this.
> >> On the other side maybe it's preferable for buildFromRepository method
> to
> >> support this normalize process, to match the trunk.
> >>
> >> Nicolas
> >>
> >> 2009/5/4 John Casey <jd...@commonjava.org>
> >>
> >>  We should fix it in 2.2.x. Unless there is an actual bug, we shouldn't
> >>> make
> >>> ANY code changes in the RC branch.
> >>>
> >>>
> >>>
> >>> nicolas de loof wrote:
> >>>
> >>>  Not sure to understand you well. Do you mean release 2.2.0 as is,
> remove
> >>>> this normalize code in 2.2.x branch and set buildFromRepository method
> >>>> signature set to List<ArtifactRepository> ?Or fix 2.2.x AND 2.2.RC
> >>>> branches
> >>>> to support the normalization process for better compatibility with
> >>>> previous
> >>>> versions of remote-resources plugin ?
> >>>>
> >>>> 2009/5/4 John Casey <jd...@commonjava.org>
> >>>>
> >>>> Personally, I think we should fix it. We're just now doing 2.2.0-RC1,
> so
> >>>>
> >>>>> there's no reason we can't make a new release of the resources plugin
> >>>>> before
> >>>>> 2.2.1 to get this fixed.
> >>>>>
> >>>>>
> >>>>> nicolas de loof wrote:
> >>>>>
> >>>>> The commit seems to be related to Seems to be related to
> >>>>>
> >>>>>> http://jira.codehaus.org/browse/MRRESOURCES-15
> >>>>>>
> >>>>>> according to remote-resource plugin source in trunk the plugin uses
> >>>>>> the
> >>>>>> expected ProjectUtils.buildArtifactRepositories to build a valid
> >>>>>> List<ArtifactRepository> from a List<Repository>, and don't call
> >>>>>> the buildFromRepository with a un-normalised List<Repository>.
> >>>>>>
> >>>>>> BUT the latest stable 1.0.1 don't use this and
> >>>>>> calls mavenProjectBuilder.buildFromRepository with
> >>>>>> ${project.repositories}.
> >>>>>>
> >>>>>> Based on this, MRRESOURCES-15 seems not to be fixed in maven 2.2.x
> >>>>>> with
> >>>>>> a
> >>>>>> non-snapshot project.
> >>>>>>
> >>>>>> Should we fix the core or expect plugins to use
> >>>>>> buildArtifactRepositories
> >>>>>> builder method - and wait for a 1.0.2 release of
> >>>>>> remote-resources-plugin
> >>>>>> ?
> >>>>>>
> >>>>>>
> >>>>>> 2009/5/4 Benjamin Bentmann <be...@udo.edu>
> >>>>>>
> >>>>>> nicolas de loof wrote:
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>>
> ------------------------------------------------------------------------
> >>>>>>>
> >>>>>>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008)
> |
> >>>>>>>
> >>>>>>>> 1
> >>>>>>>> line
> >>>>>>>>
> >>>>>>>> porting revId 616610 of trunk back to 2.0.x branch
> >>>>>>>>
> >>>>>>>> The commits mentioned suggest that your analysis was right and the
> >>>>>>>> code
> >>>>>>>>
> >>>>>>>>  in
> >>>>>>> the 2.x branches is buggy. While in the original r616610 the return
> >>>>>>> value
> >>>>>>> of
> >>>>>>> normalizeToArtifactRepositories() is used, it's not in r616830 and
> >>>>>>> the
> >>>>>>> method has no side effects either.
> >>>>>>>
> >>>>>>> This naturally leads to the question, if the code in 2.x isn't
> >>>>>>> effective
> >>>>>>> anyway and nothing requires it in practice, should we should get
> rid
> >>>>>>> of
> >>>>>>> it?
> >>>>>>>
> >>>>>>>
> >>>>>>> Benjamin
> >>>>>>>
> >>>>>>>
> >>>>>>>
> ---------------------------------------------------------------------
> >>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> ---------------------------------------------------------------------
> >>>>>>>
> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>>>> For additional commands, e-mail: dev-help@maven.apache.org
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>  ---------------------------------------------------------------------
> >>> 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
> > http://twitter.com/jvanzyl
> > http://twitter.com/SonatypeNexus
> > http://twitter.com/SonatypeM2E
> > ----------------------------------------------------------
> >
> > People develop abstractions by generalizing from concrete examples.
> > Every attempt to determine the correct abstraction on paper without
> > actually developing a running system is doomed to failure. No one
> > is that smart. A framework is a resuable design, so you develop it by
> > looking at the things it is supposed to be a design of. The more examples
> > you look at, the more general your framework will be.
> >
> >  -- Ralph Johnson & Don Roberts, Patterns for Evolving Frameworks
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> >
> >
>

Re: DefaultMavenProjectBuilder possible bug ?

Posted by nicolas de loof <ni...@gmail.com>.
I just was not aware John was creating a RC when I committed into core. My
current issue with core is that from a plugin developer point of view it is
really hard to use as too many untyped List are used, and there is no
Javadoc to explain what is expected. buildFromRepository() is typically one
of those API method that is difficult to use but required when a plugin has
to use Artifact.
My goal is not to apply generics just for the beauty of using generics, but
to have a strong-typed API on MavenProject and other common classes.



2009/5/10 Jason van Zyl <jv...@sonatype.com>

>
> On 10-May-09, at 8:05 AM, nicolas de loof wrote:
>
>  I've started my second attempt to generify the API, using call hierarchy
>> to
>> check how collections are used in code.
>>
>
> Nicolas this is not the way to learn the code and help.
>
> Pick a particular problem and learn a part of the code base. Littering
> changes all over the place to code you don't know is not a good idea.
> Changing everything to generics is not the most pressing need and you seem
> to be causing more problems then necessary. It's fine that you want to help
> but try and focus your efforts until you understand more of the code base.
> You don't need to put generics everywhere to understand a particular set of
> code. Also, changing code while John is trying to release the code is also
> not very helpful.
>
> You don't need to trample over everything to exercise your commit privs on
> the core. I would suggest you ask John about what specific needs have to be
> addressed for the next release and pick on of the problems to work on
> instead of the approach you're taking. Then a specific issues will be fixed
> and you do that a bunch of times and you'll gain far more understanding of
> how the system works.
>
>
>  I still don't know how to handle this buildFromRepository method :
>> Considering the current remote-resource-plugin releases will wail with a
>> ClassCastException I'd like to remove the unused normalize method and
>> define
>> a strong typed API - the SNAPSHOT code of remote-resource plugin is ready
>> for this.
>> On the other side maybe it's preferable for buildFromRepository method to
>> support this normalize process, to match the trunk.
>>
>> Nicolas
>>
>> 2009/5/4 John Casey <jd...@commonjava.org>
>>
>>  We should fix it in 2.2.x. Unless there is an actual bug, we shouldn't
>>> make
>>> ANY code changes in the RC branch.
>>>
>>>
>>>
>>> nicolas de loof wrote:
>>>
>>>  Not sure to understand you well. Do you mean release 2.2.0 as is, remove
>>>> this normalize code in 2.2.x branch and set buildFromRepository method
>>>> signature set to List<ArtifactRepository> ?Or fix 2.2.x AND 2.2.RC
>>>> branches
>>>> to support the normalization process for better compatibility with
>>>> previous
>>>> versions of remote-resources plugin ?
>>>>
>>>> 2009/5/4 John Casey <jd...@commonjava.org>
>>>>
>>>> Personally, I think we should fix it. We're just now doing 2.2.0-RC1, so
>>>>
>>>>> there's no reason we can't make a new release of the resources plugin
>>>>> before
>>>>> 2.2.1 to get this fixed.
>>>>>
>>>>>
>>>>> nicolas de loof wrote:
>>>>>
>>>>> The commit seems to be related to Seems to be related to
>>>>>
>>>>>> http://jira.codehaus.org/browse/MRRESOURCES-15
>>>>>>
>>>>>> according to remote-resource plugin source in trunk the plugin uses
>>>>>> the
>>>>>> expected ProjectUtils.buildArtifactRepositories to build a valid
>>>>>> List<ArtifactRepository> from a List<Repository>, and don't call
>>>>>> the buildFromRepository with a un-normalised List<Repository>.
>>>>>>
>>>>>> BUT the latest stable 1.0.1 don't use this and
>>>>>> calls mavenProjectBuilder.buildFromRepository with
>>>>>> ${project.repositories}.
>>>>>>
>>>>>> Based on this, MRRESOURCES-15 seems not to be fixed in maven 2.2.x
>>>>>> with
>>>>>> a
>>>>>> non-snapshot project.
>>>>>>
>>>>>> Should we fix the core or expect plugins to use
>>>>>> buildArtifactRepositories
>>>>>> builder method - and wait for a 1.0.2 release of
>>>>>> remote-resources-plugin
>>>>>> ?
>>>>>>
>>>>>>
>>>>>> 2009/5/4 Benjamin Bentmann <be...@udo.edu>
>>>>>>
>>>>>> nicolas de loof wrote:
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> ------------------------------------------------------------------------
>>>>>>>
>>>>>>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) |
>>>>>>>
>>>>>>>> 1
>>>>>>>> line
>>>>>>>>
>>>>>>>> porting revId 616610 of trunk back to 2.0.x branch
>>>>>>>>
>>>>>>>> The commits mentioned suggest that your analysis was right and the
>>>>>>>> code
>>>>>>>>
>>>>>>>>  in
>>>>>>> the 2.x branches is buggy. While in the original r616610 the return
>>>>>>> value
>>>>>>> of
>>>>>>> normalizeToArtifactRepositories() is used, it's not in r616830 and
>>>>>>> the
>>>>>>> method has no side effects either.
>>>>>>>
>>>>>>> This naturally leads to the question, if the code in 2.x isn't
>>>>>>> effective
>>>>>>> anyway and nothing requires it in practice, should we should get rid
>>>>>>> of
>>>>>>> it?
>>>>>>>
>>>>>>>
>>>>>>> Benjamin
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>>
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>
>>>>>
>>>>>
>>>>>
>>>>  ---------------------------------------------------------------------
>>> 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
> http://twitter.com/jvanzyl
> http://twitter.com/SonatypeNexus
> http://twitter.com/SonatypeM2E
> ----------------------------------------------------------
>
> People develop abstractions by generalizing from concrete examples.
> Every attempt to determine the correct abstraction on paper without
> actually developing a running system is doomed to failure. No one
> is that smart. A framework is a resuable design, so you develop it by
> looking at the things it is supposed to be a design of. The more examples
> you look at, the more general your framework will be.
>
>  -- Ralph Johnson & Don Roberts, Patterns for Evolving Frameworks
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: DefaultMavenProjectBuilder possible bug ?

Posted by Jason van Zyl <jv...@sonatype.com>.
On 10-May-09, at 8:05 AM, nicolas de loof wrote:

> I've started my second attempt to generify the API, using call  
> hierarchy to
> check how collections are used in code.

Nicolas this is not the way to learn the code and help.

Pick a particular problem and learn a part of the code base. Littering  
changes all over the place to code you don't know is not a good idea.  
Changing everything to generics is not the most pressing need and you  
seem to be causing more problems then necessary. It's fine that you  
want to help but try and focus your efforts until you understand more  
of the code base. You don't need to put generics everywhere to  
understand a particular set of code. Also, changing code while John is  
trying to release the code is also not very helpful.

You don't need to trample over everything to exercise your commit  
privs on the core. I would suggest you ask John about what specific  
needs have to be addressed for the next release and pick on of the  
problems to work on instead of the approach you're taking. Then a  
specific issues will be fixed and you do that a bunch of times and  
you'll gain far more understanding of how the system works.

> I still don't know how to handle this buildFromRepository method :
> Considering the current remote-resource-plugin releases will wail  
> with a
> ClassCastException I'd like to remove the unused normalize method  
> and define
> a strong typed API - the SNAPSHOT code of remote-resource plugin is  
> ready
> for this.
> On the other side maybe it's preferable for buildFromRepository  
> method to
> support this normalize process, to match the trunk.
>
> Nicolas
>
> 2009/5/4 John Casey <jd...@commonjava.org>
>
>> We should fix it in 2.2.x. Unless there is an actual bug, we  
>> shouldn't make
>> ANY code changes in the RC branch.
>>
>>
>>
>> nicolas de loof wrote:
>>
>>> Not sure to understand you well. Do you mean release 2.2.0 as is,  
>>> remove
>>> this normalize code in 2.2.x branch and set buildFromRepository  
>>> method
>>> signature set to List<ArtifactRepository> ?Or fix 2.2.x AND 2.2.RC
>>> branches
>>> to support the normalization process for better compatibility with
>>> previous
>>> versions of remote-resources plugin ?
>>>
>>> 2009/5/4 John Casey <jd...@commonjava.org>
>>>
>>> Personally, I think we should fix it. We're just now doing 2.2.0- 
>>> RC1, so
>>>> there's no reason we can't make a new release of the resources  
>>>> plugin
>>>> before
>>>> 2.2.1 to get this fixed.
>>>>
>>>>
>>>> nicolas de loof wrote:
>>>>
>>>> The commit seems to be related to Seems to be related to
>>>>> http://jira.codehaus.org/browse/MRRESOURCES-15
>>>>>
>>>>> according to remote-resource plugin source in trunk the plugin  
>>>>> uses the
>>>>> expected ProjectUtils.buildArtifactRepositories to build a valid
>>>>> List<ArtifactRepository> from a List<Repository>, and don't call
>>>>> the buildFromRepository with a un-normalised List<Repository>.
>>>>>
>>>>> BUT the latest stable 1.0.1 don't use this and
>>>>> calls mavenProjectBuilder.buildFromRepository with
>>>>> ${project.repositories}.
>>>>>
>>>>> Based on this, MRRESOURCES-15 seems not to be fixed in maven  
>>>>> 2.2.x with
>>>>> a
>>>>> non-snapshot project.
>>>>>
>>>>> Should we fix the core or expect plugins to use
>>>>> buildArtifactRepositories
>>>>> builder method - and wait for a 1.0.2 release of remote- 
>>>>> resources-plugin
>>>>> ?
>>>>>
>>>>>
>>>>> 2009/5/4 Benjamin Bentmann <be...@udo.edu>
>>>>>
>>>>> nicolas de loof wrote:
>>>>>
>>>>>>
>>>>>> ------------------------------------------------------------------------
>>>>>>
>>>>>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv.  
>>>>>> 2008) |
>>>>>>> 1
>>>>>>> line
>>>>>>>
>>>>>>> porting revId 616610 of trunk back to 2.0.x branch
>>>>>>>
>>>>>>> The commits mentioned suggest that your analysis was right and  
>>>>>>> the
>>>>>>> code
>>>>>>>
>>>>>> in
>>>>>> the 2.x branches is buggy. While in the original r616610 the  
>>>>>> return
>>>>>> value
>>>>>> of
>>>>>> normalizeToArtifactRepositories() is used, it's not in r616830  
>>>>>> and the
>>>>>> method has no side effects either.
>>>>>>
>>>>>> This naturally leads to the question, if the code in 2.x isn't
>>>>>> effective
>>>>>> anyway and nothing requires it in practice, should we should  
>>>>>> get rid of
>>>>>> it?
>>>>>>
>>>>>>
>>>>>> Benjamin
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>>
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>>
>>>>
>>>
>> ---------------------------------------------------------------------
>> 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
http://twitter.com/jvanzyl
http://twitter.com/SonatypeNexus
http://twitter.com/SonatypeM2E
----------------------------------------------------------

People develop abstractions by generalizing from concrete examples.
Every attempt to determine the correct abstraction on paper without
actually developing a running system is doomed to failure. No one
is that smart. A framework is a resuable design, so you develop it by
looking at the things it is supposed to be a design of. The more  
examples
you look at, the more general your framework will be.

   -- Ralph Johnson & Don Roberts, Patterns for Evolving Frameworks


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: DefaultMavenProjectBuilder possible bug ?

Posted by nicolas de loof <ni...@gmail.com>.
I've started my second attempt to generify the API, using call hierarchy to
check how collections are used in code.
I still don't know how to handle this buildFromRepository method :
Considering the current remote-resource-plugin releases will wail with a
ClassCastException I'd like to remove the unused normalize method and define
a strong typed API - the SNAPSHOT code of remote-resource plugin is ready
for this.
On the other side maybe it's preferable for buildFromRepository method to
support this normalize process, to match the trunk.

Nicolas

2009/5/4 John Casey <jd...@commonjava.org>

> We should fix it in 2.2.x. Unless there is an actual bug, we shouldn't make
> ANY code changes in the RC branch.
>
>
>
> nicolas de loof wrote:
>
>> Not sure to understand you well. Do you mean release 2.2.0 as is, remove
>> this normalize code in 2.2.x branch and set buildFromRepository method
>> signature set to List<ArtifactRepository> ?Or fix 2.2.x AND 2.2.RC
>> branches
>> to support the normalization process for better compatibility with
>> previous
>> versions of remote-resources plugin ?
>>
>> 2009/5/4 John Casey <jd...@commonjava.org>
>>
>>  Personally, I think we should fix it. We're just now doing 2.2.0-RC1, so
>>> there's no reason we can't make a new release of the resources plugin
>>> before
>>> 2.2.1 to get this fixed.
>>>
>>>
>>> nicolas de loof wrote:
>>>
>>>  The commit seems to be related to Seems to be related to
>>>> http://jira.codehaus.org/browse/MRRESOURCES-15
>>>>
>>>> according to remote-resource plugin source in trunk the plugin uses the
>>>> expected ProjectUtils.buildArtifactRepositories to build a valid
>>>> List<ArtifactRepository> from a List<Repository>, and don't call
>>>> the buildFromRepository with a un-normalised List<Repository>.
>>>>
>>>> BUT the latest stable 1.0.1 don't use this and
>>>> calls mavenProjectBuilder.buildFromRepository with
>>>> ${project.repositories}.
>>>>
>>>> Based on this, MRRESOURCES-15 seems not to be fixed in maven 2.2.x with
>>>> a
>>>> non-snapshot project.
>>>>
>>>> Should we fix the core or expect plugins to use
>>>> buildArtifactRepositories
>>>> builder method - and wait for a 1.0.2 release of remote-resources-plugin
>>>> ?
>>>>
>>>>
>>>> 2009/5/4 Benjamin Bentmann <be...@udo.edu>
>>>>
>>>>  nicolas de loof wrote:
>>>>
>>>>>
>>>>>  ------------------------------------------------------------------------
>>>>>
>>>>>  r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) |
>>>>>> 1
>>>>>> line
>>>>>>
>>>>>> porting revId 616610 of trunk back to 2.0.x branch
>>>>>>
>>>>>>  The commits mentioned suggest that your analysis was right and the
>>>>>> code
>>>>>>
>>>>> in
>>>>> the 2.x branches is buggy. While in the original r616610 the return
>>>>> value
>>>>> of
>>>>> normalizeToArtifactRepositories() is used, it's not in r616830 and the
>>>>> method has no side effects either.
>>>>>
>>>>> This naturally leads to the question, if the code in 2.x isn't
>>>>> effective
>>>>> anyway and nothing requires it in practice, should we should get rid of
>>>>> it?
>>>>>
>>>>>
>>>>> Benjamin
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>
>>>>>
>>>>>
>>>>>  ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: DefaultMavenProjectBuilder possible bug ?

Posted by John Casey <jd...@commonjava.org>.
We should fix it in 2.2.x. Unless there is an actual bug, we shouldn't 
make ANY code changes in the RC branch.


nicolas de loof wrote:
> Not sure to understand you well. Do you mean release 2.2.0 as is, remove
> this normalize code in 2.2.x branch and set buildFromRepository method
> signature set to List<ArtifactRepository> ?Or fix 2.2.x AND 2.2.RC branches
> to support the normalization process for better compatibility with previous
> versions of remote-resources plugin ?
> 
> 2009/5/4 John Casey <jd...@commonjava.org>
> 
>> Personally, I think we should fix it. We're just now doing 2.2.0-RC1, so
>> there's no reason we can't make a new release of the resources plugin before
>> 2.2.1 to get this fixed.
>>
>>
>> nicolas de loof wrote:
>>
>>> The commit seems to be related to Seems to be related to
>>> http://jira.codehaus.org/browse/MRRESOURCES-15
>>>
>>> according to remote-resource plugin source in trunk the plugin uses the
>>> expected ProjectUtils.buildArtifactRepositories to build a valid
>>> List<ArtifactRepository> from a List<Repository>, and don't call
>>> the buildFromRepository with a un-normalised List<Repository>.
>>>
>>> BUT the latest stable 1.0.1 don't use this and
>>> calls mavenProjectBuilder.buildFromRepository with
>>> ${project.repositories}.
>>>
>>> Based on this, MRRESOURCES-15 seems not to be fixed in maven 2.2.x with a
>>> non-snapshot project.
>>>
>>> Should we fix the core or expect plugins to use buildArtifactRepositories
>>> builder method - and wait for a 1.0.2 release of remote-resources-plugin ?
>>>
>>>
>>> 2009/5/4 Benjamin Bentmann <be...@udo.edu>
>>>
>>>  nicolas de loof wrote:
>>>>  ------------------------------------------------------------------------
>>>>
>>>>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 1
>>>>> line
>>>>>
>>>>> porting revId 616610 of trunk back to 2.0.x branch
>>>>>
>>>>>  The commits mentioned suggest that your analysis was right and the code
>>>> in
>>>> the 2.x branches is buggy. While in the original r616610 the return value
>>>> of
>>>> normalizeToArtifactRepositories() is used, it's not in r616830 and the
>>>> method has no side effects either.
>>>>
>>>> This naturally leads to the question, if the code in 2.x isn't effective
>>>> anyway and nothing requires it in practice, should we should get rid of
>>>> it?
>>>>
>>>>
>>>> Benjamin
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>>
>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: DefaultMavenProjectBuilder possible bug ?

Posted by nicolas de loof <ni...@gmail.com>.
Not sure to understand you well. Do you mean release 2.2.0 as is, remove
this normalize code in 2.2.x branch and set buildFromRepository method
signature set to List<ArtifactRepository> ?Or fix 2.2.x AND 2.2.RC branches
to support the normalization process for better compatibility with previous
versions of remote-resources plugin ?

2009/5/4 John Casey <jd...@commonjava.org>

> Personally, I think we should fix it. We're just now doing 2.2.0-RC1, so
> there's no reason we can't make a new release of the resources plugin before
> 2.2.1 to get this fixed.
>
>
> nicolas de loof wrote:
>
>> The commit seems to be related to Seems to be related to
>> http://jira.codehaus.org/browse/MRRESOURCES-15
>>
>> according to remote-resource plugin source in trunk the plugin uses the
>> expected ProjectUtils.buildArtifactRepositories to build a valid
>> List<ArtifactRepository> from a List<Repository>, and don't call
>> the buildFromRepository with a un-normalised List<Repository>.
>>
>> BUT the latest stable 1.0.1 don't use this and
>> calls mavenProjectBuilder.buildFromRepository with
>> ${project.repositories}.
>>
>> Based on this, MRRESOURCES-15 seems not to be fixed in maven 2.2.x with a
>> non-snapshot project.
>>
>> Should we fix the core or expect plugins to use buildArtifactRepositories
>> builder method - and wait for a 1.0.2 release of remote-resources-plugin ?
>>
>>
>> 2009/5/4 Benjamin Bentmann <be...@udo.edu>
>>
>>  nicolas de loof wrote:
>>>
>>>  ------------------------------------------------------------------------
>>>
>>>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 1
>>>> line
>>>>
>>>> porting revId 616610 of trunk back to 2.0.x branch
>>>>
>>>>  The commits mentioned suggest that your analysis was right and the code
>>> in
>>> the 2.x branches is buggy. While in the original r616610 the return value
>>> of
>>> normalizeToArtifactRepositories() is used, it's not in r616830 and the
>>> method has no side effects either.
>>>
>>> This naturally leads to the question, if the code in 2.x isn't effective
>>> anyway and nothing requires it in practice, should we should get rid of
>>> it?
>>>
>>>
>>> Benjamin
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: DefaultMavenProjectBuilder possible bug ?

Posted by John Casey <jd...@commonjava.org>.
Personally, I think we should fix it. We're just now doing 2.2.0-RC1, so 
there's no reason we can't make a new release of the resources plugin 
before 2.2.1 to get this fixed.

nicolas de loof wrote:
> The commit seems to be related to Seems to be related to
> http://jira.codehaus.org/browse/MRRESOURCES-15
> 
> according to remote-resource plugin source in trunk the plugin uses the
> expected ProjectUtils.buildArtifactRepositories to build a valid
> List<ArtifactRepository> from a List<Repository>, and don't call
> the buildFromRepository with a un-normalised List<Repository>.
> 
> BUT the latest stable 1.0.1 don't use this and
> calls mavenProjectBuilder.buildFromRepository with ${project.repositories}.
> 
> Based on this, MRRESOURCES-15 seems not to be fixed in maven 2.2.x with a
> non-snapshot project.
> 
> Should we fix the core or expect plugins to use buildArtifactRepositories
> builder method - and wait for a 1.0.2 release of remote-resources-plugin ?
> 
> 
> 2009/5/4 Benjamin Bentmann <be...@udo.edu>
> 
>> nicolas de loof wrote:
>>
>>  ------------------------------------------------------------------------
>>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 1
>>> line
>>>
>>> porting revId 616610 of trunk back to 2.0.x branch
>>>
>> The commits mentioned suggest that your analysis was right and the code in
>> the 2.x branches is buggy. While in the original r616610 the return value of
>> normalizeToArtifactRepositories() is used, it's not in r616830 and the
>> method has no side effects either.
>>
>> This naturally leads to the question, if the code in 2.x isn't effective
>> anyway and nothing requires it in practice, should we should get rid of it?
>>
>>
>> Benjamin
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: DefaultMavenProjectBuilder possible bug ?

Posted by nicolas de loof <ni...@gmail.com>.
The commit seems to be related to Seems to be related to
http://jira.codehaus.org/browse/MRRESOURCES-15

according to remote-resource plugin source in trunk the plugin uses the
expected ProjectUtils.buildArtifactRepositories to build a valid
List<ArtifactRepository> from a List<Repository>, and don't call
the buildFromRepository with a un-normalised List<Repository>.

BUT the latest stable 1.0.1 don't use this and
calls mavenProjectBuilder.buildFromRepository with ${project.repositories}.

Based on this, MRRESOURCES-15 seems not to be fixed in maven 2.2.x with a
non-snapshot project.

Should we fix the core or expect plugins to use buildArtifactRepositories
builder method - and wait for a 1.0.2 release of remote-resources-plugin ?


2009/5/4 Benjamin Bentmann <be...@udo.edu>

> nicolas de loof wrote:
>
>  ------------------------------------------------------------------------
>> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 1
>> line
>>
>> porting revId 616610 of trunk back to 2.0.x branch
>>
>
> The commits mentioned suggest that your analysis was right and the code in
> the 2.x branches is buggy. While in the original r616610 the return value of
> normalizeToArtifactRepositories() is used, it's not in r616830 and the
> method has no side effects either.
>
> This naturally leads to the question, if the code in 2.x isn't effective
> anyway and nothing requires it in practice, should we should get rid of it?
>
>
> Benjamin
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: DefaultMavenProjectBuilder possible bug ?

Posted by Benjamin Bentmann <be...@udo.edu>.
nicolas de loof wrote:

> ------------------------------------------------------------------------
> r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 1 line
> 
> porting revId 616610 of trunk back to 2.0.x branch

The commits mentioned suggest that your analysis was right and the code 
in the 2.x branches is buggy. While in the original r616610 the return 
value of normalizeToArtifactRepositories() is used, it's not in r616830 
and the method has no side effects either.

This naturally leads to the question, if the code in 2.x isn't effective 
anyway and nothing requires it in practice, should we should get rid of it?


Benjamin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: DefaultMavenProjectBuilder possible bug ?

Posted by nicolas de loof <ni...@gmail.com>.
Fuuny to see you created this method :)
616830    jdcasey     private List normalizeToArtifactRepositories( List
remoteArtifactRepositories,

------------------------------------------------------------------------
r616830 | jdcasey | 2008-01-30 19:24:41 +0100 (mer., 30 janv. 2008) | 1 line

porting revId 616610 of trunk back to 2.0.x branch, to allow passing in of
Repos
itory (model instance) lists into buildFromRepository(..) without causing
the bu
ild to fail (we're normalizing them to ArtifactRepository instances now).
This w
as a problem with the remote-resources plugin in the xwiki build.
------------------------------------------------------------------------

Seems the remote-resources plugin bug in xwiki build will occur with 2.2.

The side effect is that we can't use strong type collection
for buildFromRepository( .. ) methods as Repository and ArtifactRepository
don't even share a common parent class.


2009/5/4 John Casey <jd...@commonjava.org>

> I put that in in response to a NPE bug that was filed, IIRC. I don't think
> it's dead code, but only expressed in very particular situations. It's
> possible that the plugin/whatever that was expressing it has a new version
> out that no longer causes this problem, too.
>
> Maybe try a `svn annotate` to find out from which commit that method comes,
> and it may give a MNG number.
>
>
> nicolas de loof wrote:
>
>> Hi guys,
>> Trying to update the 2.2.x branch to use generics java5 syntax in
>> collections for better type safety I fall into this :
>>
>> DefaultMavenProjectBuilder.normalizeToArtifactRepositories() method
>> converts
>> a List that can contain either Repository or ArtifactRepository to a list
>> of
>> ArtifactRepository. A new ArrayList is created and returned to contain
>> those
>> normalised objects.
>>
>> Beeing a private method the only call is from public
>> Model findModelFromRepository( ... )  BUT this method does not use the
>> returned result :
>>
>>        normalizeToArtifactRepositories( remoteArtifactRepositories,
>> projectId ); // Missing "List repositories = ..."
>>
>> Two options there :
>> - this dead code,
>> - there is a valid use case to call findModelFromRepository with a
>> List<Repository>, if true we have a bug here.
>>
>> Nicolas
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: DefaultMavenProjectBuilder possible bug ?

Posted by John Casey <jd...@commonjava.org>.
I put that in in response to a NPE bug that was filed, IIRC. I don't 
think it's dead code, but only expressed in very particular situations. 
It's possible that the plugin/whatever that was expressing it has a new 
version out that no longer causes this problem, too.

Maybe try a `svn annotate` to find out from which commit that method 
comes, and it may give a MNG number.

nicolas de loof wrote:
> Hi guys,
> Trying to update the 2.2.x branch to use generics java5 syntax in
> collections for better type safety I fall into this :
> 
> DefaultMavenProjectBuilder.normalizeToArtifactRepositories() method converts
> a List that can contain either Repository or ArtifactRepository to a list of
> ArtifactRepository. A new ArrayList is created and returned to contain those
> normalised objects.
> 
> Beeing a private method the only call is from public
> Model findModelFromRepository( ... )  BUT this method does not use the
> returned result :
> 
>         normalizeToArtifactRepositories( remoteArtifactRepositories,
> projectId ); // Missing "List repositories = ..."
> 
> Two options there :
> - this dead code,
> - there is a valid use case to call findModelFromRepository with a
> List<Repository>, if true we have a bug here.
> 
> Nicolas
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org