Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows bat exit code fix #423

Merged
merged 3 commits into from
Dec 7, 2014
Merged

windows bat exit code fix #423

merged 3 commits into from
Dec 7, 2014

Conversation

bjuric
Copy link
Contributor

@bjuric bjuric commented Dec 1, 2014

Return exit code 1 in windows bat script if java application exits with
a return code greater than 0. Update windows bat regression tests to
check the returned code.

Return exit code 1 in windows bat script if java application exits with
a return code greater than 0.  Update windows bat regression tests to
check the returned code.
@bjuric bjuric mentioned this pull request Dec 1, 2014
@bjuric
Copy link
Contributor Author

bjuric commented Dec 1, 2014

This includes tests for both returnCode == 0 and returnCode == 1. The existing test in src/sbt-test/windows/test-bat-template/src/main/scala/test/Test.scala was changed to support both exit codes and a checkOutputEnv(Map("return-code-1"->"true"), 1, "arg #0 is [RC1]\nSUCCESS!", "RC1") line was added to the bottom of checkOutput in src/sbt-test/windows/test-bat-template/build.sbt to test the negative case. All previously existing test cases check for return code 0.

This pull request targets 0.8.x.

Developed and tested on Windows 7 SP1.

@@ -12,5 +12,10 @@ object Test extends App {
}
}
println("SUCCESS!")
if(System.getenv("return-code-1") == "true"){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems weird to mix tabs and spaces. i know that's what the rest of the file is doing, but i think it'd be nice to replace all the tabs with spaces

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Maybe we target this in a following pr.

Change tabs to spaces and adjust indentats.
Print FAILURE for exit code 1 and SUCCESS for exit code 0 in test.
@muuki88
Copy link
Contributor

muuki88 commented Dec 2, 2014

@nazoking @jsuereth could one of you cross check this as well?

@jsuereth
Copy link
Member

jsuereth commented Dec 3, 2014

Fix looks good to me (visually). Will check on windows when I can.

@nazoking
Copy link
Contributor

nazoking commented Dec 5, 2014

oh. it is good.

if you change
https://github.com/sbt/sbt-native-packager/pull/423/files#diff-0893ecb3791881c87ff523facf8b1880L134

set ERROR_CODE=1

set ERROR_CODE=%ERRORLEVEL%

then batch error code is same java exit code.

so test is

diff --git a/src/sbt-test/windows/test-bat-template/build.sbt b/src/sbt-test/windows/test-bat-template/build.sbt
index e03b482..f5c643e 100644
--- a/src/sbt-test/windows/test-bat-template/build.sbt
+++ b/src/sbt-test/windows/test-bat-template/build.sbt
@@ -98,6 +98,7 @@ TaskKey[Unit]("check-script") <<= (stagingDirectory in Universal, name, streams)
   // include space and double-quote is failed...
   // can't success include double-quote. arguments pass from Process(Seq("-Da=xx\"yy", "aa\"bb")) is parsed (%1="-Da", %2="
   //checkOutput(0, "arg #0 is [xx\"yy]\nproperty(test.hoge) is [aa\"bb]\nvmarg #0 is [-Dtest.hoge=aa\"bb]\nSUCCESS!", "-Dte-  checkOutputEnv(Map("return-code-1"->"true"), 1, "arg #0 is [RC1]\nFAILURE!", "RC1")
+  checkOutputEnv(Map("return-code"->"1"), 1, "arg #0 is [RC1]\nFAILURE!", "RC1")
+  checkOutputEnv(Map("return-code"->"2"), 2, "arg #0 is [RC2]\nFAILURE!", "RC2")
   assert(fails.toString == "", fails.toString)
 }
diff --git a/src/sbt-test/windows/test-bat-template/src/main/scala/test/Test.scala b/src/sbt-test/windows/test-bat-template/index 8438d82..74d10d0 100644
--- a/src/sbt-test/windows/test-bat-template/src/main/scala/test/Test.scala
+++ b/src/sbt-test/windows/test-bat-template/src/main/scala/test/Test.scala
@@ -11,9 +11,9 @@ object Test extends App {
         println("vmarg #" + i + " is [" + x + "]")
       }
     }
