Maven 2 & 3

mvn.bat always exits 0 on Windows 2000 and higher

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Won't Fix
  • Affects Version/s: 2.0, 2.0.1, 2.0.2
  • Fix Version/s: None
  • Component/s: Command Line
  • Labels:
    None
  • Environment:
    I'm on Windows 2003 Server, but this will affect any OS for which the %OS% environment variable is Windows_NT, including Windows XP and Windows 2000.
  • Complexity:
    Intermediate
  • Testcase included:
    yes
  • Number of attachments :
    3

Description

Write the following ant script and run it on Windows 2000 or higher: <project default="main"><target name="main"><exec executable="mvn.bat" failonerror="true" /></target></project>

This will run "mvn" with no arguments, which will always fail. But the ant script will claim "build successful", because the exit value of mvn.bat was 0. It is absolutely critical that this work correctly, or else I can't integrate Maven into any other automated system.

This is happening because mvn.bat is improperly abusing local scoping. On line 130 of mvn.bat, we execute maven, but we don't do anything with its exit value... we just always goto end. The fix for this is to add a line 131 that says "if errorlevel 1 goto error", which will behave correctly.

(I marked this as having a test case because I've included a test ant script, but technically this isn't a JUnit test case, so it may be an inappropriate use of the "testcase included" marker.)

  1. maven-task.xml
    10/Mar/06 6:44 AM
    5 kB
    Steve Loughran
  2. mvnfixed.bat
    07/Mar/06 8:00 PM
    5 kB
    Dan Fabulich
  3. mvnfixed.bat
    07/Mar/06 5:25 PM
    5 kB
    Dan Fabulich

Issue Links

Activity

Hide
Emmanuel Venisse added a comment -

You must set MAVEN_TERMINATE_CMD env var to 'on' :

<project default="main">
<target name="main">
<exec executable="mvn.bat" failonerror="true">
<env key="MAVEN_TERMINATE_CMD" value="on" />
</exec>
</target>
</project>

Show
Emmanuel Venisse added a comment - You must set MAVEN_TERMINATE_CMD env var to 'on' : <project default="main"> <target name="main"> <exec executable="mvn.bat" failonerror="true"> <env key="MAVEN_TERMINATE_CMD" value="on" /> </exec> </target> </project>
Hide
Wayne Fay added a comment -

Any particular reason this isn't ON by default?

It makes no sense to me that this needs to be turned ON when it seems like normal behavoir for commandline utilities in any Operating system. But there is probably a good reason why this is off for Windows, and I just don't know enough to realize why...

Show
Wayne Fay added a comment - Any particular reason this isn't ON by default? It makes no sense to me that this needs to be turned ON when it seems like normal behavoir for commandline utilities in any Operating system. But there is probably a good reason why this is off for Windows, and I just don't know enough to realize why...
Hide
Emmanuel Venisse added a comment -

We can't use on by default because when you activate this env var with 'on', mvn exit with /B so the command line is closed, and if you use it in a dos environment, your window would be closed too.

Ypu can try it like this :
SET MAVEN_TERMINATE_CMD=on
mvn.bat

Show
Emmanuel Venisse added a comment - We can't use on by default because when you activate this env var with 'on', mvn exit with /B so the command line is closed, and if you use it in a dos environment, your window would be closed too. Ypu can try it like this : SET MAVEN_TERMINATE_CMD=on mvn.bat
Hide
Wayne Fay added a comment -

Like I said... "I just don't know enough to realize why"

Thanks Emmanuel

Show
Wayne Fay added a comment - Like I said... "I just don't know enough to realize why" Thanks Emmanuel
Hide
Dan Fabulich added a comment -

This undocumented environment variable is totally unnecessary in a batch script... we shouldn't be using exit /B like that. All that's needed is the one-line fix I proposed in the bug description. I've filed an improvement MNG-2132 to highlight this.

Show
Dan Fabulich added a comment - This undocumented environment variable is totally unnecessary in a batch script... we shouldn't be using exit /B like that. All that's needed is the one-line fix I proposed in the bug description. I've filed an improvement MNG-2132 to highlight this.
Hide
Brett Porter added a comment -

