You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/02/09 15:09:11 UTC

[GitHub] [iotdb] ericpai opened a new pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

ericpai opened a new pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026


   See JIRA: https://issues.apache.org/jira/browse/IOTDB-2523
   
   After this fix being applied, we got this result in my desktop with Windows 10 installed, which is as our expected.
   ![1644418965(1)](https://user-images.githubusercontent.com/3821212/153229204-470f0818-d400-43e8-8a82-990c73dc809f.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [iotdb] HTHou commented on a change in pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026#discussion_r803809423



##########
File path: cli/src/assembly/resources/sbin/start-cli.bat
##########
@@ -59,16 +59,19 @@ echo %PARAMETERS% | findstr /c:"-h ">nul && (set PARAMETERS=%PARAMETERS%) || (se
 echo %PARAMETERS%
 
 "%JAVA_HOME%\bin\java" %JAVA_OPTS% -cp %CLASSPATH% %MAIN_CLASS% %PARAMETERS%
-
+set ret_code=%ERRORLEVEL%
 goto finally
 
 
 :err
 echo JAVA_HOME environment variable must be set!
+set ret_code=1
 pause
 
 
 @REM -----------------------------------------------------------------------------
 :finally
 
-ENDLOCAL
\ No newline at end of file
+ENDLOCAL
+
+EXIT %ret_code%

Review comment:
       Hi, does `start-cli.sh` need to do such changes?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [iotdb] ericpai commented on pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

Posted by GitBox <gi...@apache.org>.
ericpai commented on pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026#issuecomment-1035007266


   @HTHou @cornmonster PTAL~


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [iotdb] coveralls edited a comment on pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026#issuecomment-1033946596


   
   [![Coverage Status](https://coveralls.io/builds/46414599/badge)](https://coveralls.io/builds/46414599)
   
   Coverage increased (+0.01%) to 67.735% when pulling **638056ee60212f8364c65b8fa757886cfdd729e5 on ericpai:bugfix/iotdb-2523** into **38eaf0ebd51b250ba6e66cb93c25680574d8c5dc on apache:master**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [iotdb] ericpai commented on a change in pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026#discussion_r804284610



##########
File path: cli/src/assembly/resources/sbin/start-cli.bat
##########
@@ -59,16 +59,19 @@ echo %PARAMETERS% | findstr /c:"-h ">nul && (set PARAMETERS=%PARAMETERS%) || (se
 echo %PARAMETERS%
 
 "%JAVA_HOME%\bin\java" %JAVA_OPTS% -cp %CLASSPATH% %MAIN_CLASS% %PARAMETERS%
-
+set ret_code=%ERRORLEVEL%
 goto finally
 
 
 :err
 echo JAVA_HOME environment variable must be set!
+set ret_code=1
 pause
 
 
 @REM -----------------------------------------------------------------------------
 :finally
 
-ENDLOCAL
\ No newline at end of file
+ENDLOCAL
+
+EXIT %ret_code%

Review comment:
       In `start-cli.sh`, it uses `exec` to yield the current process to `java`, which means that the exit code is just the same as the one of `java` process(the L83 `exit $?` is useless as it will be never executed). So there's no need to change the script in unix system.
   However, there's no any command like `exec` in Windows. The final exit code of origin `start-cli.bat` is the last command `ENDLOCAL`, which is always 0. So we need to save the code after the `java` command is finished and do an `EXIT $ret_code`.
   The above conclusions can be proved by the test cases modified in this PR.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [iotdb] ericpai merged pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

Posted by GitBox <gi...@apache.org>.
ericpai merged pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [iotdb] ericpai commented on a change in pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026#discussion_r804284610



##########
File path: cli/src/assembly/resources/sbin/start-cli.bat
##########
@@ -59,16 +59,19 @@ echo %PARAMETERS% | findstr /c:"-h ">nul && (set PARAMETERS=%PARAMETERS%) || (se
 echo %PARAMETERS%
 
 "%JAVA_HOME%\bin\java" %JAVA_OPTS% -cp %CLASSPATH% %MAIN_CLASS% %PARAMETERS%
-
+set ret_code=%ERRORLEVEL%
 goto finally
 
 
 :err
 echo JAVA_HOME environment variable must be set!
+set ret_code=1
 pause
 
 
 @REM -----------------------------------------------------------------------------
 :finally
 
-ENDLOCAL
\ No newline at end of file
+ENDLOCAL
+
+EXIT %ret_code%

Review comment:
       In `start-cli.sh`, it uses `exec` to yield the current process to `java`, which means that the exit code is just the same as the one of `java` process(the L83 `exit $?` is useless as it will be never executed). So there's no need to change the script in unix system.
   However, there's no any command like `exec` in Windows. The final exist code of origin `start-cli.bat` is the last command `ENDLOCAL`, which is always 0. So we need to save the code after the `java` command is finished and do an `EXIT $ret_code`.
   The above conclusions can be proved by the test cases modified in this PR.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [iotdb] coveralls commented on pull request #5026: [IOTDB-2523] Fix exiting with 0 when executing error in cli

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #5026:
URL: https://github.com/apache/iotdb/pull/5026#issuecomment-1033946596


   
   [![Coverage Status](https://coveralls.io/builds/46382056/badge)](https://coveralls.io/builds/46382056)
   
   Coverage decreased (-0.008%) to 67.722% when pulling **15d9a90c2c5c6ab47ae57918cc1ffecf43f65442 on ericpai:bugfix/iotdb-2523** into **bc7cd107bca61b480a50f2e9cce6e531c3d19e37 on apache:master**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org