fix(version): --atomic fallback when GIT_REDIRECT_STDERR is enabl…

…ed (#2467)

commands/version/__tests__/git-push.test.js

@@ -15,6 +15,7 @@ beforeEach(() => {
 
 afterEach(() => {
   jest.restoreAllMocks();
+  delete process.env.GIT_REDIRECT_STDERR;
 });
 
 test("gitPush", async () => {
@@ -71,6 +72,40 @@ test("remote that does not support --atomic", async () => {
   expect(list).toMatch("v4.5.6");
 });
 
+test("remote that does not support --atomic and git stderr redirected to stdout ", async () => {
+  const { cwd } = await cloneFixture("root-manifest-only");
+
+  process.env.GIT_REDIRECT_STDERR = "2>&1";
+
+  await execa("git", ["commit", "--allow-empty", "-m", "change"], { cwd });
+  await execa("git", ["tag", "v4.5.6", "-m", "v4.5.6"], { cwd });
+
+  // the first time the command is executed, simulate remote error
+  childProcess.exec.mockImplementationOnce(async () => {
+    const stdout = "fatal: the receiving end does not support --atomic push";
+    const error = new Error(
+      ["Command failed: git push --follow-tags --atomic --no-verify origin master", stdout].join("\n")
+    );
+
+    error.stdout = stdout;
+
+    throw error;
+  });
+
+  // this call should _not_ throw
+  await gitPush("origin", "master", { cwd });
+
+  expect(childProcess.exec).toHaveBeenCalledTimes(2);
+  expect(childProcess.exec).toHaveBeenLastCalledWith(
+    "git",
+    ["push", "--follow-tags", "--no-verify", "origin", "master"],
+    { cwd }
+  );
+
+  const list = await listRemoteTags(cwd);
+  expect(list).toMatch("v4.5.6");
+});
+
 test("git cli that does not support --atomic", async () => {
   const { cwd } = await cloneFixture("root-manifest-only");
 

commands/version/lib/git-push.js

@@ -12,12 +12,16 @@ function gitPush(remote, branch, opts) {
     .exec("git", ["push", "--follow-tags", "--no-verify", "--atomic", remote, branch], opts)
     .catch(error => {
       // @see https://github.com/sindresorhus/execa/blob/v1.0.0/index.js#L159-L179
-      // the error message _should_ be on stderr, and I don't care if Windows does it wrong
-      if (/atomic/.test(error.stderr)) {
+      // the error message _should_ be on stderr except when GIT_REDIRECT_STDERR has been configured to redirect
+      // to stdout. More details in https://git-scm.com/docs/git#Documentation/git.txt-codeGITREDIRECTSTDERRcode
+      if (
+        /atomic/.test(error.stderr) ||
+        (process.env.GIT_REDIRECT_STDERR === "2>&1" && /atomic/.test(error.stdout))
+      ) {
         // --atomic is only supported in git >=2.4.0, which some crusty CI environments deem unnecessary to upgrade.
         // so let's try again without attempting to pass an option that is almost 5 years old as of this writing...
-        log.warn("gitPush", "--atomic failed, attempting non-atomic push");
         log.warn("gitPush", error.stderr);
+        log.info("gitPush", "--atomic failed, attempting non-atomic push");
 
         return childProcess.exec("git", ["push", "--follow-tags", "--no-verify", remote, branch], opts);
       }