Browse Source

Revert "Make depot_tools scripts return exit codes properly on all platforms."

This reverts commit 7c4f7ec40818ddd883aa0a3b132cea06af92a713.

Reason for revert: bugs reported by users (see comments in Gerrit).
error: block. was unexpected at this time.

Original change's description:
> Make depot_tools scripts return exit codes properly on all platforms.
>
> Changes:
> 1. Windows: exit /b %errorlevel% should be used instead of goto :EOF to get valid exit codes during cmd /c <script>.bat invocation.
> 2. Windows: delayed var expansion is required in update_depot_tools.bat exit code generation.
> 3. Posix: update_depot_tools returns exit code from update_git_repo function in case of a failure.
>
> A rule of thumb on Windows: goto :EOF should not be used if %errorlevel% must be returned for all possible invocations.
>
> Test case for update_depot_tools changes:
> 1. Make a change to depot_tools sources that will conflict with next depot_tools update
> 2. Run update_depot_tools either directly or via gclient
> 3. Expect a git error is triggered
> 4. Inspect %errorlevel% or $? depending on platform
> 5. Expected 1, but the actual result is 0.
>
> Test case for changes in other .bat files:
> 1. Make a change to depot_tools sources that will conflict with next depot_tools update
> 2. Run cmd /c gclient
> 3. Expect a git error is triggered
> 4. Inspect %errorlevel%
> 5. Expected 1, but the actual result is 0.
>
> Change-Id: I64459982bcd9cc3db1319a9b39224b7a7af8c5aa
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3688632
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>
> Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
> Reviewed-by: Bruce Dawson <brucedawson@chromium.org>

Change-Id: I85d598af01d75588cdee77165d6af22270ee031d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3694139
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Josip Sokcevic 3 years ago
parent
commit
6754c49e02
5 changed files with 12 additions and 24 deletions
  1. 2 2
      fetch.bat
  2. 2 2
      gclient.bat
  3. 2 2
      gsutil.py.bat
  4. 0 4
      update_depot_tools
  5. 6 14
      update_depot_tools.bat

+ 2 - 2
fetch.bat

@@ -7,8 +7,8 @@ setlocal
 :: Synchronize the root directory before deferring control back to gclient.py.
 call "%~dp0\update_depot_tools.bat"
 :: Abort the script if we failed to update depot_tools.
-IF %ERRORLEVEL% NEQ 0 (
-  exit /b %ERRORLEVEL%
+IF %errorlevel% NEQ 0 (
+  goto :EOF
 )
 
 :: Ensure that "depot_tools" is somewhere in PATH so this tool can be used

+ 2 - 2
gclient.bat

@@ -10,8 +10,8 @@ IF "%DEPOT_TOOLS_UPDATE%" == "0" GOTO :CALL_GCLIENT
 :: Synchronize the root directory before deferring control back to gclient.py.
 call "%~dp0update_depot_tools.bat" %*
 :: Abort the script if we failed to update depot_tools.
-IF %ERRORLEVEL% NEQ 0 (
-  exit /b %ERRORLEVEL%
+IF %errorlevel% NEQ 0 (
+  goto :EOF
 )
 
 :CALL_GCLIENT

+ 2 - 2
gsutil.py.bat

@@ -10,8 +10,8 @@ IF "%DEPOT_TOOLS_UPDATE%" == "0" GOTO :CALL_GSUTIL
 :: Synchronize the root directory before deferring control back to gsutil.py.
 call "%~dp0update_depot_tools.bat" %*
 :: Abort the script if we failed to update depot_tools.
-IF %ERRORLEVEL% NEQ 0 (
-  exit /b %ERRORLEVEL%
+IF %errorlevel% NEQ 0 (
+  goto :EOF
 )
 
 :CALL_GSUTIL

+ 0 - 4
update_depot_tools

@@ -129,11 +129,7 @@ if [ "X$DEPOT_TOOLS_UPDATE" != "X0" ]; then
   if [ -e "$base_dir/.git" ]; then
     cd "$base_dir"
     update_git_repo
-    UPDATE_RESULT=$?
     cd - > /dev/null
-    if [[ $UPDATE_RESULT -ne 0 ]]; then
-      exit $UPDATE_RESULT
-    fi
   fi
 
   # Sync CIPD-boostrapped packages.

+ 6 - 14
update_depot_tools.bat

@@ -5,24 +5,17 @@
 
 :: This batch file will try to sync the root directory.
 
-setlocal enabledelayedexpansion
+setlocal
 
 :: Windows freaks out if a file is overwritten while it's being executed.  Copy
 :: this script off to a temporary location and reinvoke from there before
 :: running any git commands.
 IF "%~nx0"=="update_depot_tools.bat" (
   COPY /Y "%~dp0update_depot_tools.bat" "%TEMP%\update_depot_tools_tmp.bat" >nul
-  if errorlevel 1 (
-    echo Error updating depot_tools, can't copy update_depot_tools.bat to TEMP.
-    :: !VAR! syntax is used to get delayed expansion, because %VAR% syntax would
-    :: return a value that was set prior entering the IF () block.
-    exit /b !ERRORLEVEL!
-  )
+  if errorlevel 1 goto :EOF
   :: Use call/exit to avoid leaving an orphaned window title.
   call "%TEMP%\update_depot_tools_tmp.bat" "%~dp0" %*
-  :: !VAR! syntax is used to get delayed expansion, because %VAR% syntax would
-  :: return a value that was set prior entering the IF () block.
-  exit /b !ERRORLEVEL!
+  exit /b %ERRORLEVEL%
 )
 
 set DEPOT_TOOLS_DIR=%~1
@@ -32,16 +25,15 @@ SHIFT
 IF EXIST "%DEPOT_TOOLS_DIR%.disable_auto_update" GOTO :EOF
 IF "%DEPOT_TOOLS_UPDATE%" == "0" GOTO :EOF
 
-echo Updating depot_tools...
 set GIT_URL=https://chromium.googlesource.com/chromium/tools/depot_tools.git
 
 :: Download git for the first time if it's not present.
 call git --version > nul 2>&1
-if %ERRORLEVEL% == 0 goto :GIT_UPDATE
+if %errorlevel% == 0 goto :GIT_UPDATE
 call "%DEPOT_TOOLS_DIR%bootstrap\win_tools.bat"
 if errorlevel 1 (
   echo Error updating depot_tools, no revision tool found.
-  exit /b %ERRORLEVEL%
+  goto :EOF
 )
 
 :GIT_UPDATE
@@ -68,7 +60,7 @@ call git fetch -q origin > NUL
 call git checkout -q origin/main > NUL
 if errorlevel 1 (
   echo Failed to update depot_tools.
-  exit /b %ERRORLEVEL%
+  goto :EOF
 )
 
 :: Sync CIPD and CIPD client tools.