Skip to content

Commit

Permalink
Count the number of parents of the current commit to check it is stil…
Browse files Browse the repository at this point in the history
…l a merge

Work around a race condition in actions where sometimes GITHUB_SHA != git rev-parse head
  • Loading branch information
Simon Engledew committed Mar 22, 2021
1 parent 5d467d0 commit ef92c5a
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 23 deletions.
20 changes: 14 additions & 6 deletions lib/actions-util.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/actions-util.js.map

Large diffs are not rendered by default.

23 changes: 20 additions & 3 deletions lib/actions-util.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/actions-util.test.js.map

Large diffs are not rendered by default.

26 changes: 23 additions & 3 deletions src/actions-util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,40 @@ test("getRef() returns merge PR ref if GITHUB_SHA still checked out", async (t)
process.env["GITHUB_REF"] = expectedRef;
process.env["GITHUB_SHA"] = currentSha;

sinon.stub(actionsutil, "getCommitOid").resolves(currentSha);
const callback = sinon.stub(actionsutil, "getCommitOid");
callback.withArgs("HEAD").resolves(currentSha);

const actualRef = await actionsutil.getRef();
t.deepEqual(actualRef, expectedRef);
callback.restore();
});

test("getRef() returns head PR ref if GITHUB_SHA not currently checked out", async (t) => {
test("getRef() returns merge PR ref if GITHUB_REF still checked out but sha has changed (actions checkout@v1)", async (t) => {
const expectedRef = "refs/pull/1/merge";
process.env["GITHUB_REF"] = expectedRef;
process.env["GITHUB_SHA"] = "b".repeat(40);
const sha = "a".repeat(40);

const callback = sinon.stub(actionsutil, "getCommitOid");
callback.withArgs("refs/pull/1/merge").resolves(sha);
callback.withArgs("HEAD").resolves(sha);

const actualRef = await actionsutil.getRef();
t.deepEqual(actualRef, expectedRef);
callback.restore();
});

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

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

const actualRef = await actionsutil.getRef();
t.deepEqual(actualRef, "refs/pull/1/head");
callback.restore();
});

test("getAnalysisKey() when a local run", async (t) => {
Expand Down
26 changes: 17 additions & 9 deletions src/actions-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function prepareLocalRunEnvironment() {
/**
* Gets the SHA of the commit that is currently checked out.
*/
export const getCommitOid = async function (): Promise<string> {
export const getCommitOid = async function (ref = "HEAD"): Promise<string> {
// Try to use git to get the current commit SHA. If that fails then
// log but otherwise silently fall back to using the SHA from the environment.
// The only time these two values will differ is during analysis of a PR when
Expand All @@ -87,7 +87,7 @@ export const getCommitOid = async function (): Promise<string> {
let commitOid = "";
await new toolrunner.ToolRunner(
await safeWhich.safeWhich("git"),
["rev-parse", "HEAD"],
["rev-parse", ref],
{
silent: true,
listeners: {
Expand Down Expand Up @@ -425,19 +425,27 @@ export async function getRef(): Promise<string> {
// Will be in the form "refs/heads/master" on a push event
// or in the form "refs/pull/N/merge" on a pull_request event
const ref = getRequiredEnvParam("GITHUB_REF");
const sha = getRequiredEnvParam("GITHUB_SHA");

// For pull request refs we want to detect whether the workflow
// has run `git checkout HEAD^2` to analyze the 'head' ref rather
// than the 'merge' ref. If so, we want to convert the ref that
// we report back.
const pull_ref_regex = /refs\/pull\/(\d+)\/merge/;
const checkoutSha = await getCommitOid();

if (
pull_ref_regex.test(ref) &&
checkoutSha !== getRequiredEnvParam("GITHUB_SHA")
) {
return ref.replace(pull_ref_regex, "refs/pull/$1/head");
const head = await getCommitOid("HEAD");
// in actions@v2 we can check if git rev-parse HEAD == GITHUB_SHA
// in actions@v1 this may not be true as it checks out the repository
// using GITHUB_REF. There is a subtle race condition where
// git rev-parse GITHUB_REF != GITHUB_SHA, so we must check
// git git-parse GITHUB_REF == git rev-parse HEAD instead.
const hasChangedRef = sha !== head && (await getCommitOid(ref)) !== head;

if (pull_ref_regex.test(ref) && hasChangedRef) {
const newRef = ref.replace(pull_ref_regex, "refs/pull/$1/head");
core.info(
`No longer on merge commit, rewriting ref from ${ref} to ${newRef}.`
);
return newRef;
} else {
return ref;
}
Expand Down

0 comments on commit ef92c5a

Please sign in to comment.