From c53bae839d12cf598a2bf7ab1fa141d88eed3600 Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Wed, 16 Dec 2020 23:12:31 +0200
Subject: [PATCH] ci: fail CI jobs when rebasing on master fails (#40161)

As part of the `setup` CI job (which is a prerequisite for all other CI
jobs), we rebase the current code on master to make sure the PR changes
are compatible with the latest code from master, even if the PR has not
been rebased recently.

When it is not possible to automatically rebase (i.e. when there are
conflicts that need to be resolved manually), the job and subsequently
the entire workflow should fail.

This behavior has been accidentally broken in #39592, so that the job
would succeed even if the rebase operation failed.

This commit fixes it by ensuring the `exec()` helper used in
`rebase-pr.js` will throw an error if the underlying command execution
fails. Previously, the function would always return stdout output as a
string and attach a `code` property indicating the exit code of the
command.

Since the exit code isn't necessary in the `rebase-pr.js` script, this
commit simplifies the `exec()` helper by making it return the stdout
output as a plain string (without extra properties) and re-throw any
errors (unless the `ignoreError` argument is set to `true`).

(Initially reported [here][1] by @JoostK.)

[1]: https://angular-team.slack.com/archives/C042EU9T5/p1608070403128900

PR Close #40161
---
 .circleci/rebase-pr.js | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.circleci/rebase-pr.js b/.circleci/rebase-pr.js
index 085ab4398d..9f1e8125ed 100644
--- a/.circleci/rebase-pr.js
+++ b/.circleci/rebase-pr.js
@@ -152,7 +152,7 @@ function getCommonAncestorSha(sha1, sha2) {
  */
 function addAndFetchRemote(owner, name) {
   const remoteName = `${owner}_${name}`;
-  exec(`git remote add ${remoteName} https://github.com/${owner}/${name}.git`, false);
+  exec(`git remote add ${remoteName} https://github.com/${owner}/${name}.git`, true);
   exec(`git fetch ${remoteName}`);
   return remoteName;
 }
@@ -194,14 +194,14 @@ function getRefsAndShasForChange() {
  *
  * Return the trimmed stdout as a string, with an added attribute of the exit code.
  */
-function exec(command, allowStderr = true) {
-  let output = new String();
-  output.code = 0;
+function exec(command, ignoreError = false) {
   try {
-    output += execSync(command, {stdio: ['pipe', 'pipe', 'pipe']}).toString().trim();
+    return execSync(command, {stdio: 'pipe'}).toString().trim();
   } catch (err) {
-    allowStderr && console.error(err.stderr.toString());
-    output.code = err.status;
+    if (ignoreError) {
+      return '';
+    }
+
+    throw err;
   }
-  return output;
 }