You are viewing a plain text version of this content. The canonical link for it is here.
Posted to npanday-dev@incubator.apache.org by Brett Porter <br...@apache.org> on 2010/10/12 08:45:26 UTC
Re: svn commit: r1021289 - in /incubator/npanday/branches/npanday-vs2010-support: components/dotnet-artifact/src/main/resources/META-INF/plexus/ components/dotnet-core/src/main/java/npanday/ components/dotnet-executable/src/main/java/npanday/executable/com...
On 11/10/2010, at 8:29 PM, apadilla@apache.org wrote:
> Author: apadilla
> Date: Mon Oct 11 09:29:12 2010
> New Revision: 1021289
>
> URL: http://svn.apache.org/viewvc?rev=1021289&view=rev
> Log:
> [NPANDAY-288] - import and compile simple VS 2010 projects
>
>
...
> Modified: incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java?rev=1021289&r1=1021288&r2=1021289&view=diff
> ==============================================================================
> --- incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java (original)
> +++ incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java Mon Oct 11 09:29:12 2010
> @@ -98,7 +98,8 @@ public class DefaultRepositoryNetExecuta
> ( ( getExecutionPath() != null ) ? getExecutionPath().getAbsolutePath() : "unknown" ) + ", Command = " +
> commands, e );
> }
> - if ( commandExecutor.getStandardOut().contains( "error" ) )
> + if ( commandExecutor.getStandardOut().contains( "error" )
> + && !commandExecutor.getStandardOut().contains( "exit code = 0" ) )
> {
> throw new ExecutionException(
> "NPANDAY-063-001: Executable = " + getExecutable() + ",Command = " + commands );
>
What was this change for?
I guess there were some commands that said "error", even though successful - but are we sure they'll output that text?
Also, can you add any integration tests for all of these changes?
- Brett
--
Brett Porter
brett@apache.org
http://brettporter.wordpress.com/
Re: svn commit: r1021289 - in
/incubator/npanday/branches/npanday-vs2010-support:
components/dotnet-artifact/src/main/resources/META-INF/plexus/
components/dotnet-core/src/main/java/npanday/
components/dotnet-executable/src/main/java/npanday/executable...
Posted by Adelita Padilla <ap...@g2ix.net>.
Hi,
Sorry for the late reply.
I also agree that checking on "error" is not really a good idea. My apologies that I've already committed it. I'd suggest we check the exit code instead.
So for the changes made in :
> incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java (r1023237)
Here's my proposed checking:
if (commandExecutor.getResult() != 0)
{
throw new ExecutionException(
"NPANDAY-063-001: Executable = " + getExecutable() + ",Command = " + commands );
}
Any thoughts?
Thanks,
liit
----- "Brett Porter" <br...@apache.org> wrote:
> On 12/10/2010, at 8:23 PM, Dmitry Lanin wrote:
>
> > There is an example of such command - msbuild :)
> > And
> trunk\plugins\netplugins\NPanday.Plugin.Msbuild\src\main\csharp\NPanday\Plugin\Msbuild\MsbuildMojo.cs
> file contains the following:
> > // must use /v:q here, as /v:m and above report the csc command,
> that includes '/errorprompt', which
> > // erroneously triggers the NPANDAY-063-001 error
> >
> > Also checking command output for "error" substring should be
> avoided, command's exit code should be checked instead.
>
> Yep, exactly. I added the above change to avoid this check, trying to
> keep the impact minimal at the time since I wasn't sure what might be
> relying on the string check. We shouldn't add another string check in
> there - I'd prefer to remove the "error" check entirely, if we can be
> sure that nothing was relying on it.
>
> - Brett
>
> >
> > Regards,
> > Dmitry
> >
> > -----Original Message-----
> > From: Brett Porter [mailto:brett@porterclan.net] On Behalf Of Brett
> Porter
> > Sent: Tuesday, October 12, 2010 12:45 PM
> > To: npanday-dev@incubator.apache.org
> > Subject: Re: svn commit: r1021289 - in
> /incubator/npanday/branches/npanday-vs2010-support:
> components/dotnet-artifact/src/main/resources/META-INF/plexus/
> components/dotnet-core/src/main/java/npanday/
> components/dotnet-executable/src/main/java/npanday/executable...
> >
> >
> > On 11/10/2010, at 8:29 PM, apadilla@apache.org wrote:
> >
> >> Author: apadilla
> >> Date: Mon Oct 11 09:29:12 2010
> >> New Revision: 1021289
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1021289&view=rev
> >> Log:
> >> [NPANDAY-288] - import and compile simple VS 2010 projects
> >>
> >>
> > ...
> >> Modified:
> incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java
> >> URL:
> http://svn.apache.org/viewvc/incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java?rev=1021289&r1=1021288&r2=1021289&view=diff
> >>
> ==============================================================================
> >> ---
> incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java
> (original)
> >> +++
> incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java
> Mon Oct 11 09:29:12 2010
> >> @@ -98,7 +98,8 @@ public class DefaultRepositoryNetExecuta
> >> ( ( getExecutionPath() != null ) ?
> getExecutionPath().getAbsolutePath() : "unknown" ) + ", Command = " +
> >> commands, e );
> >> }
> >> - if ( commandExecutor.getStandardOut().contains( "error" )
> )
> >> + if ( commandExecutor.getStandardOut().contains( "error" )
> >> + && !commandExecutor.getStandardOut().contains( "exit
> code = 0" ) )
> >> {
> >> throw new ExecutionException(
> >> "NPANDAY-063-001: Executable = " + getExecutable() +
> ",Command = " + commands );
> >>
> >
> > What was this change for?
> >
> > I guess there were some commands that said "error", even though
> successful - but are we sure they'll output that text?
> >
> > Also, can you add any integration tests for all of these changes?
> >
> > - Brett
> >
> > --
> > Brett Porter
> > brett@apache.org
> > http://brettporter.wordpress.com/
> >
> >
> >
> >
> >
> > This email and any files transmitted with it are confidential and
> intended solely for the use of the individual or entity to whom they
> are addressed. Please note that any disclosure, copying or
> distribution of the content of this information is strictly forbidden.
> If you have received this email message in error please notify its
> sender and then delete it from your files.
>
> --
> Brett Porter
> brett@apache.org
> http://brettporter.wordpress.com/
Re: svn commit: r1021289 - in /incubator/npanday/branches/npanday-vs2010-support: components/dotnet-artifact/src/main/resources/META-INF/plexus/ components/dotnet-core/src/main/java/npanday/ components/dotnet-executable/src/main/java/npanday/executable...
Posted by Brett Porter <br...@apache.org>.
On 12/10/2010, at 8:23 PM, Dmitry Lanin wrote:
> There is an example of such command - msbuild :)
> And trunk\plugins\netplugins\NPanday.Plugin.Msbuild\src\main\csharp\NPanday\Plugin\Msbuild\MsbuildMojo.cs file contains the following:
> // must use /v:q here, as /v:m and above report the csc command, that includes '/errorprompt', which
> // erroneously triggers the NPANDAY-063-001 error
>
> Also checking command output for "error" substring should be avoided, command's exit code should be checked instead.
Yep, exactly. I added the above change to avoid this check, trying to keep the impact minimal at the time since I wasn't sure what might be relying on the string check. We shouldn't add another string check in there - I'd prefer to remove the "error" check entirely, if we can be sure that nothing was relying on it.
- Brett
>
> Regards,
> Dmitry
>
> -----Original Message-----
> From: Brett Porter [mailto:brett@porterclan.net] On Behalf Of Brett Porter
> Sent: Tuesday, October 12, 2010 12:45 PM
> To: npanday-dev@incubator.apache.org
> Subject: Re: svn commit: r1021289 - in /incubator/npanday/branches/npanday-vs2010-support: components/dotnet-artifact/src/main/resources/META-INF/plexus/ components/dotnet-core/src/main/java/npanday/ components/dotnet-executable/src/main/java/npanday/executable...
>
>
> On 11/10/2010, at 8:29 PM, apadilla@apache.org wrote:
>
>> Author: apadilla
>> Date: Mon Oct 11 09:29:12 2010
>> New Revision: 1021289
>>
>> URL: http://svn.apache.org/viewvc?rev=1021289&view=rev
>> Log:
>> [NPANDAY-288] - import and compile simple VS 2010 projects
>>
>>
> ...
>> Modified: incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java
>> URL: http://svn.apache.org/viewvc/incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java?rev=1021289&r1=1021288&r2=1021289&view=diff
>> ==============================================================================
>> --- incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java (original)
>> +++ incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java Mon Oct 11 09:29:12 2010
>> @@ -98,7 +98,8 @@ public class DefaultRepositoryNetExecuta
>> ( ( getExecutionPath() != null ) ? getExecutionPath().getAbsolutePath() : "unknown" ) + ", Command = " +
>> commands, e );
>> }
>> - if ( commandExecutor.getStandardOut().contains( "error" ) )
>> + if ( commandExecutor.getStandardOut().contains( "error" )
>> + && !commandExecutor.getStandardOut().contains( "exit code = 0" ) )
>> {
>> throw new ExecutionException(
>> "NPANDAY-063-001: Executable = " + getExecutable() + ",Command = " + commands );
>>
>
> What was this change for?
>
> I guess there were some commands that said "error", even though successful - but are we sure they'll output that text?
>
> Also, can you add any integration tests for all of these changes?
>
> - Brett
>
> --
> Brett Porter
> brett@apache.org
> http://brettporter.wordpress.com/
>
>
>
>
>
> This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. Please note that any disclosure, copying or distribution of the content of this information is strictly forbidden. If you have received this email message in error please notify its sender and then delete it from your files.
--
Brett Porter
brett@apache.org
http://brettporter.wordpress.com/
RE: svn commit: r1021289 - in
/incubator/npanday/branches/npanday-vs2010-support:
components/dotnet-artifact/src/main/resources/META-INF/plexus/
components/dotnet-core/src/main/java/npanday/
components/dotnet-executable/src/main/java/npanday/executable...
Posted by Dmitry Lanin <Dm...@dbMotion.com>.
There is an example of such command - msbuild :)
And trunk\plugins\netplugins\NPanday.Plugin.Msbuild\src\main\csharp\NPanday\Plugin\Msbuild\MsbuildMojo.cs file contains the following:
// must use /v:q here, as /v:m and above report the csc command, that includes '/errorprompt', which
// erroneously triggers the NPANDAY-063-001 error
Also checking command output for "error" substring should be avoided, command's exit code should be checked instead.
Regards,
Dmitry
-----Original Message-----
From: Brett Porter [mailto:brett@porterclan.net] On Behalf Of Brett Porter
Sent: Tuesday, October 12, 2010 12:45 PM
To: npanday-dev@incubator.apache.org
Subject: Re: svn commit: r1021289 - in /incubator/npanday/branches/npanday-vs2010-support: components/dotnet-artifact/src/main/resources/META-INF/plexus/ components/dotnet-core/src/main/java/npanday/ components/dotnet-executable/src/main/java/npanday/executable...
On 11/10/2010, at 8:29 PM, apadilla@apache.org wrote:
> Author: apadilla
> Date: Mon Oct 11 09:29:12 2010
> New Revision: 1021289
>
> URL: http://svn.apache.org/viewvc?rev=1021289&view=rev
> Log:
> [NPANDAY-288] - import and compile simple VS 2010 projects
>
>
...
> Modified: incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java?rev=1021289&r1=1021288&r2=1021289&view=diff
> ==============================================================================
> --- incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java (original)
> +++ incubator/npanday/branches/npanday-vs2010-support/components/dotnet-executable/src/main/java/npanday/executable/impl/DefaultRepositoryNetExecutable.java Mon Oct 11 09:29:12 2010
> @@ -98,7 +98,8 @@ public class DefaultRepositoryNetExecuta
> ( ( getExecutionPath() != null ) ? getExecutionPath().getAbsolutePath() : "unknown" ) + ", Command = " +
> commands, e );
> }
> - if ( commandExecutor.getStandardOut().contains( "error" ) )
> + if ( commandExecutor.getStandardOut().contains( "error" )
> + && !commandExecutor.getStandardOut().contains( "exit code = 0" ) )
> {
> throw new ExecutionException(
> "NPANDAY-063-001: Executable = " + getExecutable() + ",Command = " + commands );
>
What was this change for?
I guess there were some commands that said "error", even though successful - but are we sure they'll output that text?
Also, can you add any integration tests for all of these changes?
- Brett
--
Brett Porter
brett@apache.org
http://brettporter.wordpress.com/
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. Please note that any disclosure, copying or distribution of the content of this information is strictly forbidden. If you have received this email message in error please notify its sender and then delete it from your files.