Skip to content

Fix powershell cloning

What does this MR do?

Change how we handle errors in PowerShell, to fix issues like !1338 (merged)

Why was this MR needed?

We are using if/else when we want to handle condtional for PowerShell, this can be an issue when the user set ErrorActionPreference to Stop the clone script will fails as described in #4188 (closed), the only way to do error handling in this case is using try/catch blocks.

For example the clone script looks like this:

& "git" "remote" "add" "origin" "http://gitlab-ci-token:1sBznS3-vo5JCqzRFe1F@192.168.1.79:3000/root/ci-scratch-pad.git" 2>$null
if($?) {
  echo "[32;1mCreated fresh repository.[0;m"
} else {
  & "git" "remote" "set-url" "origin" "http://gitlab-ci-token:1sBznS3-vo5JCqzRFe1F@192.168.1.79:3000/root/ci-scratch-pad.git"
  if(!$?) { Exit $LASTEXITCODE }

}

Now it looks like this:

Try {
  & "git" "remote" "add" "origin" "http://gitlab-ci-token:K36SMw9mGozWTwxqqzQS@192.168.1.79:3000/root/ci-scratch-pad.git" 2>$null
  echo "[32;1mCreated fresh repository.[0;m"
} Catch {
  & "git" "remote" "set-url" "origin" "http://gitlab-ci-token:K36SMw9mGozWTwxqqzQS@192.168.1.79:3000/root/ci-scratch-pad.git"
  if(!$?) { Exit $LASTEXITCODE }

}

The only drawback of this is that we have to call else every time we use an if. If we put the catch line inside of the ifCmd we can on longer put more the 1 line inside of the if which is not ideal.

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

  • Should we do such a philosophical change for PowerShell without any fallback? In 9ccd0ce7 I added back the original behavior so when ErrorActionPreference is not Stop it will use the old error checking, but this seems a bit messy.

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 #4188 (closed)

Edited by Steve Xuereb

Merge request reports