Skip to content

Use delayed variable expansion for error check in cmd

What does this MR do?

Use delayed variable expansion for error in cmd

Why was this MR needed?

Batch expands variables when it is read not executed, which can lead to unexpected behavior in if and for statements in cmd. To read more about it check https://devblogs.microsoft.com/oldnewthing/20060823-00/ & http://batcheero.blogspot.com/2007/06/how-to-enabledelayedexpansion.html

In !1203 (merged) we introduced the follow code https://gitlab.com/gitlab-org/gitlab-runner/blob/a9b92ea558289218606269073e2c537b017fa66a/shells/abstract.go#L166-171 which leads to the following batch file:

"git" "remote" "add" "origin" "http://gitlab-ci-token:xxx@192.168.1.79:3000/root/ci-scratch-pad.git"
IF %errorlevel% EQU 0 (
  echo Created fresh repository.
) ELSE (
  "git" "remote" "set-url" "origin" "http://gitlab-ci-token:xxx@192.168.1.79:3000/root/ci-scratch-pad.git"
  IF %errorlevel% NEQ 0 exit /b %errorlevel%

  del /f /q ".git\index.lock" 2>NUL 1>NUL
  del /f /q ".git\shallow.lock" 2>NUL 1>NUL
  del /f /q ".git\HEAD.lock" 2>NUL 1>NUL
  del /f /q ".git\hooks\post-checkout" 2>NUL 1>NUL
  "git" "clean" "-ffdx"
  IF %errorlevel% NEQ 0 exit /b %errorlevel%

  "git" "diff" "--no-ext-diff" "--quiet" "--exit-code" 2>NUL 1>NUL
  IF %errorlevel% EQU 0 (
    echo Clean repository
  ) ELSE (
    "git" "reset" "--hard"
    IF %errorlevel% NEQ 0 exit /b %errorlevel%

  )
)

Now, %errorlevel% would always be the value of the first command since the variable is expended everywhere.

Using !errorlevel! instead would allow us to check the exit status of the commands inside of the else condition.

Are there points in the code the reviewer needs to double check?

Testing

I've done the testing with the following .gitlab-ci.yml

stages:
- test

test:
  stage: test
  script:
  - mkdir .test
  - dir

When the repository is already cloned

Screen_Shot_2019-03-26_at_15.57.41

When the repository is already cloned and FF_CMD_DISABLE_DELAYED_ERROR_LEVEL_EXPANSION set to true

As you can see, the job failed because %errorlevel% had the value of command from git remote add

Screen_Shot_2019-03-26_at_15.59.06

Does this MR meet the acceptance criteria?

  • Documentation created/updated
  • Added tests for this feature/bug
  • In case of conflicts with master - branch was rebased

What are the relevant issue numbers?

Closes #4080 (closed)

Edited by Steve Xuereb

Merge request reports