Compare commits

..

12 Commits

Author SHA1 Message Date
Kasper Svendsen 8c8b134bbc Log numbered of PR diff-range filtered alerts 2025-10-28 10:32:34 +01:00
Kasper Svendsen d321539bd2 WIP: Move generation out absolute paths for diff-informed PR ranges 2025-10-28 10:26:19 +01:00
Alex Eyers-Taylor 5e51e6e0db Consume precomputed diff ranges in analyze 2025-10-28 09:25:49 +01:00
Alex Eyers-Taylor 7329c8462a Persist PR diff ranges during init
We don't use them yet and will re-save them during analysis.
2025-10-28 09:25:42 +01:00
Kasper Svendsen 8d77149e0c Merge pull request #3238 from github/kaspersv/extract-diff-range-computation
Move diff-range computation into utils
2025-10-27 15:40:12 +01:00
Kasper Svendsen cc17bed958 Move diff-range computation tests 2025-10-27 09:46:16 +01:00
Kasper Svendsen 91ec0ed58f Move diff-range computation into utils for reuse 2025-10-27 09:43:11 +01:00
Kasper Svendsen 4e0b2cd814 Merge pull request #3232 from github/kaspersv/unique-overlay-base-keys
Ensure uniqueness of overlay-base database cache keys
2025-10-27 08:36:12 +01:00
Kasper Svendsen 66759e57b2 Improve error handling for overlay-base cache key creation 2025-10-24 15:49:26 +02:00
Kasper Svendsen cbcae45fff Reorder components of overlay-base cache key postfix 2025-10-24 15:46:17 +02:00
Kasper Svendsen b4ce335286 Ensure uniqueness of overlay-base database cache keys 2025-10-24 11:11:57 +02:00
Chuan-kai Lin c4b73722ba Add overlay-base database cache key tests 2025-10-24 10:47:17 +02:00
15 changed files with 1167 additions and 1056 deletions
+48 -193
View File
@@ -90161,29 +90161,6 @@ var persistInputs = function() {
);
core4.saveState(persistedInputsKey, JSON.stringify(inputEnvironmentVariables));
};
function getPullRequestBranches() {
const pullRequest = github.context.payload.pull_request;
if (pullRequest) {
return {
base: pullRequest.base.ref,
// We use the head label instead of the head ref here, because the head
// ref lacks owner information and by itself does not uniquely identify
// the head branch (which may be in a forked repository).
head: pullRequest.head.label
};
}
const codeScanningRef = process.env.CODE_SCANNING_REF;
const codeScanningBaseBranch = process.env.CODE_SCANNING_BASE_BRANCH;
if (codeScanningRef && codeScanningBaseBranch) {
return {
base: codeScanningBaseBranch,
// PR analysis under Default Setup analyzes the PR head commit instead of
// the merge commit, so we can use the provided ref directly.
head: codeScanningRef
};
}
return void 0;
}
var qualityCategoryMapping = {
"c#": "csharp",
cpp: "c-cpp",
@@ -90251,6 +90228,9 @@ var path16 = __toESM(require("path"));
var import_perf_hooks2 = require("perf_hooks");
var io5 = __toESM(require_io());
// src/autobuild.ts
var core11 = __toESM(require_core());
// src/api-client.ts
var core5 = __toESM(require_core());
var githubUtils = __toESM(require_utils4());
@@ -90430,9 +90410,6 @@ function wrapApiConfigurationError(e) {
return e;
}
// src/autobuild.ts
var core11 = __toESM(require_core());
// src/codeql.ts
var fs14 = __toESM(require("fs"));
var path14 = __toESM(require("path"));
@@ -91070,7 +91047,8 @@ async function uploadOverlayBaseDatabaseToCache(codeql, config, logger) {
const cacheSaveKey = await getCacheSaveKey(
config,
codeQlVersion,
checkoutPath
checkoutPath,
logger
);
logger.info(
`Uploading overlay-base database to Actions cache with key ${cacheSaveKey}`
@@ -91095,13 +91073,23 @@ async function uploadOverlayBaseDatabaseToCache(codeql, config, logger) {
logger.info(`Successfully uploaded overlay-base database from ${dbLocation}`);
return true;
}
async function getCacheSaveKey(config, codeQlVersion, checkoutPath) {
async function getCacheSaveKey(config, codeQlVersion, checkoutPath, logger) {
let runId = 1;
let attemptId = 1;
try {
runId = getWorkflowRunID();
attemptId = getWorkflowRunAttempt();
} catch (e) {
logger.warning(
`Failed to get workflow run ID or attempt ID. Reason: ${getErrorMessage(e)}`
);
}
const sha = await getCommitOid(checkoutPath);
const restoreKeyPrefix = await getCacheRestoreKeyPrefix(
config,
codeQlVersion
);
return `${restoreKeyPrefix}${sha}`;
return `${restoreKeyPrefix}${sha}-${runId}-${attemptId}`;
}
async function getCacheRestoreKeyPrefix(config, codeQlVersion) {
const languages = [...config.languages].sort().join("_");
@@ -91583,34 +91571,9 @@ var GitHubFeatureFlags = class {
};
// src/diff-informed-analysis-utils.ts
async function getDiffInformedAnalysisBranches(codeql, features, logger) {
if (!await features.getValue("diff_informed_queries" /* DiffInformedQueries */, codeql)) {
return void 0;
}
const gitHubVersion = await getGitHubVersion();
if (gitHubVersion.type === 1 /* GHES */ && satisfiesGHESVersion(gitHubVersion.version, "<3.19", true)) {
return void 0;
}
const branches = getPullRequestBranches();
if (!branches) {
logger.info(
"Not performing diff-informed analysis because we are not analyzing a pull request."
);
}
return branches;
}
function getDiffRangesJsonFilePath() {
return path9.join(getTemporaryDirectory(), "pr-diff-range.json");
}
function writeDiffRangesJsonFile(logger, ranges) {
const jsonContents = JSON.stringify(ranges, null, 2);
const jsonFilePath = getDiffRangesJsonFilePath();
fs8.writeFileSync(jsonFilePath, jsonContents);
logger.debug(
`Wrote pr-diff-range JSON file to ${jsonFilePath}:
${jsonContents}`
);
}
function readDiffRangesJsonFile(logger) {
const jsonFilePath = getDiffRangesJsonFilePath();
if (!fs8.existsSync(jsonFilePath)) {
@@ -93690,14 +93653,31 @@ async function finalizeDatabaseCreation(codeql, config, threadsFlag, memoryFlag,
trap_import_duration_ms: Math.round(trapImportTime)
};
}
async function setupDiffInformedQueryRun(branches, logger) {
async function setupDiffInformedQueryRun(logger) {
return await withGroupAsync(
"Generating diff range extension pack",
async () => {
let diffRanges;
try {
diffRanges = readDiffRangesJsonFile(logger);
} catch (e) {
logger.debug(
`Failed to read precomputed diff ranges: ${getErrorMessage(e)}`
);
diffRanges = void 0;
}
if (diffRanges === void 0) {
logger.info(
"No precomputed diff ranges found; skipping diff-informed analysis stage."
);
return void 0;
}
const fileCount = new Set(
diffRanges.filter((r) => r.path).map((r) => r.path)
).size;
logger.info(
`Calculating diff ranges for ${branches.base}...${branches.head}`
`Using precomputed diff ranges (${diffRanges.length} ranges across ${fileCount} files).`
);
const diffRanges = await getPullRequestEditedDiffRanges(branches, logger);
const packDir = writeDiffRangeDataExtensionPack(logger, diffRanges);
if (packDir === void 0) {
logger.warning(
@@ -93712,117 +93692,6 @@ async function setupDiffInformedQueryRun(branches, logger) {
}
);
}
async function getPullRequestEditedDiffRanges(branches, logger) {
const fileDiffs = await getFileDiffsWithBasehead(branches, logger);
if (fileDiffs === void 0) {
return void 0;
}
if (fileDiffs.length >= 300) {
logger.warning(
`Cannot retrieve the full diff because there are too many (${fileDiffs.length}) changed files in the pull request.`
);
return void 0;
}
const results = [];
for (const filediff of fileDiffs) {
const diffRanges = getDiffRanges(filediff, logger);
if (diffRanges === void 0) {
return void 0;
}
results.push(...diffRanges);
}
return results;
}
async function getFileDiffsWithBasehead(branches, logger) {
const repositoryNwo = getRepositoryNwoFromEnv(
"CODE_SCANNING_REPOSITORY",
"GITHUB_REPOSITORY"
);
const basehead = `${branches.base}...${branches.head}`;
try {
const response = await getApiClient().rest.repos.compareCommitsWithBasehead(
{
owner: repositoryNwo.owner,
repo: repositoryNwo.repo,
basehead,
per_page: 1
}
);
logger.debug(
`Response from compareCommitsWithBasehead(${basehead}):
${JSON.stringify(response, null, 2)}`
);
return response.data.files;
} catch (error2) {
if (error2.status) {
logger.warning(`Error retrieving diff ${basehead}: ${error2.message}`);
logger.debug(
`Error running compareCommitsWithBasehead(${basehead}):
Request: ${JSON.stringify(error2.request, null, 2)}
Error Response: ${JSON.stringify(error2.response, null, 2)}`
);
return void 0;
} else {
throw error2;
}
}
}
function getDiffRanges(fileDiff, logger) {
const filename = path16.join(getRequiredInput("checkout_path"), fileDiff.filename).replaceAll(path16.sep, "/");
if (fileDiff.patch === void 0) {
if (fileDiff.changes === 0) {
return [];
}
return [
{
path: filename,
startLine: 0,
endLine: 0
}
];
}
let currentLine = 0;
let additionRangeStartLine = void 0;
const diffRanges = [];
const diffLines = fileDiff.patch.split("\n");
diffLines.push(" ");
for (const diffLine of diffLines) {
if (diffLine.startsWith("-")) {
continue;
}
if (diffLine.startsWith("+")) {
if (additionRangeStartLine === void 0) {
additionRangeStartLine = currentLine;
}
currentLine++;
continue;
}
if (additionRangeStartLine !== void 0) {
diffRanges.push({
path: filename,
startLine: additionRangeStartLine,
endLine: currentLine - 1
});
additionRangeStartLine = void 0;
}
if (diffLine.startsWith("@@ ")) {
const match = diffLine.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
if (match === null) {
logger.warning(
`Cannot parse diff hunk header for ${fileDiff.filename}: ${diffLine}`
);
return void 0;
}
currentLine = parseInt(match[1], 10);
continue;
}
if (diffLine.startsWith(" ")) {
currentLine++;
continue;
}
}
return diffRanges;
}
function writeDiffRangeDataExtensionPack(logger, ranges) {
if (ranges === void 0) {
return void 0;
@@ -93852,15 +93721,11 @@ extensions:
checkPresence: false
data:
`;
let data = ranges.map(
(range) => (
// Using yaml.dump() with `forceQuotes: true` ensures that all special
// characters are escaped, and that the path is always rendered as a
// quoted string on a single line.
` - [${dump(range.path, { forceQuotes: true }).trim()}, ${range.startLine}, ${range.endLine}]
`
)
).join("");
let data = ranges.map((range) => {
const filename = path16.join(getRequiredInput("checkout_path"), range.path).replaceAll(path16.sep, "/");
return ` - [${dump(filename, { forceQuotes: true }).trim()}, ${range.startLine}, ${range.endLine}]
`;
}).join("");
if (!data) {
data = ' - ["", 0, 0]\n';
}
@@ -93871,7 +93736,6 @@ extensions:
`Wrote pr-diff-range extension pack to ${extensionFilePath}:
${extensionContents}`
);
writeDiffRangesJsonFile(logger, ranges);
return diffRangeDir;
}
var defaultSuites = /* @__PURE__ */ new Set([
@@ -95919,7 +95783,6 @@ async function postProcessSarifFiles(logger, features, checkoutPath, sarifPaths,
}
async function writePostProcessedFiles(logger, pathInput, uploadTarget, postProcessingResults) {
const outputPath = pathInput || getOptionalEnvVar("CODEQL_ACTION_SARIF_DUMP_DIR" /* SARIF_DUMP_DIR */);
logger.info(`Post-processed SARIF output path: ${outputPath}`);
if (outputPath !== void 0) {
dumpSarifFile(
JSON.stringify(postProcessingResults.sarif),
@@ -96149,9 +96012,9 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!diffRanges?.length) {
return sarif;
}
const checkoutPath = getRequiredInput("checkout_path");
for (const run2 of sarif.runs) {
if (run2.results) {
const preAlertCount = run2.results.length;
run2.results = run2.results.filter((result) => {
const locations = [
...(result.locations || []).map((loc) => loc.physicalLocation),
@@ -96163,12 +96026,13 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!locationUri || locationStartLine === void 0) {
return false;
}
const locationPath = path18.join(checkoutPath, locationUri).replaceAll(path18.sep, "/");
return diffRanges.some(
(range) => range.path === locationPath && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
(range) => range.path === locationUri && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
);
});
});
const postAlertCount = run2.results.length;
logger.info(`Filtered ${preAlertCount - postAlertCount} alerts based on diff range.`);
}
}
return sarif;
@@ -96176,7 +96040,6 @@ function filterAlertsByDiffRange(logger, sarif) {
// src/upload-sarif.ts
async function postProcessAndUploadSarif(logger, features, uploadKind, checkoutPath, sarifPath, category, postProcessedOutputPath) {
logger.info("XXX at upload-sarif.ts|postProcessAndUploadSarif");
const sarifGroups = await getGroupedSarifFilePaths(
logger,
sarifPath
@@ -96185,9 +96048,6 @@ async function postProcessAndUploadSarif(logger, features, uploadKind, checkoutP
for (const [analysisKind, sarifFiles] of unsafeEntriesInvariant(
sarifGroups
)) {
logger.info(
`XXX at upload-sarif.ts|postProcessAndUploadSarif loop for analysisKind: ${analysisKind}`
);
const analysisConfig = getAnalysisConfig(analysisKind);
const postProcessingResults = await postProcessSarifFiles(
logger,
@@ -96373,12 +96233,7 @@ async function run() {
getOptionalInput("ram") || process.env["CODEQL_RAM"],
logger
);
const branches = await getDiffInformedAnalysisBranches(
codeql,
features,
logger
);
const diffRangePackDir = branches ? await setupDiffInformedQueryRun(branches, logger) : void 0;
const diffRangePackDir = await setupDiffInformedQueryRun(logger);
await warnIfGoInstalledAfterInit(config, logger);
await runAutobuildIfLegacyGoWorkflow(config, logger);
dbCreationTimings = await runFinalize(
@@ -96415,7 +96270,7 @@ async function run() {
if (runStats) {
const checkoutPath = getRequiredInput("checkout_path");
const category = getOptionalInput("category");
if (Math.random() > -1) {
if (await features.getValue("analyze_use_new_upload" /* AnalyzeUseNewUpload */)) {
uploadResults = await postProcessAndUploadSarif(
logger,
features,
+8 -7
View File
@@ -91192,8 +91192,8 @@ var require_primordials = __commonJS({
ArrayPrototypeIndexOf(self2, el) {
return self2.indexOf(el);
},
ArrayPrototypeJoin(self2, sep5) {
return self2.join(sep5);
ArrayPrototypeJoin(self2, sep4) {
return self2.join(sep4);
},
ArrayPrototypeMap(self2, fn) {
return self2.map(fn);
@@ -103080,7 +103080,7 @@ var require_commonjs16 = __commonJS({
*
* @internal
*/
constructor(cwd = process.cwd(), pathImpl, sep5, { nocase, childrenCacheSize = 16 * 1024, fs: fs20 = defaultFS } = {}) {
constructor(cwd = process.cwd(), pathImpl, sep4, { nocase, childrenCacheSize = 16 * 1024, fs: fs20 = defaultFS } = {}) {
this.#fs = fsFromOption(fs20);
if (cwd instanceof URL || cwd.startsWith("file://")) {
cwd = (0, node_url_1.fileURLToPath)(cwd);
@@ -103091,7 +103091,7 @@ var require_commonjs16 = __commonJS({
this.#resolveCache = new ResolveCache();
this.#resolvePosixCache = new ResolveCache();
this.#children = new ChildrenCache(childrenCacheSize);
const split = cwdPath.substring(this.rootPath.length).split(sep5);
const split = cwdPath.substring(this.rootPath.length).split(sep4);
if (split.length === 1 && !split[0]) {
split.pop();
}
@@ -133524,9 +133524,9 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!diffRanges?.length) {
return sarif;
}
const checkoutPath = getRequiredInput("checkout_path");
for (const run2 of sarif.runs) {
if (run2.results) {
const preAlertCount = run2.results.length;
run2.results = run2.results.filter((result) => {
const locations = [
...(result.locations || []).map((loc) => loc.physicalLocation),
@@ -133538,12 +133538,13 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!locationUri || locationStartLine === void 0) {
return false;
}
const locationPath = path17.join(checkoutPath, locationUri).replaceAll(path17.sep, "/");
return diffRanges.some(
(range) => range.path === locationPath && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
(range) => range.path === locationUri && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
);
});
});
const postAlertCount = run2.results.length;
logger.info(`Filtered ${preAlertCount - postAlertCount} alerts based on diff range.`);
}
}
return sarif;
+562 -407
View File
File diff suppressed because it is too large Load Diff
+4 -4
View File
@@ -92733,7 +92733,6 @@ async function postProcessSarifFiles(logger, features, checkoutPath, sarifPaths,
}
async function writePostProcessedFiles(logger, pathInput, uploadTarget, postProcessingResults) {
const outputPath = pathInput || getOptionalEnvVar("CODEQL_ACTION_SARIF_DUMP_DIR" /* SARIF_DUMP_DIR */);
logger.info(`Post-processed SARIF output path: ${outputPath}`);
if (outputPath !== void 0) {
dumpSarifFile(
JSON.stringify(postProcessingResults.sarif),
@@ -92963,9 +92962,9 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!diffRanges?.length) {
return sarif;
}
const checkoutPath = getRequiredInput("checkout_path");
for (const run of sarif.runs) {
if (run.results) {
const preAlertCount = run.results.length;
run.results = run.results.filter((result) => {
const locations = [
...(result.locations || []).map((loc) => loc.physicalLocation),
@@ -92977,12 +92976,13 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!locationUri || locationStartLine === void 0) {
return false;
}
const locationPath = path14.join(checkoutPath, locationUri).replaceAll(path14.sep, "/");
return diffRanges.some(
(range) => range.path === locationPath && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
(range) => range.path === locationUri && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
);
});
});
const postAlertCount = run.results.length;
logger.info(`Filtered ${preAlertCount - postAlertCount} alerts based on diff range.`);
}
}
return sarif;
+4 -8
View File
@@ -93389,7 +93389,6 @@ async function postProcessSarifFiles(logger, features, checkoutPath, sarifPaths,
}
async function writePostProcessedFiles(logger, pathInput, uploadTarget, postProcessingResults) {
const outputPath = pathInput || getOptionalEnvVar("CODEQL_ACTION_SARIF_DUMP_DIR" /* SARIF_DUMP_DIR */);
logger.info(`Post-processed SARIF output path: ${outputPath}`);
if (outputPath !== void 0) {
dumpSarifFile(
JSON.stringify(postProcessingResults.sarif),
@@ -93589,9 +93588,9 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!diffRanges?.length) {
return sarif;
}
const checkoutPath = getRequiredInput("checkout_path");
for (const run2 of sarif.runs) {
if (run2.results) {
const preAlertCount = run2.results.length;
run2.results = run2.results.filter((result) => {
const locations = [
...(result.locations || []).map((loc) => loc.physicalLocation),
@@ -93603,12 +93602,13 @@ function filterAlertsByDiffRange(logger, sarif) {
if (!locationUri || locationStartLine === void 0) {
return false;
}
const locationPath = path15.join(checkoutPath, locationUri).replaceAll(path15.sep, "/");
return diffRanges.some(
(range) => range.path === locationPath && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
(range) => range.path === locationUri && (range.startLine <= locationStartLine && range.endLine >= locationStartLine || range.startLine === 0 && range.endLine === 0)
);
});
});
const postAlertCount = run2.results.length;
logger.info(`Filtered ${preAlertCount - postAlertCount} alerts based on diff range.`);
}
}
return sarif;
@@ -93616,7 +93616,6 @@ function filterAlertsByDiffRange(logger, sarif) {
// src/upload-sarif.ts
async function postProcessAndUploadSarif(logger, features, uploadKind, checkoutPath, sarifPath, category, postProcessedOutputPath) {
logger.info("XXX at upload-sarif.ts|postProcessAndUploadSarif");
const sarifGroups = await getGroupedSarifFilePaths(
logger,
sarifPath
@@ -93625,9 +93624,6 @@ async function postProcessAndUploadSarif(logger, features, uploadKind, checkoutP
for (const [analysisKind, sarifFiles] of unsafeEntriesInvariant(
sarifGroups
)) {
logger.info(
`XXX at upload-sarif.ts|postProcessAndUploadSarif loop for analysisKind: ${analysisKind}`
);
const analysisConfig = getAnalysisConfig(analysisKind);
const postProcessingResults = await postProcessSarifFiles(
logger,
+3 -10
View File
@@ -30,7 +30,6 @@ import {
DependencyCacheUploadStatusReport,
uploadDependencyCaches,
} from "./dependency-caching";
import { getDiffInformedAnalysisBranches } from "./diff-informed-analysis-utils";
import { EnvVar } from "./environment";
import { Feature, Features } from "./feature-flags";
import { KnownLanguage } from "./languages";
@@ -302,14 +301,8 @@ async function run() {
logger,
);
const branches = await getDiffInformedAnalysisBranches(
codeql,
features,
logger,
);
const diffRangePackDir = branches
? await setupDiffInformedQueryRun(branches, logger)
: undefined;
// Setup diff informed analysis if needed (based on whether init created the file)
const diffRangePackDir = await setupDiffInformedQueryRun(logger);
await warnIfGoInstalledAfterInit(config, logger);
await runAutobuildIfLegacyGoWorkflow(config, logger);
@@ -351,7 +344,7 @@ async function run() {
const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
const category = actionsUtil.getOptionalInput("category");
if (Math.random() > -1) {
if (await features.getValue(Feature.AnalyzeUseNewUpload)) {
uploadResults = await postProcessAndUploadSarif(
logger,
features,
-200
View File
@@ -4,10 +4,8 @@ import * as path from "path";
import test from "ava";
import * as sinon from "sinon";
import * as actionsUtil from "./actions-util";
import { CodeQuality, CodeScanning } from "./analyses";
import {
exportedForTesting,
runQueries,
defaultSuites,
resolveQuerySuiteAlias,
@@ -131,204 +129,6 @@ test("status report fields", async (t) => {
});
});
function runGetDiffRanges(changes: number, patch: string[] | undefined): any {
sinon
.stub(actionsUtil, "getRequiredInput")
.withArgs("checkout_path")
.returns("/checkout/path");
return exportedForTesting.getDiffRanges(
{
filename: "test.txt",
changes,
patch: patch?.join("\n"),
},
getRunnerLogger(true),
);
}
test("getDiffRanges: file unchanged", async (t) => {
const diffRanges = runGetDiffRanges(0, undefined);
t.deepEqual(diffRanges, []);
});
test("getDiffRanges: file diff too large", async (t) => {
const diffRanges = runGetDiffRanges(1000000, undefined);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 0,
endLine: 0,
},
]);
});
test("getDiffRanges: diff thunk with single addition range", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,6 +50,8 @@",
" a",
" b",
" c",
"+1",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 53,
endLine: 54,
},
]);
});
test("getDiffRanges: diff thunk with single deletion range", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,8 +50,6 @@",
" a",
" b",
" c",
"-1",
"-2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, []);
});
test("getDiffRanges: diff thunk with single update range", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,7 +50,7 @@",
" a",
" b",
" c",
"-1",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 53,
endLine: 53,
},
]);
});
test("getDiffRanges: diff thunk with addition ranges", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,7 +50,9 @@",
" a",
" b",
" c",
"+1",
" c",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 53,
endLine: 53,
},
{
path: "/checkout/path/test.txt",
startLine: 55,
endLine: 55,
},
]);
});
test("getDiffRanges: diff thunk with mixed ranges", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,7 +50,7 @@",
" a",
" b",
" c",
"-1",
" d",
"-2",
"+3",
" e",
" f",
"+4",
"+5",
" g",
" h",
" i",
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 54,
endLine: 54,
},
{
path: "/checkout/path/test.txt",
startLine: 57,
endLine: 58,
},
]);
});
test("getDiffRanges: multiple diff thunks", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,6 +50,8 @@",
" a",
" b",
" c",
"+1",
"+2",
" d",
" e",
" f",
"@@ -130,6 +150,8 @@",
" a",
" b",
" c",
"+1",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 53,
endLine: 54,
},
{
path: "/checkout/path/test.txt",
startLine: 153,
endLine: 154,
},
]);
});
test("getDiffRanges: no diff context lines", async (t) => {
const diffRanges = runGetDiffRanges(2, ["@@ -30 +50,2 @@", "+1", "+2"]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
startLine: 50,
endLine: 51,
},
]);
});
test("getDiffRanges: malformed thunk header", async (t) => {
const diffRanges = runGetDiffRanges(2, ["@@ 30 +50,2 @@", "+1", "+2"]);
t.deepEqual(diffRanges, undefined);
});
test("resolveQuerySuiteAlias", (t) => {
// default query suite names should resolve to something language-specific ending in `.qls`.
for (const suite of defaultSuites) {
+41 -206
View File
@@ -6,13 +6,8 @@ import * as io from "@actions/io";
import * as del from "del";
import * as yaml from "js-yaml";
import {
getRequiredInput,
getTemporaryDirectory,
PullRequestBranches,
} from "./actions-util";
import { getTemporaryDirectory, getRequiredInput } from "./actions-util";
import * as analyses from "./analyses";
import { getApiClient } from "./api-client";
import { setupCppAutobuild } from "./autobuild";
import { type CodeQL } from "./codeql";
import * as configUtils from "./config-utils";
@@ -20,14 +15,13 @@ import { getJavaTempDependencyDir } from "./dependency-caching";
import { addDiagnostic, makeDiagnostic } from "./diagnostics";
import {
DiffThunkRange,
writeDiffRangesJsonFile,
readDiffRangesJsonFile,
} from "./diff-informed-analysis-utils";
import { EnvVar } from "./environment";
import { FeatureEnablement, Feature } from "./feature-flags";
import { KnownLanguage, Language } from "./languages";
import { Logger, withGroupAsync } from "./logging";
import { OverlayDatabaseMode } from "./overlay-database-utils";
import { getRepositoryNwoFromEnv } from "./repository";
import { DatabaseCreationTimings, EventReport } from "./status-report";
import { endTracingForCluster } from "./tracer-config";
import * as util from "./util";
@@ -287,16 +281,36 @@ async function finalizeDatabaseCreation(
* the diff range information, or `undefined` if the feature is disabled.
*/
export async function setupDiffInformedQueryRun(
branches: PullRequestBranches,
logger: Logger,
): Promise<string | undefined> {
return await withGroupAsync(
"Generating diff range extension pack",
async () => {
// Only use precomputed diff ranges; never recompute here.
let diffRanges: DiffThunkRange[] | undefined;
try {
diffRanges = readDiffRangesJsonFile(logger);
} catch (e) {
logger.debug(
`Failed to read precomputed diff ranges: ${util.getErrorMessage(e)}`,
);
diffRanges = undefined;
}
if (diffRanges === undefined) {
logger.info(
"No precomputed diff ranges found; skipping diff-informed analysis stage.",
);
return undefined;
}
const fileCount = new Set(
diffRanges.filter((r) => r.path).map((r) => r.path),
).size;
logger.info(
`Calculating diff ranges for ${branches.base}...${branches.head}`,
`Using precomputed diff ranges (${diffRanges.length} ranges across ${fileCount} files).`,
);
const diffRanges = await getPullRequestEditedDiffRanges(branches, logger);
const packDir = writeDiffRangeDataExtensionPack(logger, diffRanges);
if (packDir === undefined) {
logger.warning(
@@ -313,185 +327,6 @@ export async function setupDiffInformedQueryRun(
);
}
/**
* Return the file line ranges that were added or modified in the pull request.
*
* @param branches The base and head branches of the pull request.
* @param logger
* @returns An array of tuples, where each tuple contains the absolute path of a
* file, the start line and the end line (both 1-based and inclusive) of an
* added or modified range in that file. Returns `undefined` if the action was
* not triggered by a pull request or if there was an error.
*/
async function getPullRequestEditedDiffRanges(
branches: PullRequestBranches,
logger: Logger,
): Promise<DiffThunkRange[] | undefined> {
const fileDiffs = await getFileDiffsWithBasehead(branches, logger);
if (fileDiffs === undefined) {
return undefined;
}
if (fileDiffs.length >= 300) {
// The "compare two commits" API returns a maximum of 300 changed files. If
// we see that many changed files, it is possible that there could be more,
// with the rest being truncated. In this case, we should not attempt to
// compute the diff ranges, as the result would be incomplete.
logger.warning(
`Cannot retrieve the full diff because there are too many ` +
`(${fileDiffs.length}) changed files in the pull request.`,
);
return undefined;
}
const results: DiffThunkRange[] = [];
for (const filediff of fileDiffs) {
const diffRanges = getDiffRanges(filediff, logger);
if (diffRanges === undefined) {
return undefined;
}
results.push(...diffRanges);
}
return results;
}
/**
* This interface is an abbreviated version of the file diff object returned by
* the GitHub API.
*/
interface FileDiff {
filename: string;
changes: number;
// A patch may be absent if the file is binary, if the file diff is too large,
// or if the file is unchanged.
patch?: string | undefined;
}
async function getFileDiffsWithBasehead(
branches: PullRequestBranches,
logger: Logger,
): Promise<FileDiff[] | undefined> {
// Check CODE_SCANNING_REPOSITORY first. If it is empty or not set, fall back
// to GITHUB_REPOSITORY.
const repositoryNwo = getRepositoryNwoFromEnv(
"CODE_SCANNING_REPOSITORY",
"GITHUB_REPOSITORY",
);
const basehead = `${branches.base}...${branches.head}`;
try {
const response = await getApiClient().rest.repos.compareCommitsWithBasehead(
{
owner: repositoryNwo.owner,
repo: repositoryNwo.repo,
basehead,
per_page: 1,
},
);
logger.debug(
`Response from compareCommitsWithBasehead(${basehead}):` +
`\n${JSON.stringify(response, null, 2)}`,
);
return response.data.files;
} catch (error: any) {
if (error.status) {
logger.warning(`Error retrieving diff ${basehead}: ${error.message}`);
logger.debug(
`Error running compareCommitsWithBasehead(${basehead}):` +
`\nRequest: ${JSON.stringify(error.request, null, 2)}` +
`\nError Response: ${JSON.stringify(error.response, null, 2)}`,
);
return undefined;
} else {
throw error;
}
}
}
function getDiffRanges(
fileDiff: FileDiff,
logger: Logger,
): DiffThunkRange[] | undefined {
// Diff-informed queries expect the file path to be absolute. CodeQL always
// uses forward slashes as the path separator, so on Windows we need to
// replace any backslashes with forward slashes.
const filename = path
.join(getRequiredInput("checkout_path"), fileDiff.filename)
.replaceAll(path.sep, "/");
if (fileDiff.patch === undefined) {
if (fileDiff.changes === 0) {
// There are situations where a changed file legitimately has no diff.
// For example, the file may be a binary file, or that the file may have
// been renamed with no changes to its contents. In these cases, the
// file would be reported as having 0 changes, and we can return an empty
// array to indicate no diff range in this file.
return [];
}
// If a file is reported to have nonzero changes but no patch, that may be
// due to the file diff being too large. In this case, we should fall back
// to a special diff range that covers the entire file.
return [
{
path: filename,
startLine: 0,
endLine: 0,
},
];
}
// The 1-based file line number of the current line
let currentLine = 0;
// The 1-based file line number that starts the current range of added lines
let additionRangeStartLine: number | undefined = undefined;
const diffRanges: DiffThunkRange[] = [];
const diffLines = fileDiff.patch.split("\n");
// Adding a fake context line at the end ensures that the following loop will
// always terminate the last range of added lines.
diffLines.push(" ");
for (const diffLine of diffLines) {
if (diffLine.startsWith("-")) {
// Ignore deletions completely -- we do not even want to consider them when
// calculating consecutive ranges of added lines.
continue;
}
if (diffLine.startsWith("+")) {
if (additionRangeStartLine === undefined) {
additionRangeStartLine = currentLine;
}
currentLine++;
continue;
}
if (additionRangeStartLine !== undefined) {
// Any line that does not start with a "+" or "-" terminates the current
// range of added lines.
diffRanges.push({
path: filename,
startLine: additionRangeStartLine,
endLine: currentLine - 1,
});
additionRangeStartLine = undefined;
}
if (diffLine.startsWith("@@ ")) {
// A new hunk header line resets the current line number.
const match = diffLine.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
if (match === null) {
logger.warning(
`Cannot parse diff hunk header for ${fileDiff.filename}: ${diffLine}`,
);
return undefined;
}
currentLine = parseInt(match[1], 10);
continue;
}
if (diffLine.startsWith(" ")) {
// An unchanged context line advances the current line number.
currentLine++;
continue;
}
}
return diffRanges;
}
/**
* Create an extension pack in the temporary directory that contains the file
* line ranges that were added or modified in the pull request.
@@ -550,14 +385,22 @@ extensions:
`;
let data = ranges
.map(
(range) =>
// Using yaml.dump() with `forceQuotes: true` ensures that all special
// characters are escaped, and that the path is always rendered as a
// quoted string on a single line.
` - [${yaml.dump(range.path, { forceQuotes: true }).trim()}, ` +
`${range.startLine}, ${range.endLine}]\n`,
)
.map((range) => {
// Diff-informed queries expect the file path to be absolute. CodeQL always
// uses forward slashes as the path separator, so on Windows we need to
// replace any backslashes with forward slashes.
const filename = path
.join(getRequiredInput("checkout_path"), range.path)
.replaceAll(path.sep, "/");
// Using yaml.dump() with `forceQuotes: true` ensures that all special
// characters are escaped, and that the path is always rendered as a
// quoted string on a single line.
return (
` - [${yaml.dump(filename, { forceQuotes: true }).trim()}, ` +
`${range.startLine}, ${range.endLine}]\n`
);
})
.join("");
if (!data) {
// Ensure that the data extension is not empty, so that a pull request with
@@ -572,10 +415,6 @@ extensions:
`Wrote pr-diff-range extension pack to ${extensionFilePath}:\n${extensionContents}`,
);
// Write the diff ranges to a JSON file, for action-side alert filtering by the
// upload-lib module.
writeDiffRangesJsonFile(logger, ranges);
return diffRangeDir;
}
@@ -922,7 +761,3 @@ export async function warnIfGoInstalledAfterInit(
}
}
}
export const exportedForTesting = {
getDiffRanges,
};
+198 -1
View File
@@ -4,7 +4,10 @@ import * as sinon from "sinon";
import * as actionsUtil from "./actions-util";
import type { PullRequestBranches } from "./actions-util";
import * as apiClient from "./api-client";
import { shouldPerformDiffInformedAnalysis } from "./diff-informed-analysis-utils";
import {
shouldPerformDiffInformedAnalysis,
exportedForTesting,
} from "./diff-informed-analysis-utils";
import { Feature, Features } from "./feature-flags";
import { getRunnerLogger } from "./logging";
import { parseRepositoryNwo } from "./repository";
@@ -183,3 +186,197 @@ test(
},
false,
);
function runGetDiffRanges(changes: number, patch: string[] | undefined): any {
return exportedForTesting.getDiffRanges(
{
filename: "test.txt",
changes,
patch: patch?.join("\n"),
},
getRunnerLogger(true),
);
}
test("getDiffRanges: file unchanged", async (t) => {
const diffRanges = runGetDiffRanges(0, undefined);
t.deepEqual(diffRanges, []);
});
test("getDiffRanges: file diff too large", async (t) => {
const diffRanges = runGetDiffRanges(1000000, undefined);
t.deepEqual(diffRanges, [
{
path: "test.txt",
startLine: 0,
endLine: 0,
},
]);
});
test("getDiffRanges: diff thunk with single addition range", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,6 +50,8 @@",
" a",
" b",
" c",
"+1",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "test.txt",
startLine: 53,
endLine: 54,
},
]);
});
test("getDiffRanges: diff thunk with single deletion range", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,8 +50,6 @@",
" a",
" b",
" c",
"-1",
"-2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, []);
});
test("getDiffRanges: diff thunk with single update range", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,7 +50,7 @@",
" a",
" b",
" c",
"-1",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "test.txt",
startLine: 53,
endLine: 53,
},
]);
});
test("getDiffRanges: diff thunk with addition ranges", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,7 +50,9 @@",
" a",
" b",
" c",
"+1",
" c",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "test.txt",
startLine: 53,
endLine: 53,
},
{
path: "test.txt",
startLine: 55,
endLine: 55,
},
]);
});
test("getDiffRanges: diff thunk with mixed ranges", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,7 +50,7 @@",
" a",
" b",
" c",
"-1",
" d",
"-2",
"+3",
" e",
" f",
"+4",
"+5",
" g",
" h",
" i",
]);
t.deepEqual(diffRanges, [
{
path: "test.txt",
startLine: 54,
endLine: 54,
},
{
path: "test.txt",
startLine: 57,
endLine: 58,
},
]);
});
test("getDiffRanges: multiple diff thunks", async (t) => {
const diffRanges = runGetDiffRanges(2, [
"@@ -30,6 +50,8 @@",
" a",
" b",
" c",
"+1",
"+2",
" d",
" e",
" f",
"@@ -130,6 +150,8 @@",
" a",
" b",
" c",
"+1",
"+2",
" d",
" e",
" f",
]);
t.deepEqual(diffRanges, [
{
path: "test.txt",
startLine: 53,
endLine: 54,
},
{
path: "test.txt",
startLine: 153,
endLine: 154,
},
]);
});
test("getDiffRanges: no diff context lines", async (t) => {
const diffRanges = runGetDiffRanges(2, ["@@ -30 +50,2 @@", "+1", "+2"]);
t.deepEqual(diffRanges, [
{
path: "test.txt",
startLine: 50,
endLine: 51,
},
]);
});
test("getDiffRanges: malformed thunk header", async (t) => {
const diffRanges = runGetDiffRanges(2, ["@@ 30 +50,2 @@", "+1", "+2"]);
t.deepEqual(diffRanges, undefined);
});
+180 -1
View File
@@ -3,12 +3,25 @@ import * as path from "path";
import * as actionsUtil from "./actions-util";
import type { PullRequestBranches } from "./actions-util";
import { getGitHubVersion } from "./api-client";
import { getApiClient, getGitHubVersion } from "./api-client";
import type { CodeQL } from "./codeql";
import { Feature, FeatureEnablement } from "./feature-flags";
import { Logger } from "./logging";
import { getRepositoryNwoFromEnv } from "./repository";
import { GitHubVariant, satisfiesGHESVersion } from "./util";
/**
* This interface is an abbreviated version of the file diff object returned by
* the GitHub API.
*/
interface FileDiff {
filename: string;
changes: number;
// A patch may be absent if the file is binary, if the file diff is too large,
// or if the file is unchanged.
patch?: string | undefined;
}
/**
* Check if the action should perform diff-informed analysis.
*/
@@ -93,3 +106,169 @@ export function readDiffRangesJsonFile(
);
return JSON.parse(jsonContents) as DiffThunkRange[];
}
/**
* Return the file line ranges that were added or modified in the pull request.
*
* @param branches The base and head branches of the pull request.
* @param logger
* @returns An array of tuples, where each tuple contains the absolute path of a
* file, the start line and the end line (both 1-based and inclusive) of an
* added or modified range in that file. Returns `undefined` if the action was
* not triggered by a pull request or if there was an error.
*/
export async function getPullRequestEditedDiffRanges(
branches: PullRequestBranches,
logger: Logger,
): Promise<DiffThunkRange[] | undefined> {
const fileDiffs = await getFileDiffsWithBasehead(branches, logger);
if (fileDiffs === undefined) {
return undefined;
}
if (fileDiffs.length >= 300) {
// The "compare two commits" API returns a maximum of 300 changed files. If
// we see that many changed files, it is possible that there could be more,
// with the rest being truncated. In this case, we should not attempt to
// compute the diff ranges, as the result would be incomplete.
logger.warning(
`Cannot retrieve the full diff because there are too many ` +
`(${fileDiffs.length}) changed files in the pull request.`,
);
return undefined;
}
const results: DiffThunkRange[] = [];
for (const filediff of fileDiffs) {
const diffRanges = getDiffRanges(filediff, logger);
if (diffRanges === undefined) {
return undefined;
}
results.push(...diffRanges);
}
return results;
}
async function getFileDiffsWithBasehead(
branches: PullRequestBranches,
logger: Logger,
): Promise<FileDiff[] | undefined> {
// Check CODE_SCANNING_REPOSITORY first. If it is empty or not set, fall back
// to GITHUB_REPOSITORY.
const repositoryNwo = getRepositoryNwoFromEnv(
"CODE_SCANNING_REPOSITORY",
"GITHUB_REPOSITORY",
);
const basehead = `${branches.base}...${branches.head}`;
try {
const response = await getApiClient().rest.repos.compareCommitsWithBasehead(
{
owner: repositoryNwo.owner,
repo: repositoryNwo.repo,
basehead,
per_page: 1,
},
);
logger.debug(
`Response from compareCommitsWithBasehead(${basehead}):` +
`\n${JSON.stringify(response, null, 2)}`,
);
return response.data.files;
} catch (error: any) {
if (error.status) {
logger.warning(`Error retrieving diff ${basehead}: ${error.message}`);
logger.debug(
`Error running compareCommitsWithBasehead(${basehead}):` +
`\nRequest: ${JSON.stringify(error.request, null, 2)}` +
`\nError Response: ${JSON.stringify(error.response, null, 2)}`,
);
return undefined;
} else {
throw error;
}
}
}
function getDiffRanges(
fileDiff: FileDiff,
logger: Logger,
): DiffThunkRange[] | undefined {
const filename = fileDiff.filename;
if (fileDiff.patch === undefined) {
if (fileDiff.changes === 0) {
// There are situations where a changed file legitimately has no diff.
// For example, the file may be a binary file, or that the file may have
// been renamed with no changes to its contents. In these cases, the
// file would be reported as having 0 changes, and we can return an empty
// array to indicate no diff range in this file.
return [];
}
// If a file is reported to have nonzero changes but no patch, that may be
// due to the file diff being too large. In this case, we should fall back
// to a special diff range that covers the entire file.
return [
{
path: filename,
startLine: 0,
endLine: 0,
},
];
}
// The 1-based file line number of the current line
let currentLine = 0;
// The 1-based file line number that starts the current range of added lines
let additionRangeStartLine: number | undefined = undefined;
const diffRanges: DiffThunkRange[] = [];
const diffLines = fileDiff.patch.split("\n");
// Adding a fake context line at the end ensures that the following loop will
// always terminate the last range of added lines.
diffLines.push(" ");
for (const diffLine of diffLines) {
if (diffLine.startsWith("-")) {
// Ignore deletions completely -- we do not even want to consider them when
// calculating consecutive ranges of added lines.
continue;
}
if (diffLine.startsWith("+")) {
if (additionRangeStartLine === undefined) {
additionRangeStartLine = currentLine;
}
currentLine++;
continue;
}
if (additionRangeStartLine !== undefined) {
// Any line that does not start with a "+" or "-" terminates the current
// range of added lines.
diffRanges.push({
path: filename,
startLine: additionRangeStartLine,
endLine: currentLine - 1,
});
additionRangeStartLine = undefined;
}
if (diffLine.startsWith("@@ ")) {
// A new hunk header line resets the current line number.
const match = diffLine.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
if (match === null) {
logger.warning(
`Cannot parse diff hunk header for ${fileDiff.filename}: ${diffLine}`,
);
return undefined;
}
currentLine = parseInt(match[1], 10);
continue;
}
if (diffLine.startsWith(" ")) {
// An unchanged context line advances the current line number.
currentLine++;
continue;
}
}
return diffRanges;
}
export const exportedForTesting = {
getDiffRanges,
};
+44 -1
View File
@@ -34,6 +34,11 @@ import {
logUnwrittenDiagnostics,
makeDiagnostic,
} from "./diagnostics";
import {
getPullRequestEditedDiffRanges,
writeDiffRangesJsonFile,
getDiffInformedAnalysisBranches,
} from "./diff-informed-analysis-utils";
import { EnvVar } from "./environment";
import { Feature, Features } from "./feature-flags";
import { loadPropertiesFromApi } from "./feature-flags/properties";
@@ -46,7 +51,7 @@ import {
runDatabaseInitCluster,
} from "./init";
import { KnownLanguage } from "./languages";
import { getActionsLogger, Logger } from "./logging";
import { getActionsLogger, Logger, withGroupAsync } from "./logging";
import {
downloadOverlayBaseDatabaseFromCache,
OverlayBaseDatabaseDownloadStats,
@@ -358,6 +363,7 @@ async function run() {
});
await checkInstallPython311(config.languages, codeql);
await computeAndPersistDiffRanges(codeql, features, logger);
} catch (unwrappedError) {
const error = wrapError(unwrappedError);
core.setFailed(error.message);
@@ -770,6 +776,43 @@ async function run() {
);
}
/**
* Compute and persist diff ranges when diff-informed analysis
* is enabled (feature flag + PR context). This writes the standard pr-diff-range.json
* file for later reuse in the analyze step. Failures are logged but non-fatal.
*/
async function computeAndPersistDiffRanges(
codeql: CodeQL,
features: Features,
logger: Logger,
): Promise<void> {
try {
await withGroupAsync("Compute PR diff ranges", async () => {
const branches = await getDiffInformedAnalysisBranches(
codeql,
features,
logger,
);
if (!branches) {
return;
}
const ranges = await getPullRequestEditedDiffRanges(branches, logger);
if (ranges === undefined) {
return;
}
writeDiffRangesJsonFile(logger, ranges);
const distinctFiles = new Set(ranges.map((r) => r.path)).size;
logger.info(
`Persisted ${ranges.length} diff range(s) across ${distinctFiles} file(s).`,
);
});
} catch (e) {
logger.warning(
`Failed to compute and persist PR diff ranges: ${getErrorMessage(e)}`,
);
}
}
function getTrapCachingEnabled(): boolean {
// If the workflow specified something always respect that
const trapCaching = getOptionalInput("trap-caching");
+47
View File
@@ -11,6 +11,8 @@ import * as gitUtils from "./git-utils";
import { getRunnerLogger } from "./logging";
import {
downloadOverlayBaseDatabaseFromCache,
getCacheRestoreKeyPrefix,
getCacheSaveKey,
OverlayDatabaseMode,
writeBaseDatabaseOidsFile,
writeOverlayChangesFile,
@@ -261,3 +263,48 @@ test(
},
false,
);
test("overlay-base database cache keys remain stable", async (t) => {
const logger = getRunnerLogger(true);
const config = createTestConfig({ languages: ["python", "javascript"] });
const codeQlVersion = "2.23.0";
const commitOid = "abc123def456";
sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
sinon.stub(gitUtils, "getCommitOid").resolves(commitOid);
sinon.stub(actionsUtil, "getWorkflowRunID").returns(12345);
sinon.stub(actionsUtil, "getWorkflowRunAttempt").returns(1);
const saveKey = await getCacheSaveKey(
config,
codeQlVersion,
"checkout-path",
logger,
);
const expectedSaveKey =
"codeql-overlay-base-database-1-c5666c509a2d9895-javascript_python-2.23.0-abc123def456-12345-1";
t.is(
saveKey,
expectedSaveKey,
"Cache save key changed unexpectedly. " +
"This may indicate breaking changes in the cache key generation logic.",
);
const restoreKeyPrefix = await getCacheRestoreKeyPrefix(
config,
codeQlVersion,
);
const expectedRestoreKeyPrefix =
"codeql-overlay-base-database-1-c5666c509a2d9895-javascript_python-2.23.0-";
t.is(
restoreKeyPrefix,
expectedRestoreKeyPrefix,
"Cache restore key prefix changed unexpectedly. " +
"This may indicate breaking changes in the cache key generation logic.",
);
t.true(
saveKey.startsWith(restoreKeyPrefix),
`Expected save key "${saveKey}" to start with restore key prefix "${restoreKeyPrefix}"`,
);
});
+22 -4
View File
@@ -4,13 +4,19 @@ import * as path from "path";
import * as actionsCache from "@actions/cache";
import { getRequiredInput, getTemporaryDirectory } from "./actions-util";
import {
getRequiredInput,
getTemporaryDirectory,
getWorkflowRunAttempt,
getWorkflowRunID,
} from "./actions-util";
import { getAutomationID } from "./api-client";
import { type CodeQL } from "./codeql";
import { type Config } from "./config-utils";
import { getCommitOid, getFileOidsUnderPath } from "./git-utils";
import { Logger, withGroupAsync } from "./logging";
import {
getErrorMessage,
isInTestMode,
tryGetFolderBytes,
waitForResultWithTimeLimit,
@@ -266,6 +272,7 @@ export async function uploadOverlayBaseDatabaseToCache(
config,
codeQlVersion,
checkoutPath,
logger,
);
logger.info(
`Uploading overlay-base database to Actions cache with key ${cacheSaveKey}`,
@@ -443,17 +450,28 @@ export async function downloadOverlayBaseDatabaseFromCache(
* The key consists of the restore key prefix (which does not include the
* commit SHA) and the commit SHA of the current checkout.
*/
async function getCacheSaveKey(
export async function getCacheSaveKey(
config: Config,
codeQlVersion: string,
checkoutPath: string,
logger: Logger,
): Promise<string> {
let runId = 1;
let attemptId = 1;
try {
runId = getWorkflowRunID();
attemptId = getWorkflowRunAttempt();
} catch (e) {
logger.warning(
`Failed to get workflow run ID or attempt ID. Reason: ${getErrorMessage(e)}`,
);
}
const sha = await getCommitOid(checkoutPath);
const restoreKeyPrefix = await getCacheRestoreKeyPrefix(
config,
codeQlVersion,
);
return `${restoreKeyPrefix}${sha}`;
return `${restoreKeyPrefix}${sha}-${runId}-${attemptId}`;
}
/**
@@ -470,7 +488,7 @@ async function getCacheSaveKey(
* not include the commit SHA. This allows us to restore the most recent
* compatible overlay-base database.
*/
async function getCacheRestoreKeyPrefix(
export async function getCacheRestoreKeyPrefix(
config: Config,
codeQlVersion: string,
): Promise<string> {
+6 -9
View File
@@ -748,7 +748,7 @@ export async function writePostProcessedFiles(
) {
// If there's an explicit input, use that. Otherwise, use the value from the environment variable.
const outputPath = pathInput || util.getOptionalEnvVar(EnvVar.SARIF_DUMP_DIR);
logger.info(`Post-processed SARIF output path: ${outputPath}`);
// If we have a non-empty output path, write the SARIF file to it.
if (outputPath !== undefined) {
dumpSarifFile(
@@ -1140,10 +1140,9 @@ function filterAlertsByDiffRange(logger: Logger, sarif: SarifFile): SarifFile {
return sarif;
}
const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
for (const run of sarif.runs) {
if (run.results) {
const preAlertCount = run.results.length;
run.results = run.results.filter((result) => {
const locations = [
...(result.locations || []).map((loc) => loc.physicalLocation),
@@ -1156,11 +1155,6 @@ function filterAlertsByDiffRange(logger: Logger, sarif: SarifFile): SarifFile {
if (!locationUri || locationStartLine === undefined) {
return false;
}
// CodeQL always uses forward slashes as the path separator, so on Windows we
// need to replace any backslashes with forward slashes.
const locationPath = path
.join(checkoutPath, locationUri)
.replaceAll(path.sep, "/");
// Alert filtering here replicates the same behavior as the restrictAlertsTo
// extensible predicate in CodeQL. See the restrictAlertsTo documentation
// https://codeql.github.com/codeql-standard-libraries/csharp/codeql/util/AlertFiltering.qll/predicate.AlertFiltering$restrictAlertsTo.3.html
@@ -1168,13 +1162,16 @@ function filterAlertsByDiffRange(logger: Logger, sarif: SarifFile): SarifFile {
// of an alert location.
return diffRanges.some(
(range) =>
range.path === locationPath &&
range.path === locationUri &&
((range.startLine <= locationStartLine &&
range.endLine >= locationStartLine) ||
(range.startLine === 0 && range.endLine === 0)),
);
});
});
const postAlertCount = run.results.length;
logger.info(`Filtered ${preAlertCount - postAlertCount} alerts based on diff range.`);
}
}
-5
View File
@@ -32,7 +32,6 @@ export async function postProcessAndUploadSarif(
category?: string,
postProcessedOutputPath?: string,
): Promise<UploadSarifResults> {
logger.info("XXX at upload-sarif.ts|postProcessAndUploadSarif");
const sarifGroups = await upload_lib.getGroupedSarifFilePaths(
logger,
sarifPath,
@@ -42,10 +41,6 @@ export async function postProcessAndUploadSarif(
for (const [analysisKind, sarifFiles] of unsafeEntriesInvariant(
sarifGroups,
)) {
logger.info(
`XXX at upload-sarif.ts|postProcessAndUploadSarif loop for analysisKind: ${analysisKind}`,
);
const analysisConfig = analyses.getAnalysisConfig(analysisKind);
const postProcessingResults = await upload_lib.postProcessSarifFiles(
logger,