Skip to content

Commit

Permalink
Merge pull request #858 from github/use-better-base-sha
Browse files Browse the repository at this point in the history
Declare the merge base as base for code scanning comparisons
  • Loading branch information
cannist authored Feb 4, 2022
2 parents ff33f03 + 9b14aa7 commit 904d0ac
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 28 deletions.
51 changes: 50 additions & 1 deletion 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.

26 changes: 19 additions & 7 deletions lib/upload-lib.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/upload-lib.js.map

Large diffs are not rendered by default.

15 changes: 12 additions & 3 deletions lib/upload-lib.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/upload-lib.test.js.map

Large diffs are not rendered by default.

60 changes: 60 additions & 0 deletions src/actions-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,66 @@ export const getCommitOid = async function (ref = "HEAD"): Promise<string> {
}
};

/**
* If the action was triggered by a pull request, determine the commit sha of the merge base.
* Returns undefined if run by other triggers or the merge base cannot be determined.
*/
export const determineMergeBaseCommitOid = async function (): Promise<
string | undefined
> {
if (process.env.GITHUB_EVENT_NAME !== "pull_request") {
return undefined;
}

const mergeSha = getRequiredEnvParam("GITHUB_SHA");

try {
let commitOid = "";
let baseOid = "";
let headOid = "";

await new toolrunner.ToolRunner(
await safeWhich.safeWhich("git"),
["show", "-s", "--format=raw", mergeSha],
{
silent: true,
listeners: {
stdline: (data) => {
if (data.startsWith("commit ") && commitOid === "") {
commitOid = data.substring(7);
} else if (data.startsWith("parent ")) {
if (baseOid === "") {
baseOid = data.substring(7);
} else if (headOid === "") {
headOid = data.substring(7);
}
}
},
stderr: (data) => {
process.stderr.write(data);
},
},
}
).exec();

// Let's confirm our assumptions: We had a merge commit and the parsed parent data looks correct
if (
commitOid === mergeSha &&
headOid.length === 40 &&
baseOid.length === 40
) {
return baseOid;
}
return undefined;
} catch (e) {
core.info(
`Failed to call git to determine merge base. Continuing with data from environment: ${e}`
);
core.info((e as Error).stack || "NO STACK");
return undefined;
}
};

interface WorkflowJobStep {
run: any;
}
Expand Down
31 changes: 28 additions & 3 deletions src/upload-lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,17 @@ test("validate correct payload used per version", async (t) => {
"/opt/src",
undefined,
["CodeQL", "eslint"],
version
version,
"mergeBaseCommit"
);
// Not triggered by a pull request
t.falsy(payload.base_ref);
t.falsy(payload.base_sha);
}

process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "commit";
process.env["GITHUB_BASE_REF"] = "master";
process.env[
"GITHUB_EVENT_PATH"
] = `${__dirname}/../src/testdata/pull_request.json`;
Expand All @@ -79,8 +82,29 @@ test("validate correct payload used per version", async (t) => {
"/opt/src",
undefined,
["CodeQL", "eslint"],
version
version,
"mergeBaseCommit"
);
// Uploads for a merge commit use the merge base
t.deepEqual(payload.base_ref, "refs/heads/master");
t.deepEqual(payload.base_sha, "mergeBaseCommit");
}

for (const version of newVersions) {
const payload: any = uploadLib.buildPayload(
"headCommit",
"refs/pull/123/head",
"key",
undefined,
"",
undefined,
"/opt/src",
undefined,
["CodeQL", "eslint"],
version,
"mergeBaseCommit"
);
// Uploads for the head use the PR base
t.deepEqual(payload.base_ref, "refs/heads/master");
t.deepEqual(payload.base_sha, "f95f852bd8fca8fcc58a9a2d6c842781e32a215e");
}
Expand All @@ -96,7 +120,8 @@ test("validate correct payload used per version", async (t) => {
"/opt/src",
undefined,
["CodeQL", "eslint"],
version
version,
"mergeBaseCommit"
);
// These older versions won't expect these values
t.falsy(payload.base_ref);
Expand Down
37 changes: 26 additions & 11 deletions src/upload-lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ export function buildPayload(
checkoutURI: string,
environment: string | undefined,
toolNames: string[],
gitHubVersion: util.GitHubVersion
gitHubVersion: util.GitHubVersion,
mergeBaseCommitOid: string | undefined
) {
if (util.isActions()) {
const payloadObj = {
Expand All @@ -314,15 +315,28 @@ export function buildPayload(
gitHubVersion.type !== util.GitHubVariant.GHES ||
semver.satisfies(gitHubVersion.version, `>=3.1`)
) {
if (
process.env.GITHUB_EVENT_NAME === "pull_request" &&
process.env.GITHUB_EVENT_PATH
) {
const githubEvent = JSON.parse(
fs.readFileSync(process.env.GITHUB_EVENT_PATH, "utf8")
);
payloadObj.base_ref = `refs/heads/${githubEvent.pull_request.base.ref}`;
payloadObj.base_sha = githubEvent.pull_request.base.sha;
if (process.env.GITHUB_EVENT_NAME === "pull_request") {
if (
commitOid === util.getRequiredEnvParam("GITHUB_SHA") &&
mergeBaseCommitOid
) {
// We're uploading results for the merge commit
// and were able to determine the merge base.
// So we use that as the most accurate base.
payloadObj.base_ref = `refs/heads/${util.getRequiredEnvParam(
"GITHUB_BASE_REF"
)}`;
payloadObj.base_sha = mergeBaseCommitOid;
} else if (process.env.GITHUB_EVENT_PATH) {
// Either we're not uploading results for the merge commit
// or we could not determine the merge base.
// Using the PR base is the only option here
const githubEvent = JSON.parse(
fs.readFileSync(process.env.GITHUB_EVENT_PATH, "utf8")
);
payloadObj.base_ref = `refs/heads/${githubEvent.pull_request.base.ref}`;
payloadObj.base_sha = githubEvent.pull_request.base.sha;
}
}
}
return payloadObj;
Expand Down Expand Up @@ -389,7 +403,8 @@ async function uploadFiles(
checkoutURI,
environment,
toolNames,
gitHubVersion
gitHubVersion,
await actionsUtil.determineMergeBaseCommitOid()
);

// Log some useful debug info about the info
Expand Down

0 comments on commit 904d0ac

Please sign in to comment.