Merge pull request #1860 from github/aeisenberg/better-error-messages

Add better error messages when determining merge-base
This commit is contained in:
Andrew Eisenberg 2023-08-29 13:51:10 -07:00 committed by GitHub
commit c5acfe3b0d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 132 additions and 26 deletions

View file

@ -11,6 +11,7 @@ No user facing changes.
- Update default CodeQL bundle version to 2.14.3. [#1845](https://github.com/github/codeql-action/pull/1845)
- Fixed a bug in CodeQL Action 2.21.3 onwards that affected beta support for [Project Lombok](https://projectlombok.org/) when analyzing Java. The environment variable `CODEQL_EXTRACTOR_JAVA_RUN_ANNOTATION_PROCESSORS` will now be respected if it was manually configured in the workflow. [#1844](https://github.com/github/codeql-action/pull/1844)
- Enable support for Kotlin 1.9.20 when running with CodeQL CLI v2.13.4 through v2.14.3. [#1853](https://github.com/github/codeql-action/pull/1853)
- Better error message when there is a failure to determine the merge base of the code to analysis. [#1860](https://github.com/github/codeql-action/pull/1860)
## 2.21.4 - 14 Aug 2023

30
lib/actions-util.js generated
View file

@ -75,6 +75,7 @@ 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], {
@ -84,7 +85,7 @@ const getCommitOid = async function (checkoutPath, ref = "HEAD") {
commitOid += data.toString();
},
stderr: (data) => {
process.stderr.write(data);
stderr += data.toString();
},
},
cwd: checkoutPath,
@ -92,9 +93,13 @@ const getCommitOid = async function (checkoutPath, ref = "HEAD") {
return commitOid.trim();
}
catch (e) {
core.info("Could not determine current commit SHA using git. Continuing with data from user input or environment.");
core.debug(`Reason: ${e.message}`);
core.debug(e.stack || "NO STACK");
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");
}
};
@ -103,12 +108,13 @@ 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.
*/
const determineMergeBaseCommitOid = async function () {
const determineMergeBaseCommitOid = async function (checkoutPathOverride) {
if (getWorkflowEventName() !== "pull_request") {
return undefined;
}
const mergeSha = (0, util_1.getRequiredEnvParam)("GITHUB_SHA");
const checkoutPath = (0, exports.getOptionalInput)("checkout_path");
const checkoutPath = checkoutPathOverride ?? (0, exports.getOptionalInput)("checkout_path");
let stderr = "";
try {
let commitOid = "";
let baseOid = "";
@ -130,7 +136,7 @@ const determineMergeBaseCommitOid = async function () {
}
},
stderr: (data) => {
process.stderr.write(data);
stderr += data.toString();
},
},
cwd: checkoutPath,
@ -144,8 +150,14 @@ const determineMergeBaseCommitOid = async function () {
return undefined;
}
catch (e) {
core.info(`Failed to call git to determine merge base. Continuing with data from environment: ${e}`);
core.info(e.stack || "NO STACK");
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;
}
};

File diff suppressed because one or more lines are too long

View file

@ -28,6 +28,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
Object.defineProperty(exports, "__esModule", { value: true });
const fs = __importStar(require("fs"));
const path = __importStar(require("path"));
const core = __importStar(require("@actions/core"));
const ava_1 = __importDefault(require("ava"));
const sinon = __importStar(require("sinon"));
const actionsUtil = __importStar(require("./actions-util"));
@ -213,4 +214,32 @@ const util_1 = require("./util");
getAdditionalInputStub.restore();
});
});
(0, ava_1.default)("determineMergeBaseCommitOid 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);
t.deepEqual(result, undefined);
t.deepEqual(0, infoStub.callCount);
infoStub.restore();
});
(0, ava_1.default)("determineMergeBaseCommitOid no error", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
await actionsUtil.determineMergeBaseCommitOid(path.join(__dirname, "../.."));
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."));
infoStub.restore();
});
(0, ava_1.default)("determineMergeBaseCommitOid 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"));
t.deepEqual(result, undefined);
t.deepEqual(1, infoStub.callCount);
t.assert(infoStub.firstCall.args[0].startsWith("Failed to call git to determine merge base."));
infoStub.restore();
});
//# sourceMappingURL=actions-util.test.js.map

File diff suppressed because one or more lines are too long

View file

@ -1,6 +1,7 @@
import * as fs from "fs";
import * as path from "path";
import * as core from "@actions/core";
import test from "ava";
import * as sinon from "sinon";
@ -267,3 +268,50 @@ test("isAnalyzingDefaultBranch()", async (t) => {
getAdditionalInputStub.restore();
});
});
test("determineMergeBaseCommitOid 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);
t.deepEqual(result, undefined);
t.deepEqual(0, infoStub.callCount);
infoStub.restore();
});
test("determineMergeBaseCommitOid no error", async (t) => {
const infoStub = sinon.stub(core, "info");
process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
await actionsUtil.determineMergeBaseCommitOid(path.join(__dirname, "../.."));
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.",
),
);
infoStub.restore();
});
test("determineMergeBaseCommitOid 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"),
);
t.deepEqual(result, undefined);
t.deepEqual(1, infoStub.callCount);
t.assert(
infoStub.firstCall.args[0].startsWith(
"Failed to call git to determine merge base.",
),
);
infoStub.restore();
});

View file

@ -63,6 +63,7 @@ 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(
@ -75,7 +76,7 @@ export const getCommitOid = async function (
commitOid += data.toString();
},
stderr: (data) => {
process.stderr.write(data);
stderr += data.toString();
},
},
cwd: checkoutPath,
@ -83,11 +84,17 @@ export const getCommitOid = async function (
).exec();
return commitOid.trim();
} catch (e) {
core.info(
"Could not determine current commit SHA using git. Continuing with data from user input or environment.",
);
core.debug(`Reason: ${(e as Error).message}`);
core.debug((e as Error).stack || "NO STACK");
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");
}
};
@ -96,15 +103,17 @@ export const getCommitOid = async function (
* 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
> {
export const determineMergeBaseCommitOid = async function (
checkoutPathOverride?: string,
): Promise<string | undefined> {
if (getWorkflowEventName() !== "pull_request") {
return undefined;
}
const mergeSha = getRequiredEnvParam("GITHUB_SHA");
const checkoutPath = getOptionalInput("checkout_path");
const checkoutPath =
checkoutPathOverride ?? getOptionalInput("checkout_path");
let stderr = "";
try {
let commitOid = "";
@ -129,7 +138,7 @@ export const determineMergeBaseCommitOid = async function (): Promise<
}
},
stderr: (data) => {
process.stderr.write(data);
stderr += data.toString();
},
},
cwd: checkoutPath,
@ -146,10 +155,17 @@ export const determineMergeBaseCommitOid = async function (): Promise<
}
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");
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;
}
};