Skip to content

Commit 4c8a765

Browse files
author
Simon Engledew
committed
Count the number of parents of the current commit to check it is still a merge
Work around a race condition in actions where sometimes GITHUB_SHA != git rev-parse head
1 parent 5d467d0 commit 4c8a765

File tree

6 files changed

+41
-30
lines changed

6 files changed

+41
-30
lines changed

lib/actions-util.js

Lines changed: 7 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/actions-util.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/actions-util.test.js

Lines changed: 11 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/actions-util.test.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/actions-util.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,30 @@ test("getRef() throws on the empty string", async (t) => {
1919
await t.throwsAsync(actionsutil.getRef);
2020
});
2121

22-
test("getRef() returns merge PR ref if GITHUB_SHA still checked out", async (t) => {
22+
test("getRef() returns merge PR ref if GITHUB_REF still checked out", async (t) => {
2323
const expectedRef = "refs/pull/1/merge";
24-
const currentSha = "a".repeat(40);
2524
process.env["GITHUB_REF"] = expectedRef;
26-
process.env["GITHUB_SHA"] = currentSha;
25+
const sha = "a".repeat(40);
2726

28-
sinon.stub(actionsutil, "getCommitOid").resolves(currentSha);
27+
const callback = sinon.stub(actionsutil, "getCommitOid");
28+
callback.withArgs("refs/pull/1/merge").resolves(sha);
29+
callback.withArgs("HEAD").resolves(sha);
2930

3031
const actualRef = await actionsutil.getRef();
3132
t.deepEqual(actualRef, expectedRef);
33+
callback.restore();
3234
});
3335

34-
test("getRef() returns head PR ref if GITHUB_SHA not currently checked out", async (t) => {
36+
test("getRef() returns head PR ref if GITHUB_REF no longer checked out", async (t) => {
3537
process.env["GITHUB_REF"] = "refs/pull/1/merge";
36-
process.env["GITHUB_SHA"] = "a".repeat(40);
3738

38-
sinon.stub(actionsutil, "getCommitOid").resolves("b".repeat(40));
39+
const callback = sinon.stub(actionsutil, "getCommitOid");
40+
callback.withArgs("refs/pull/1/merge").resolves("a".repeat(40));
41+
callback.withArgs("HEAD").resolves("b".repeat(40));
3942

4043
const actualRef = await actionsutil.getRef();
4144
t.deepEqual(actualRef, "refs/pull/1/head");
45+
callback.restore();
4246
});
4347

4448
test("getAnalysisKey() when a local run", async (t) => {

src/actions-util.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export function prepareLocalRunEnvironment() {
7575
/**
7676
* Gets the SHA of the commit that is currently checked out.
7777
*/
78-
export const getCommitOid = async function (): Promise<string> {
78+
export const getCommitOid = async function (ref = "HEAD"): Promise<string> {
7979
// Try to use git to get the current commit SHA. If that fails then
8080
// log but otherwise silently fall back to using the SHA from the environment.
8181
// The only time these two values will differ is during analysis of a PR when
@@ -87,7 +87,7 @@ export const getCommitOid = async function (): Promise<string> {
8787
let commitOid = "";
8888
await new toolrunner.ToolRunner(
8989
await safeWhich.safeWhich("git"),
90-
["rev-parse", "HEAD"],
90+
["rev-parse", ref],
9191
{
9292
silent: true,
9393
listeners: {
@@ -431,13 +431,15 @@ export async function getRef(): Promise<string> {
431431
// than the 'merge' ref. If so, we want to convert the ref that
432432
// we report back.
433433
const pull_ref_regex = /refs\/pull\/(\d+)\/merge/;
434-
const checkoutSha = await getCommitOid();
434+
const hasChangedRef =
435+
(await getCommitOid(ref)) !== (await getCommitOid("HEAD"));
435436

436-
if (
437-
pull_ref_regex.test(ref) &&
438-
checkoutSha !== getRequiredEnvParam("GITHUB_SHA")
439-
) {
440-
return ref.replace(pull_ref_regex, "refs/pull/$1/head");
437+
if (pull_ref_regex.test(ref) && hasChangedRef) {
438+
const newRef = ref.replace(pull_ref_regex, "refs/pull/$1/head");
439+
core.info(
440+
`No longer on merge commit, rewriting ref from ${ref} to ${newRef}.`
441+
);
442+
return newRef;
441443
} else {
442444
return ref;
443445
}

0 commit comments

Comments
 (0)