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 2009/02/24 14:58:40 UTC
Re: svn commit: r745955
Hi John,
I found another problem here:
On 20/02/2009, at 5:39 AM, jdcasey@apache.org wrote:
> Author: jdcasey
> Date: Thu Feb 19 18:39:09 2009
> New Revision: 745955
>
> URL: http://svn.apache.org/viewvc?rev=745955&view=rev
> Log:
> [MNG-3057] Interpolate versions in POMs before installing/deploying
> them. Leave other expressions alone.
>
[snip]
>
> Modified: maven/components/branches/maven-2.1.x/maven-artifact-
> manager/src/main/java/org/apache/maven/artifact/installer/
> DefaultArtifactInstaller.java
> URL: http://svn.apache.org/viewvc/maven/components/branches/maven-2.1.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java?rev=745955&r1=745954&r2=745955&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- maven/components/branches/maven-2.1.x/maven-artifact-manager/src/
> main/java/org/apache/maven/artifact/installer/
> DefaultArtifactInstaller.java (original)
> +++ maven/components/branches/maven-2.1.x/maven-artifact-manager/src/
> main/java/org/apache/maven/artifact/installer/
> DefaultArtifactInstaller.java Thu Feb 19 18:39:09 2009
> @@ -55,9 +55,36 @@
> public void install( File source, Artifact artifact,
> ArtifactRepository localRepository )
> throws ArtifactInstallationException
> {
> +
> + // If we're installing the POM, we need to transform it
> first. The source file supplied for
> + // installation here may be the POM, but that POM may not
> be set as the file of the supplied
> + // artifact. Since the transformation only has access to
> the artifact and not the supplied
> + // source file, we have to use the Artifact.setFile(..) and
> Artifact.getFile(..) methods
> + // to shunt the POM file into the transformation process.
> + // Here, we also set a flag indicating that the POM has
> been shunted through the Artifact,
> + // and to expect the transformed version to be available in
> the Artifact afterwards...
> + boolean useArtifactFile = false;
> + if ( "pom".equals( artifact.getType() ) )
> + {
> + if ( artifact.getFile() == null )
> + {
> + artifact.setFile( source );
> + }
> +
> + useArtifactFile = true;
> + }
> +
This part of the change is tripping up projects that suffer from a
different bug I found: MCOMPILER-94. Take a look at the POM sample in
there - a project with something like that will succeed in 2.1.0-M1
and fail in 2.1.0-SNAPSHOT. I think you would need to remove the
artifact.getFile() == null check to avoid it.
Even though this is really a bug in the compiler plugin, do you think
we should preserve behavior for plugins that abuse setFile for a POM
by adding an integration test, or should we just mandate a compiler
plugin upgrade for anyone that trips this (and get a release out)?
Or given the rewriting problems are you reconsidering this fix for
2.1.0?
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
Re: svn commit: r745955
Posted by Brett Porter <br...@apache.org>.
Sorry John, I sent a response before I went to sleep but when I came
back found it hadn't gone through. The solution you put in place was
right, it needs to set it regardless. I see you updated the IT too.
Thanks!
- Brett
On 25/02/2009, at 1:22 AM, John Casey wrote:
> I'm still hopeful that I can get the rewriting problems fixed.
>
> I don't really understand the details of what you're saying here,
> though. If the artifact has no file, and it's packaging == "pom",
> why is it a bad idea to set the POM as the artifact's file?
>
> I'll read up on that MCOMPILER issue today, I don't have time right
> this moment, but if you could distill the issue WRT
> Artifact.setFile() it might save me an hour or so...
>
> -john
>
--
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: svn commit: r745955
Posted by John Casey <jd...@commonjava.org>.
I'm still hopeful that I can get the rewriting problems fixed.
I don't really understand the details of what you're saying here,
though. If the artifact has no file, and it's packaging == "pom", why is
it a bad idea to set the POM as the artifact's file?
I'll read up on that MCOMPILER issue today, I don't have time right this
moment, but if you could distill the issue WRT Artifact.setFile() it
might save me an hour or so...
-john
Brett Porter wrote:
> Hi John,
>
> I found another problem here:
>
> On 20/02/2009, at 5:39 AM, jdcasey@apache.org wrote:
>
>> Author: jdcasey
>> Date: Thu Feb 19 18:39:09 2009
>> New Revision: 745955
>>
>> URL: http://svn.apache.org/viewvc?rev=745955&view=rev
>> Log:
>> [MNG-3057] Interpolate versions in POMs before installing/deploying
>> them. Leave other expressions alone.
>>
> [snip]
>>
>> Modified:
>> maven/components/branches/maven-2.1.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java
>>
>> URL:
>> http://svn.apache.org/viewvc/maven/components/branches/maven-2.1.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java?rev=745955&r1=745954&r2=745955&view=diff
>>
>> ==============================================================================
>>
>> ---
>> maven/components/branches/maven-2.1.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java
>> (original)
>> +++
>> maven/components/branches/maven-2.1.x/maven-artifact-manager/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java
>> Thu Feb 19 18:39:09 2009
>> @@ -55,9 +55,36 @@
>> public void install( File source, Artifact artifact,
>> ArtifactRepository localRepository )
>> throws ArtifactInstallationException
>> {
>> +
>> + // If we're installing the POM, we need to transform it
>> first. The source file supplied for
>> + // installation here may be the POM, but that POM may not be
>> set as the file of the supplied
>> + // artifact. Since the transformation only has access to the
>> artifact and not the supplied
>> + // source file, we have to use the Artifact.setFile(..) and
>> Artifact.getFile(..) methods
>> + // to shunt the POM file into the transformation process.
>> + // Here, we also set a flag indicating that the POM has been
>> shunted through the Artifact,
>> + // and to expect the transformed version to be available in
>> the Artifact afterwards...
>> + boolean useArtifactFile = false;
>> + if ( "pom".equals( artifact.getType() ) )
>> + {
>> + if ( artifact.getFile() == null )
>> + {
>> + artifact.setFile( source );
>> + }
>> +
>> + useArtifactFile = true;
>> + }
>> +
>
> This part of the change is tripping up projects that suffer from a
> different bug I found: MCOMPILER-94. Take a look at the POM sample in
> there - a project with something like that will succeed in 2.1.0-M1 and
> fail in 2.1.0-SNAPSHOT. I think you would need to remove the
> artifact.getFile() == null check to avoid it.
>
> Even though this is really a bug in the compiler plugin, do you think we
> should preserve behavior for plugins that abuse setFile for a POM by
> adding an integration test, or should we just mandate a compiler plugin
> upgrade for anyone that trips this (and get a release out)?
>
> Or given the rewriting problems are you reconsidering this fix for 2.1.0?
>
> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org