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" <[email protected]> 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:[email protected]] On Behalf Of Brett
> Porter
> > Sent: Tuesday, October 12, 2010 12:45 PM
> > To: [email protected]
> > 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, [email protected] 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
> > [email protected]
> > 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
> [email protected]
> http://brettporter.wordpress.com/