-    if(System.getenv("return-code-1") == "true"){
+    if(System.getenv("return-code") != null){
       println("FAILURE!")
-      System.exit(1)
+      System.exit(System.getenv("return-code").toInt)
     } else {
       println("SUCCESS!")
       System.exit(0)

@nazoking
Copy link
Contributor

nazoking commented Dec 5, 2014

Sorry, The above post was wrong . That is not pass negative error code.

if ERRORLEVEL 1 goto error

if %ERRORLEVEL% neq 0 goto error

all is

diff --git a/src/main/resources/com/typesafe/sbt/packager/archetypes/bat-template b/src/main/resources/com/typesafe/sbt/packindex 1cbf0e5..5dab7d7 100644
--- a/src/main/resources/com/typesafe/sbt/packager/archetypes/bat-template
+++ b/src/main/resources/com/typesafe/sbt/packager/archetypes/bat-template
@@ -130,11 +130,11 @@ rem Call the application and pass all arguments unchanged.

 @endlocal

-if ERRORLEVEL 1 goto error
+if %ERRORLEVEL% neq 0 goto error
 goto end

 :error
-set ERROR_CODE=1
+set ERROR_CODE=%ERRORLEVEL%

 :end

diff --git a/src/sbt-test/windows/test-bat-template/build.sbt b/src/sbt-test/windows/test-bat-template/build.sbt
index e03b482..ce18a3f 100644
--- a/src/sbt-test/windows/test-bat-template/build.sbt
+++ b/src/sbt-test/windows/test-bat-template/build.sbt
@@ -98,6 +98,8 @@ TaskKey[Unit]("check-script") <<= (stagingDirectory in Universal, name, streams)
   // include space and double-quote is failed...
   // can't success include double-quote. arguments pass from Process(Seq("-Da=xx\"yy", "aa\"bb")) is parsed (%1="-Da", %2="
   //checkOutput(0, "arg #0 is [xx\"yy]\nproperty(test.hoge) is [aa\"bb]\nvmarg #0 is [-Dtest.hoge=aa\"bb]\nSUCCESS!", "-Dte-  checkOutputEnv(Map("return-code-1"->"true"), 1, "arg #0 is [RC1]\nFAILURE!", "RC1")
+  checkOutputEnv(Map("return-code"->"1"), 1, "arg #0 is [RC1]\nFAILURE!", "RC1")
+  checkOutputEnv(Map("return-code"->"2"), 2, "arg #0 is [RC2]\nFAILURE!", "RC2")
+  checkOutputEnv(Map("return-code"->"-1"), -1, "arg #0 is [RC-1]\nFAILURE!", "RC-1")
   assert(fails.toString == "", fails.toString)
 }
diff --git a/src/sbt-test/windows/test-bat-template/src/main/scala/test/Test.scala b/src/sbt-test/windows/test-bat-template/index 8438d82..74d10d0 100644
--- a/src/sbt-test/windows/test-bat-template/src/main/scala/test/Test.scala
+++ b/src/sbt-test/windows/test-bat-template/src/main/scala/test/Test.scala
@@ -11,9 +11,9 @@ object Test extends App {
         println("vmarg #" + i + " is [" + x + "]")
       }
     }
-    if(System.getenv("return-code-1") == "true"){
+    if(System.getenv("return-code") != null){
       println("FAILURE!")
-      System.exit(1)
+      System.exit(System.getenv("return-code").toInt)
     } else {
       println("SUCCESS!")
       System.exit(0)

@bjuric
Copy link
Contributor Author

bjuric commented Dec 5, 2014

If you like, I can push out another commit to this PR and make it so.
Or you can do it. Whichever is easier.

@bjuric
Copy link
Contributor Author

bjuric commented Dec 5, 2014

Perhaps too, we could simply exit with:

exit /B %ERRORLEVEL%

Then we can do away with the ERROR_CODE variable and goto error handling altogether.

@bjuric
Copy link
Contributor Author

bjuric commented Dec 7, 2014

Thanks @nazoking :) It's merged in.

@nazoking
Copy link
Contributor

nazoking commented Dec 7, 2014

Thanks @bjuric .

@muuki88 Test is success. Looks good to me.

@muuki88
Copy link
Contributor

muuki88 commented Dec 7, 2014

Thanks a lot for the great team work :)
I'll try to release this in a next RC/Milestone

muuki88 added a commit that referenced this pull request Dec 7, 2014
@muuki88 muuki88 merged commit b642574 into sbt:0.8.x Dec 7, 2014
@muuki88 muuki88 mentioned this pull request Dec 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants