robojerk/codeql-action

Merge pull request #2521 from github/cklin/run-git-command

Browse source 
Misc cleanups on git command invocation
This commit is contained in:
Chuan-kai Lin 2024-10-03 13:40:45 -07:00 • committed by GitHub
commit 8b33300963
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 140 additions and 148 deletions
Download patch file Download diff file Expand all files Collapse all files

109
lib/actions-util.js generated
View file

@ -23,7 +23,7 @@ var __importStar = (this && this.__importStar) || function (mod) {
return result;
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.CommandInvocationError = exports.getFileType = exports.FileCmdNotFoundError = exports.determineMergeBaseCommitOid = exports.getCommitOid = exports.getOptionalInput = exports.getRequiredInput = void 0;
exports.CommandInvocationError = exports.getFileType = exports.FileCmdNotFoundError = exports.determineBaseBranchHeadCommitOid = exports.getCommitOid = exports.getOptionalInput = exports.getRequiredInput = void 0;
exports.getTemporaryDirectory = getTemporaryDirectory;
exports.getRef = getRef;
exports.getActionVersion = getActionVersion;
@ -79,6 +79,34 @@ function getTemporaryDirectory() {
? value
: (0, util_1.getRequiredEnvParam)("RUNNER_TEMP");
}
async function runGitCommand(checkoutPath, args, customErrorMessage) {
let stdout = "";
let stderr = "";
try {
await new toolrunner.ToolRunner(await safeWhich.safeWhich("git"), args, {
silent: true,
listeners: {
stdout: (data) => {
stdout += data.toString();
},
stderr: (data) => {
stderr += data.toString();
},
},
cwd: checkoutPath,
}).exec();
return stdout;
}
catch (error) {
let reason = stderr;
if (stderr.includes("not a git repository")) {
reason =
"The checkout path provided to the action does not appear to be a git repository.";
}
core.info(`git call failed. ${customErrorMessage} Error: ${reason}`);
throw error;
}
}
/**
* Gets the SHA of the commit that is currently checked out.
*/
@ -90,72 +118,45 @@ const getCommitOid = async function (checkoutPath, ref = "HEAD") {
// the merge commit, which must mean that git is available.
// Even if this does go wrong, it's not a huge problem for the alerts to
// reported on the merge commit.
let stderr = "";
try {
let commitOid = "";
await new toolrunner.ToolRunner(await safeWhich.safeWhich("git"), ["rev-parse", ref], {
silent: true,
listeners: {
stdout: (data) => {
commitOid += data.toString();
},
stderr: (data) => {
stderr += data.toString();
},
},
cwd: checkoutPath,
}).exec();
return commitOid.trim();
const stdout = await runGitCommand(checkoutPath, ["rev-parse", ref], "Continuing with commit SHA from user input or environment.");
return stdout.trim();
}
catch {
if (stderr.includes("not a git repository")) {
core.info("Could not determine current commit SHA using git. Continuing with data from user input or environment. " +
"The checkout path provided to the action does not appear to be a git repository.");
}
else {
core.info(`Could not determine current commit SHA using git. Continuing with data from user input or environment. ${stderr}`);
}
return (0, exports.getOptionalInput)("sha") || (0, util_1.getRequiredEnvParam)("GITHUB_SHA");
}
};
exports.getCommitOid = getCommitOid;
/**
* 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.
* If the action was triggered by a pull request, determine the commit sha at
* the head of the base branch, using the merge commit that this workflow analyzes.
* Returns undefined if run by other triggers or the base branch commit cannot be
* determined.
*/
const determineMergeBaseCommitOid = async function (checkoutPathOverride) {
const determineBaseBranchHeadCommitOid = async function (checkoutPathOverride) {
if (getWorkflowEventName() !== "pull_request") {
return undefined;
}
const mergeSha = (0, util_1.getRequiredEnvParam)("GITHUB_SHA");
const checkoutPath = checkoutPathOverride ?? (0, exports.getOptionalInput)("checkout_path");
let stderr = "";
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) => {
stderr += data.toString();
},
},
cwd: checkoutPath,
}).exec();
const stdout = await runGitCommand(checkoutPath, ["show", "-s", "--format=raw", mergeSha], "Will calculate the base branch SHA on the server.");
for (const data of stdout.split("\n")) {
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);
}
}
}
// Let's confirm our assumptions: We had a merge commit and the parsed parent data looks correct
if (commitOid === mergeSha &&
headOid.length === 40 &&
@ -165,18 +166,10 @@ const determineMergeBaseCommitOid = async function (checkoutPathOverride) {
return undefined;
}
catch {
if (stderr.includes("not a git repository")) {
core.info("The checkout path provided to the action does not appear to be a git repository. " +
"Will calculate the merge base on the server.");
}
else {
core.info(`Failed to call git to determine merge base. Will calculate the merge base on ` +
`the server. Reason: ${stderr}`);
}
return undefined;
}
};
exports.determineMergeBaseCommitOid = determineMergeBaseCommitOid;
exports.determineBaseBranchHeadCommitOid = determineBaseBranchHeadCommitOid;
/**
* Get the ref currently being analyzed.
*/