will investigate

Show
Brett Porter added a comment - will investigate
Hide
Dan Fabulich added a comment -

Thanks for re-opening this. I've just discovered that Emmanuel's workaround doesn't work either, because that makes mvn.bat always exit with error code 1, even if the build succeeds!

Also note that my one-line fix needs to be added to all of the other validation points in this file, including the JAVA_HOME and M2_HOME verification checks. I'm attaching a new batch file to this issue which I believe fixes the problem.

Show
Dan Fabulich added a comment - Thanks for re-opening this. I've just discovered that Emmanuel's workaround doesn't work either, because that makes mvn.bat always exit with error code 1, even if the build succeeds! Also note that my one-line fix needs to be added to all of the other validation points in this file, including the JAVA_HOME and M2_HOME verification checks. I'm attaching a new batch file to this issue which I believe fixes the problem.
Hide
Dan Fabulich added a comment -

I believe this fixes the issue.

Show
Dan Fabulich added a comment - I believe this fixes the issue.
Hide
Dan Fabulich added a comment -

Oops, sorry... it's not that it always fails with Emmanuel's workaround; the problem with mvn.bat right now is that if it ever fails, it sets the ERROR_CODE=1, (not the DOS official env var %ERRORLEVEL%, but an arbitrary env var) which means that the script will always fail from then on.

I'm updating my mvnfixed.bat file with enhancements to make this go away. I've tested this on Windows 2003 and Windows NT, but not on Win9x. It works great everywhere I've tried it. Note that I've added this sneaky line to set the Windows errorlevel: "echo 1 | choice /c:12 /n >nul >2&1". I got this from here: http://www.robvanderwoude.com/index.html

He claims this works on Win9x, but I don't have Win9x available.

Show
Dan Fabulich added a comment - Oops, sorry... it's not that it always fails with Emmanuel's workaround; the problem with mvn.bat right now is that if it ever fails, it sets the ERROR_CODE=1, (not the DOS official env var %ERRORLEVEL%, but an arbitrary env var) which means that the script will always fail from then on. I'm updating my mvnfixed.bat file with enhancements to make this go away. I've tested this on Windows 2003 and Windows NT, but not on Win9x. It works great everywhere I've tried it. Note that I've added this sneaky line to set the Windows errorlevel: "echo 1 | choice /c:12 /n >nul >2&1". I got this from here: http://www.robvanderwoude.com/index.html He claims this works on Win9x, but I don't have Win9x available.
Hide
Emmanuel Venisse added a comment -

choice command is available only with ms resource kit

Show
Emmanuel Venisse added a comment - choice command is available only with ms resource kit
Hide
Brett Porter added a comment -

this is a misunderstanding. I'm assuming you tested your fix by checking if errorlevel 1 echo something

the missing if errorlevel 1 was already added to svn, and I've fixed the location of the original setting of ERROR_CODE. Not using ERROR_CODE still works, setting the errorlevel correctly, but should the script change to have an intervening command that resets it this would be lost so it is good to keep, plus it allows returning "1" for the failure to set JAVA_HOME, etc.

Emmanuel was entirely correct, and you'll find running the new script inside Ant still succeeds, as does running Ant inside Ant. The reason for this is that batch files are called like this:
cmd /c mvn.bat

The exit code of mvn.bat is swallowed by cmd.exe. However, if you use the "exit" command, cmd.exe is terminated with the correct exit code. There doesn't appear to be any alternative to this at this point. I'll keep looking.

I test this on both XP SP2 and XP SP1a. We don't support Win9x.

Show
Brett Porter added a comment - this is a misunderstanding. I'm assuming you tested your fix by checking if errorlevel 1 echo something the missing if errorlevel 1 was already added to svn, and I've fixed the location of the original setting of ERROR_CODE. Not using ERROR_CODE still works, setting the errorlevel correctly, but should the script change to have an intervening command that resets it this would be lost so it is good to keep, plus it allows returning "1" for the failure to set JAVA_HOME, etc. Emmanuel was entirely correct, and you'll find running the new script inside Ant still succeeds, as does running Ant inside Ant. The reason for this is that batch files are called like this: cmd /c mvn.bat The exit code of mvn.bat is swallowed by cmd.exe. However, if you use the "exit" command, cmd.exe is terminated with the correct exit code. There doesn't appear to be any alternative to this at this point. I'll keep looking. I test this on both XP SP2 and XP SP1a. We don't support Win9x.
Hide
Brett Porter added a comment -

