From 544594e0b2760e2ded229d6b06782f01d82b0a6c Mon Sep 17 00:00:00 2001
From: Aleksey Khoroshilov <akhoroshilov@brave.com>
Date: Mon, 13 Jun 2022 17:22:20 +0000
Subject: [PATCH] Reland "Make depot_tools scripts return exit codes properly
 on all platforms."

This reverts commit 6754c49e02b62d54b9b5d8b8b8a93e77bae935f8.

Reason for revert: will be relanded with fixed batch comments that triggered parser error.

Original change's description:
> 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>

Change-Id: I377f966ea1b1a567de815caca703b5e124a76b64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3696396
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
---
 fetch.bat              |  4 ++--
 gclient.bat            |  4 ++--
 gsutil.py.bat          |  4 ++--
 update_depot_tools     |  4 ++++
 update_depot_tools.bat | 20 ++++++++++++++------
 5 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/fetch.bat b/fetch.bat
index eef301915..26d8bc130 100755
--- a/fetch.bat
+++ b/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 (
-  goto :EOF
+IF %ERRORLEVEL% NEQ 0 (
+  exit /b %ERRORLEVEL%
 )
 
 :: Ensure that "depot_tools" is somewhere in PATH so this tool can be used
diff --git a/gclient.bat b/gclient.bat
index 2934a6ce1..5a6ca1938 100755
--- a/gclient.bat
+++ b/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 (
-  goto :EOF
+IF %ERRORLEVEL% NEQ 0 (
+  exit /b %ERRORLEVEL%
 )
 
 :CALL_GCLIENT
diff --git a/gsutil.py.bat b/gsutil.py.bat
index 5f367cb2a..ab9719087 100755
--- a/gsutil.py.bat
+++ b/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 (
-  goto :EOF
+IF %ERRORLEVEL% NEQ 0 (
+  exit /b %ERRORLEVEL%
 )
 
 :CALL_GSUTIL
diff --git a/update_depot_tools b/update_depot_tools
index 76ebf191a..b017e9ffa 100755
--- a/update_depot_tools
+++ b/update_depot_tools
@@ -129,7 +129,11 @@ 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
   else
     echo "Warning: Your depot_tools directory does not appear to be a git repository, and cannot be updated." 1>&2
     echo "Consider deleting your depot_tools directory and following the instructions at https://www.chromium.org/developers/how-tos/install-depot-tools/ to reinstall it." 1>&2
diff --git a/update_depot_tools.bat b/update_depot_tools.bat
index 4381a6806..b57cdff49 100644
--- a/update_depot_tools.bat
+++ b/update_depot_tools.bat
@@ -5,19 +5,26 @@
 
 :: This batch file will try to sync the root directory.
 
-setlocal
+setlocal enabledelayedexpansion
 
 :: 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.
+:: !ERRORLEVEL! syntax is used to get delayed expansion, because %ERRORLEVEL%
+:: would return a value that was set prior entering the IF block.
 IF "%~nx0"=="update_depot_tools.bat" (
   COPY /Y "%~dp0update_depot_tools.bat" "%TEMP%\update_depot_tools_tmp.bat" >nul
-  if errorlevel 1 goto :EOF
+  if errorlevel 1 (
+    echo Error updating depot_tools, can't copy update_depot_tools.bat to TEMP.
+    exit /b !ERRORLEVEL!
+  )
   :: Use call/exit to avoid leaving an orphaned window title.
   call "%TEMP%\update_depot_tools_tmp.bat" "%~dp0" %*
-  exit /b %ERRORLEVEL%
+  exit /b !ERRORLEVEL!
 )
 
+setlocal disabledelayedexpansion
+
 set DEPOT_TOOLS_DIR=%~1
 SHIFT
 
@@ -25,15 +32,16 @@ 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.
-  goto :EOF
+  exit /b %ERRORLEVEL%
 )
 
 :GIT_UPDATE
@@ -60,7 +68,7 @@ call git fetch -q origin > NUL
 call git checkout -q origin/main > NUL
 if errorlevel 1 (
   echo Failed to update depot_tools.
-  goto :EOF
+  exit /b %ERRORLEVEL%
 )
 
 :: Sync CIPD and CIPD client tools.