Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(version): --atomic fallback when GIT_REDIRECT_STDERR is enabled #2467

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 35 additions & 0 deletions commands/version/__tests__/git-push.test.js
Expand Up @@ -15,6 +15,7 @@ beforeEach(() => {

afterEach(() => {
jest.restoreAllMocks();
delete process.env.GIT_REDIRECT_STDERR;
});

test("gitPush", async () => {
Expand Down Expand Up @@ -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");

Expand Down
10 changes: 7 additions & 3 deletions commands/version/lib/git-push.js
Expand Up @@ -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);
}
Expand Down