2
lib/actions-util.js.map
View file

File diff suppressed because one or more lines are too long

18
lib/actions-util.test.js generated
View file

@ -214,34 +214,36 @@ const util_1 = require("./util");
getAdditionalInputStub.restore();
});
});
(0, ava_1.default)("determineMergeBaseCommitOid non-pullrequest", async (t) => {
(0, ava_1.default)("determineBaseBranchHeadCommitOid non-pullrequest", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "hucairz";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
const result = await actionsUtil.determineMergeBaseCommitOid(__dirname);
const result = await actionsUtil.determineBaseBranchHeadCommitOid(__dirname);
t.deepEqual(result, undefined);
t.deepEqual(0, infoStub.callCount);
infoStub.restore();
});
(0, ava_1.default)("determineMergeBaseCommitOid no error", async (t) => {
(0, ava_1.default)("determineBaseBranchHeadCommitOid not git repository", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
await (0, util_1.withTmpDir)(async (tmpDir) => {
await actionsUtil.determineMergeBaseCommitOid(tmpDir);
await actionsUtil.determineBaseBranchHeadCommitOid(tmpDir);
});
t.deepEqual(1, infoStub.callCount);
t.assert(infoStub.firstCall.args[0].startsWith("The checkout path provided to the action does not appear to be a git repository."));
t.deepEqual(infoStub.firstCall.args[0], "git call failed. Will calculate the base branch SHA on the server. Error: " +
"The checkout path provided to the action does not appear to be a git repository.");
infoStub.restore();
});
(0, ava_1.default)("determineMergeBaseCommitOid other error", async (t) => {
(0, ava_1.default)("determineBaseBranchHeadCommitOid other error", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
const result = await actionsUtil.determineMergeBaseCommitOid(path.join(__dirname, "../../i-dont-exist"));
const result = await actionsUtil.determineBaseBranchHeadCommitOid(path.join(__dirname, "../../i-dont-exist"));
t.deepEqual(result, undefined);
t.deepEqual(1, infoStub.callCount);
t.assert(infoStub.firstCall.args[0].startsWith("Failed to call git to determine merge base."));
t.assert(infoStub.firstCall.args[0].startsWith("git call failed. Will calculate the base branch SHA on the server. Error: "));
t.assert(!infoStub.firstCall.args[0].endsWith("The checkout path provided to the action does not appear to be a git repository."));
infoStub.restore();
});
//# sourceMappingURL=actions-util.test.js.map

2
lib/actions-util.test.js.map
View file

File diff suppressed because one or more lines are too long

2
lib/upload-lib.js generated
View file

@ -413,7 +413,7 @@ async function uploadFiles(sarifPath, checkoutPath, category, features, logger)
logger.debug(`Compressing serialized SARIF`);
const zippedSarif = zlib_1.default.gzipSync(sarifPayload).toString("base64");
const checkoutURI = (0, file_url_1.default)(checkoutPath);
const payload = buildPayload(await actionsUtil.getCommitOid(checkoutPath), await actionsUtil.getRef(), analysisKey, util.getRequiredEnvParam("GITHUB_WORKFLOW"), zippedSarif, actionsUtil.getWorkflowRunID(), actionsUtil.getWorkflowRunAttempt(), checkoutURI, environment, toolNames, await actionsUtil.determineMergeBaseCommitOid());
const payload = buildPayload(await actionsUtil.getCommitOid(checkoutPath), await actionsUtil.getRef(), analysisKey, util.getRequiredEnvParam("GITHUB_WORKFLOW"), zippedSarif, actionsUtil.getWorkflowRunID(), actionsUtil.getWorkflowRunAttempt(), checkoutURI, environment, toolNames, await actionsUtil.determineBaseBranchHeadCommitOid());
// Log some useful debug info about the info
const rawUploadSizeBytes = sarifPayload.length;
logger.debug(`Raw upload size: ${rawUploadSizeBytes} bytes`);

2
lib/upload-lib.js.map
View file

File diff suppressed because one or more lines are too long

26
src/actions-util.test.ts
View file

@ -269,50 +269,56 @@ test("isAnalyzingDefaultBranch()", async (t) => {
});
});
test("determineMergeBaseCommitOid non-pullrequest", async (t) => {
test("determineBaseBranchHeadCommitOid non-pullrequest", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "hucairz";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
const result = await actionsUtil.determineMergeBaseCommitOid(__dirname);
const result = await actionsUtil.determineBaseBranchHeadCommitOid(__dirname);
t.deepEqual(result, undefined);
t.deepEqual(0, infoStub.callCount);
infoStub.restore();
});
test("determineMergeBaseCommitOid no error", async (t) => {
test("determineBaseBranchHeadCommitOid not git repository", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
await withTmpDir(async (tmpDir) => {
await actionsUtil.determineMergeBaseCommitOid(tmpDir);
await actionsUtil.determineBaseBranchHeadCommitOid(tmpDir);
});
t.deepEqual(1, infoStub.callCount);
t.assert(
infoStub.firstCall.args[0].startsWith(
t.deepEqual(
infoStub.firstCall.args[0],
"git call failed. Will calculate the base branch SHA on the server. Error: " +
"The checkout path provided to the action does not appear to be a git repository.",
),
);
infoStub.restore();
});
test("determineMergeBaseCommitOid other error", async (t) => {
test("determineBaseBranchHeadCommitOid other error", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
const result = await actionsUtil.determineMergeBaseCommitOid(
const result = await actionsUtil.determineBaseBranchHeadCommitOid(
path.join(__dirname, "../../i-dont-exist"),
);
t.deepEqual(result, undefined);
t.deepEqual(1, infoStub.callCount);
t.assert(
infoStub.firstCall.args[0].startsWith(
"Failed to call git to determine merge base.",
"git call failed. Will calculate the base branch SHA on the server. Error: ",
),
);
t.assert(
!infoStub.firstCall.args[0].endsWith(
"The checkout path provided to the action does not appear to be a git repository.",
),
);

125
src/actions-util.ts
View file

@ -49,6 +49,38 @@ export function getTemporaryDirectory(): string {
: getRequiredEnvParam("RUNNER_TEMP");
}
async function runGitCommand(
checkoutPath: string | undefined,
args: string[],
customErrorMessage: string,
): Promise<string> {
let stdout = "";
let stderr = "";
try {
await new toolrunner.ToolRunner(await safeWhich.safeWhich("git"), args, {
silent: true,
listeners: {
stdout: (data) => {
stdout += data.toString();
},
stderr: (data) => {
stderr += data.toString();
},
},
cwd: checkoutPath,
}).exec();
return stdout;
} catch (error) {
let reason = stderr;
if (stderr.includes("not a git repository")) {
reason =
"The checkout path provided to the action does not appear to be a git repository.";
}
core.info(`git call failed. ${customErrorMessage} Error: ${reason}`);
throw error;
}
}
/**
* Gets the SHA of the commit that is currently checked out.
*/
@ -63,47 +95,25 @@ export const getCommitOid = async function (
// the merge commit, which must mean that git is available.
// Even if this does go wrong, it's not a huge problem for the alerts to
// reported on the merge commit.
let stderr = "";
try {
let commitOid = "";
await new toolrunner.ToolRunner(
await safeWhich.safeWhich("git"),
const stdout = await runGitCommand(
checkoutPath,
["rev-parse", ref],
{
silent: true,
listeners: {
stdout: (data) => {
commitOid += data.toString();
},
stderr: (data) => {
stderr += data.toString();
},
},
cwd: checkoutPath,
},
).exec();
return commitOid.trim();
"Continuing with commit SHA from user input or environment.",
);
return stdout.trim();
} catch {
if (stderr.includes("not a git repository")) {
core.info(
"Could not determine current commit SHA using git. Continuing with data from user input or environment. " +
"The checkout path provided to the action does not appear to be a git repository.",
);
} else {
core.info(
`Could not determine current commit SHA using git. Continuing with data from user input or environment. ${stderr}`,
);
}
return getOptionalInput("sha") || getRequiredEnvParam("GITHUB_SHA");
}
};
/**
* 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.
* If the action was triggered by a pull request, determine the commit sha at
* the head of the base branch, using the merge commit that this workflow analyzes.
* Returns undefined if run by other triggers or the base branch commit cannot be
* determined.
*/
export const determineMergeBaseCommitOid = async function (
export const determineBaseBranchHeadCommitOid = async function (
checkoutPathOverride?: string,
): Promise<string | undefined> {
if (getWorkflowEventName() !== "pull_request") {
@ -113,37 +123,29 @@ export const determineMergeBaseCommitOid = async function (
const mergeSha = getRequiredEnvParam("GITHUB_SHA");
const checkoutPath =
checkoutPathOverride ?? getOptionalInput("checkout_path");
let stderr = "";
try {
let commitOid = "";
let baseOid = "";
let headOid = "";
await new toolrunner.ToolRunner(
await safeWhich.safeWhich("git"),
const stdout = await runGitCommand(
checkoutPath,
["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) => {
stderr += data.toString();
},
},
cwd: checkoutPath,
},
).exec();
"Will calculate the base branch SHA on the server.",
);
for (const data of stdout.split("\n")) {
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);
}
}
}
// Let's confirm our assumptions: We had a merge commit and the parsed parent data looks correct
if (
@ -155,17 +157,6 @@ export const determineMergeBaseCommitOid = async function (
}
return undefined;
} catch {
if (stderr.includes("not a git repository")) {
core.info(
"The checkout path provided to the action does not appear to be a git repository. " +
"Will calculate the merge base on the server.",
);
} else {
core.info(
`Failed to call git to determine merge base. Will calculate the merge base on ` +
`the server. Reason: ${stderr}`,
);
}
return undefined;
}
};

2
src/upload-lib.ts
View file

@ -609,7 +609,7 @@ export async function uploadFiles(
checkoutURI,
environment,
toolNames,
await actionsUtil.determineMergeBaseCommitOid(),
await actionsUtil.determineBaseBranchHeadCommitOid(),
);
// Log some useful debug info about the info