Ant should be using:

cmd /C call mvn.bat

instead, though it doesn't know that it's a batch file, I assume.

Emmanuel, can we do this in Continuum to ensure the correct result?

Show
Brett Porter added a comment - Ant should be using: cmd /C call mvn.bat instead, though it doesn't know that it's a batch file, I assume. Emmanuel, can we do this in Continuum to ensure the correct result?
Hide
Brett Porter added a comment -

It looks like we could add that to setDefaultShell() in plexus utils for continuum to fix it permanently.

Show
Brett Porter added a comment - It looks like we could add that to setDefaultShell() in plexus utils for continuum to fix it permanently.
Hide
Emmanuel Venisse added a comment -

ant doesn't use cmd /C call ...

it use cmd /C cd DIR && command for Windows XP/2000/NT
and it use an external launcher (antrun.bat for windows 95/98

http://svn.apache.org/repos/asf/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/Execute.java

I'm not sure cmd /C call ... will return the correct result and in continuum we always get the correct result.

Show
Emmanuel Venisse added a comment - ant doesn't use cmd /C call ... it use cmd /C cd DIR && command for Windows XP/2000/NT and it use an external launcher (antrun.bat for windows 95/98 http://svn.apache.org/repos/asf/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/Execute.java I'm not sure cmd /C call ... will return the correct result and in continuum we always get the correct result.
Hide
Brett Porter added a comment -

I know ant doesn't, and I'm saying ant doesn't work here either. cmd /C still swallows the result.

Continuum works by setting the TERMINATE_CMD, without requiring any user intervention? What about if it calls m1, or ant?

I think cmd /c call is cleaner if that works...

Show
Brett Porter added a comment - I know ant doesn't, and I'm saying ant doesn't work here either. cmd /C still swallows the result. Continuum works by setting the TERMINATE_CMD, without requiring any user intervention? What about if it calls m1, or ant? I think cmd /c call is cleaner if that works...
Hide
Emmanuel Venisse added a comment -

ok, i'll look at it.

Actually, Continuum works by setting the MAVEN_TERMINATE_CMD, without requiring any user intervention. For ant, we have a modified script on continuum site. m1 script use MAVEN_TERMINATE_CMD

Show
Emmanuel Venisse added a comment - ok, i'll look at it. Actually, Continuum works by setting the MAVEN_TERMINATE_CMD, without requiring any user intervention. For ant, we have a modified script on continuum site. m1 script use MAVEN_TERMINATE_CMD
Hide
Dan Fabulich added a comment -

Wow... I'm definitely surprised to see that ant.bat suffers from this same problem. Let that teach me a valuable lesson.

I'm definitely surprised to hear that we don't support Win9x, because there's a bunch of code in this batch script designed to handle Win9x, including a batch label called :Win9xApp, which only triggers when %OS% is not equal to Windows_NT, which is always true on Windows_NT, Windows 2000, Windows XP, and Windows 2003. Is any of that code necessary?

Anyway, using the current latest mvn.bat from SVN (384215) helps quite a bit.
http://svn.apache.org/viewcvs.cgi/maven/components/trunk/maven-core/src/bin/mvn.bat?rev=384215&view=markup

With that said, on line 154, it says "exit /b %ERROR_CODE%". This breaks on Windows NT... the batch script always returns 1, because that's an invalid command on NT. Does Maven support Windows NT?

Interestingly, since the latest SVN mvn.bat uses exit /b, but includes the "if errorlevel 1 goto error" fix, it works properly in Ant on Windows 2003 even without the MAVEN_TERMINATE_CMD environment variable. Here's my test, which works on Windows 2003, but not on Windows XP:

<property name="maven.executable" location="mvnfixed.bat" />
<exec executable="${maven.executable}" failonerror="false" resultproperty="x">
</exec>
<echo message="x = ${x}" />
<condition property="should.have.failed">
<equals arg1="0" arg2="${x}" />
</condition>
<fail if="should.have.failed" message="should have failed"/>
<exec executable="${maven.executable}" failonerror="true">
<arg value="--help" />
</exec>

Anyway, I think I can live with this.

Show
Dan Fabulich added a comment - Wow... I'm definitely surprised to see that ant.bat suffers from this same problem. Let that teach me a valuable lesson. I'm definitely surprised to hear that we don't support Win9x, because there's a bunch of code in this batch script designed to handle Win9x, including a batch label called :Win9xApp, which only triggers when %OS% is not equal to Windows_NT, which is always true on Windows_NT, Windows 2000, Windows XP, and Windows 2003. Is any of that code necessary? Anyway, using the current latest mvn.bat from SVN (384215) helps quite a bit. http://svn.apache.org/viewcvs.cgi/maven/components/trunk/maven-core/src/bin/mvn.bat?rev=384215&view=markup With that said, on line 154, it says "exit /b %ERROR_CODE%". This breaks on Windows NT... the batch script always returns 1, because that's an invalid command on NT. Does Maven support Windows NT? Interestingly, since the latest SVN mvn.bat uses exit /b, but includes the "if errorlevel 1 goto error" fix, it works properly in Ant on Windows 2003 even without the MAVEN_TERMINATE_CMD environment variable. Here's my test, which works on Windows 2003, but not on Windows XP: <property name="maven.executable" location="mvnfixed.bat" /> <exec executable="${maven.executable}" failonerror="false" resultproperty="x"> </exec> <echo message="x = ${x}" /> <condition property="should.have.failed"> <equals arg1="0" arg2="${x}" /> </condition> <fail if="should.have.failed" message="should have failed"/> <exec executable="${maven.executable}" failonerror="true"> <arg value="--help" /> </exec> Anyway, I think I can live with this.
Hide
Steve Loughran added a comment -

I believe Ant's return code on winnt systems was fixed as of last week. We really need to think of a good regression test for this, because ant.bat is the one of the two startup scripts that scare the team. the other is cygwin support in ant.sh. the perl and python startup scripts are fine, its just those two.

the underlying problem with ant.bat is trying to support win9x, which makes things much more contrived, and much more brittle -we have a vmware image of '9x around just to check things work.

as for last weeks patch, it turns out that if you unset an env variable that was never set, the error code was set. So to clear all env variables (remember, you can't use @setlocal on '9x), ant needs to set every transient env var to a value just before clearing, to be 100% sure that is was set by all paths to the exit logic.

If starting maven on ant is important (and clearly it is), I would recomend you look at the script and do a <java> task to set it up and call it. That or someone needs to write a proper <maven> task.

Show
Steve Loughran added a comment - I believe Ant's return code on winnt systems was fixed as of last week. We really need to think of a good regression test for this, because ant.bat is the one of the two startup scripts that scare the team. the other is cygwin support in ant.sh. the perl and python startup scripts are fine, its just those two. the underlying problem with ant.bat is trying to support win9x, which makes things much more contrived, and much more brittle -we have a vmware image of '9x around just to check things work. as for last weeks patch, it turns out that if you unset an env variable that was never set, the error code was set. So to clear all env variables (remember, you can't use @setlocal on '9x), ant needs to set every transient env var to a value just before clearing, to be 100% sure that is was set by all paths to the exit logic. If starting maven on ant is important (and clearly it is), I would recomend you look at the script and do a <java> task to set it up and call it. That or someone needs to write a proper <maven> task.
Hide
Dan Fabulich added a comment -

Good points, Steve. I've filed a wish MNG-2139 for a <maven> ant task to launch Maven.

When you say you believe Ant's return code on "winnt systems" was fixed, do you mean Windows NT, or everything NT-based (2003, 2000, XP, etc)? Can you provide a link/pointer to the revision in SCM?

Hopefully since Maven doesn't need 9x support it can at least make sure mvn.bat returns a reasonable result code on the supported operating systems.

Show
Dan Fabulich added a comment - Good points, Steve. I've filed a wish MNG-2139 for a <maven> ant task to launch Maven. When you say you believe Ant's return code on "winnt systems" was fixed, do you mean Windows NT, or everything NT-based (2003, 2000, XP, etc)? Can you provide a link/pointer to the revision in SCM? Hopefully since Maven doesn't need 9x support it can at least make sure mvn.bat returns a reasonable result code on the supported operating systems.
Hide
Brett Porter added a comment -

I guess we don't support Windows NT either

Perhaps we should remove the exit /B altogether and rely on errorlevel not getting murdered by carefully checking it also.

It's awfully tempting to write a little .exe that takes care of this.

Show
Brett Porter added a comment - I guess we don't support Windows NT either Perhaps we should remove the exit /B altogether and rely on errorlevel not getting murdered by carefully checking it also. It's awfully tempting to write a little .exe that takes care of this.
Hide
Dan Fabulich added a comment -

exit /B is the only thing making this work nicely on Windows 2003. I'd rather say "if errorlevel 1 exit /B >NUL 2>&1" which will work on Windows NT by sending an invalid syntax to exit, which is probably not the way you meant it to work, but I think it does work.

Show
Dan Fabulich added a comment - exit /B is the only thing making this work nicely on Windows 2003. I'd rather say "if errorlevel 1 exit /B >NUL 2>&1" which will work on Windows NT by sending an invalid syntax to exit, which is probably not the way you meant it to work, but I think it does work.
Hide
Steve Loughran added a comment -

Dan, the last fix was bugzilla ID 32069
http://issues.apache.org/bugzilla/show_bug.cgi?id=32069

and the SVN HEAD of ant.bat contains it, just after the :end statement
http://svn.apache.org/repos/asf/ant/core/trunk/src/script/ant.bat

As far as ant is concerned, winnt==anything running on the winnt kernel, from version 3.1 up. specifically,. anything whose %OS% env variable is set to winnt or Windows_NT ,the latter being what XP systems do.

The way the ant script sets the error code is to abuse the color command to try and set the fg and bg colours to match. From the documentation
"The COLOR command sets ERRORLEVEL to 1 if an attempt is made to execute
the COLOR command with a foreground and background color that are the
same."

so COLOR 00 sets ERROLEVEL to 1, which is what you want

Show
Steve Loughran added a comment - Dan, the last fix was bugzilla ID 32069 http://issues.apache.org/bugzilla/show_bug.cgi?id=32069 and the SVN HEAD of ant.bat contains it, just after the :end statement http://svn.apache.org/repos/asf/ant/core/trunk/src/script/ant.bat As far as ant is concerned, winnt==anything running on the winnt kernel, from version 3.1 up. specifically,. anything whose %OS% env variable is set to winnt or Windows_NT ,the latter being what XP systems do. The way the ant script sets the error code is to abuse the color command to try and set the fg and bg colours to match. From the documentation "The COLOR command sets ERRORLEVEL to 1 if an attempt is made to execute the COLOR command with a foreground and background color that are the same." so COLOR 00 sets ERROLEVEL to 1, which is what you want
Hide
Steve Loughran added a comment -

This is a build file to <import> into your project that gives you a <maven> task. Needs maven.home, env.MAVEN_HOME set, and possibly some help finding tools.jar

-steve

Show
Steve Loughran added a comment - This is a build file to <import> into your project that gives you a <maven> task. Needs maven.home, env.MAVEN_HOME set, and possibly some help finding tools.jar -steve
Hide
Brett Porter added a comment -

its also for Maven 1, so it won't work for m2.

Show
Brett Porter added a comment - its also for Maven 1, so it won't work for m2.
Hide
Martin Gilday added a comment -

What is the situation with this on WinXP SP2, with Maven 2.0.7? I have set MAVEN_TERMINATE_CMD=on and regardless of which command I run I am always getting %ERRORLEVEL% as 0.

Show
Martin Gilday added a comment - What is the situation with this on WinXP SP2, with Maven 2.0.7? I have set MAVEN_TERMINATE_CMD=on and regardless of which command I run I am always getting %ERRORLEVEL% as 0.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: