Skip to content

Commit 9d16bed

Browse files
committed
refactor: fall back to non-overlay analysis when diff-informed analysis is unavailable
1 parent ee09113 commit 9d16bed

File tree

6 files changed

+221
-86
lines changed

6 files changed

+221
-86
lines changed

lib/init-action.js

Lines changed: 59 additions & 36 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/config-utils.ts

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ import {
3131
addNoLanguageDiagnostic,
3232
makeTelemetryDiagnostic,
3333
} from "./diagnostics";
34-
import { shouldPerformDiffInformedAnalysis } from "./diff-informed-analysis-utils";
34+
import {
35+
computeAndPersistDiffRanges,
36+
getDiffInformedAnalysisBranches,
37+
} from "./diff-informed-analysis-utils";
3538
import { EnvVar } from "./environment";
3639
import * as errorMessages from "./error-messages";
3740
import { Feature, FeatureEnablement } from "./feature-flags";
@@ -49,8 +52,12 @@ import {
4952
isAnalyzingDefaultBranch,
5053
} from "./git-utils";
5154
import { KnownLanguage, Language } from "./languages";
52-
import { Logger } from "./logging";
53-
import { CODEQL_OVERLAY_MINIMUM_VERSION, OverlayDatabaseMode } from "./overlay";
55+
import { Logger, withGroupAsync } from "./logging";
56+
import {
57+
CODEQL_OVERLAY_MINIMUM_VERSION,
58+
OverlayDatabaseMode,
59+
revertOverlayModeIfDiffInformedUnavailable,
60+
} from "./overlay";
5461
import {
5562
addOverlayDisablementDiagnostics,
5663
OverlayDisabledReason,
@@ -1229,19 +1236,55 @@ export async function initConfig(
12291236
);
12301237
}
12311238

1232-
if (
1233-
config.overlayDatabaseMode === OverlayDatabaseMode.Overlay ||
1234-
(await shouldPerformDiffInformedAnalysis(
1239+
let shouldRunDiffInformedAnalysis = false;
1240+
let diffInformedRangesPrepared = false;
1241+
try {
1242+
const diffInformedAnalysisBranches = await getDiffInformedAnalysisBranches(
12351243
inputs.codeql,
12361244
inputs.features,
12371245
logger,
1238-
))
1246+
);
1247+
shouldRunDiffInformedAnalysis = diffInformedAnalysisBranches !== undefined;
1248+
1249+
if (diffInformedAnalysisBranches) {
1250+
await withGroupAsync("Computing PR diff ranges", async () => {
1251+
try {
1252+
diffInformedRangesPrepared = await computeAndPersistDiffRanges(
1253+
diffInformedAnalysisBranches,
1254+
logger,
1255+
);
1256+
} catch (e) {
1257+
logger.warning(
1258+
`Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}`,
1259+
);
1260+
}
1261+
});
1262+
}
1263+
} catch (e) {
1264+
logger.warning(
1265+
`Failed to determine diff-informed analysis availability: ${getErrorMessage(e)}`,
1266+
);
1267+
// Treat errors conservatively so overlay falls back if diff-informed
1268+
// availability could not be checked.
1269+
shouldRunDiffInformedAnalysis = true;
1270+
}
1271+
1272+
if (
1273+
config.overlayDatabaseMode === OverlayDatabaseMode.Overlay ||
1274+
shouldRunDiffInformedAnalysis
12391275
) {
12401276
config.extraQueryExclusions.push({
12411277
exclude: { tags: "exclude-from-incremental" },
12421278
});
12431279
}
12441280

1281+
revertOverlayModeIfDiffInformedUnavailable(
1282+
config,
1283+
shouldRunDiffInformedAnalysis,
1284+
diffInformedRangesPrepared,
1285+
logger,
1286+
);
1287+
12451288
if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) {
12461289
const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime(
12471290
inputs.codeql,

src/diff-informed-analysis-utils.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,33 @@ export async function getPullRequestEditedDiffRanges(
151151
return results;
152152
}
153153

154+
/**
155+
* Compute and persist the diff ranges for a pull request. This fetches the
156+
* diff from the GitHub API and writes it to the diff ranges JSON file so that
157+
* CodeQL can use it for diff-informed analysis.
158+
*
159+
* @param branches The base and head branches of the pull request, as returned
160+
* by `getDiffInformedAnalysisBranches`.
161+
* @param logger
162+
* @returns `true` if the diff ranges were successfully computed and persisted,
163+
* otherwise `false`.
164+
*/
165+
export async function computeAndPersistDiffRanges(
166+
branches: PullRequestBranches,
167+
logger: Logger,
168+
): Promise<boolean> {
169+
const ranges = await getPullRequestEditedDiffRanges(branches, logger);
170+
if (ranges === undefined) {
171+
return false;
172+
}
173+
writeDiffRangesJsonFile(logger, ranges);
174+
const distinctFiles = new Set(ranges.map((r) => r.path)).size;
175+
logger.info(
176+
`Persisted ${ranges.length} diff range(s) across ${distinctFiles} file(s).`,
177+
);
178+
return true;
179+
}
180+
154181
async function getFileDiffsWithBasehead(
155182
branches: PullRequestBranches,
156183
logger: Logger,

src/init-action.ts

Lines changed: 1 addition & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,6 @@ import {
3737
makeDiagnostic,
3838
makeTelemetryDiagnostic,
3939
} from "./diagnostics";
40-
import {
41-
getDiffInformedAnalysisBranches,
42-
getPullRequestEditedDiffRanges,
43-
writeDiffRangesJsonFile,
44-
} from "./diff-informed-analysis-utils";
4540
import { EnvVar } from "./environment";
4641
import { Feature, FeatureEnablement, initFeatures } from "./feature-flags";
4742
import {
@@ -59,7 +54,7 @@ import {
5954
runDatabaseInitCluster,
6055
} from "./init";
6156
import { JavaEnvVars, KnownLanguage } from "./languages";
62-
import { getActionsLogger, Logger, withGroupAsync } from "./logging";
57+
import { getActionsLogger, Logger } from "./logging";
6358
import {
6459
downloadOverlayBaseDatabaseFromCache,
6560
OverlayBaseDatabaseDownloadStats,
@@ -427,7 +422,6 @@ async function run(startedAt: Date) {
427422
}
428423

429424
await checkInstallPython311(config.languages, codeql);
430-
await computeAndPersistDiffRanges(codeql, features, logger);
431425
} catch (unwrappedError) {
432426
const error = wrapError(unwrappedError);
433427
core.setFailed(error.message);
@@ -818,42 +812,6 @@ async function loadRepositoryProperties(
818812
}
819813
}
820814

821-
/**
822-
* Compute and persist diff ranges when diff-informed analysis is enabled
823-
* (feature flag + PR context). This writes the standard pr-diff-range.json
824-
* file for later reuse in the analyze step. Failures are logged but non-fatal.
825-
*/
826-
async function computeAndPersistDiffRanges(
827-
codeql: CodeQL,
828-
features: FeatureEnablement,
829-
logger: Logger,
830-
): Promise<void> {
831-
await withGroupAsync("Computing PR diff ranges", async () => {
832-
try {
833-
const branches = await getDiffInformedAnalysisBranches(
834-
codeql,
835-
features,
836-
logger,
837-
);
838-
if (!branches) {
839-
return;
840-
}
841-
const ranges = await getPullRequestEditedDiffRanges(branches, logger);
842-
if (ranges === undefined) {
843-
return;
844-
}
845-
writeDiffRangesJsonFile(logger, ranges);
846-
const distinctFiles = new Set(ranges.map((r) => r.path)).size;
847-
logger.info(
848-
`Persisted ${ranges.length} diff range(s) across ${distinctFiles} file(s).`,
849-
);
850-
} catch (e) {
851-
logger.warning(
852-
`Failed to compute and persist PR diff ranges: ${getErrorMessage(e)}`,
853-
);
854-
}
855-
});
856-
}
857815
async function recordZstdAvailability(
858816
config: configUtils.Config,
859817
zstdAvailability: ZstdAvailability,

src/overlay/index.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
getCacheRestoreKeyPrefix,
2525
getCacheSaveKey,
2626
OverlayDatabaseMode,
27+
revertOverlayModeIfDiffInformedUnavailable,
2728
writeBaseDatabaseOidsFile,
2829
writeOverlayChangesFile,
2930
} from ".";
@@ -602,3 +603,54 @@ test.serial("overlay-base database cache keys remain stable", async (t) => {
602603
`Expected save key "${saveKey}" to start with restore key prefix "${restoreKeyPrefix}"`,
603604
);
604605
});
606+
607+
test.serial(
608+
"revertOverlayModeIfDiffInformedUnavailable: no-op when mode is not Overlay",
609+
(t) => {
610+
const config = createTestConfig({});
611+
config.overlayDatabaseMode = OverlayDatabaseMode.None;
612+
const logger = getRunnerLogger(true);
613+
614+
revertOverlayModeIfDiffInformedUnavailable(config, true, false, logger);
615+
616+
t.is(config.overlayDatabaseMode, OverlayDatabaseMode.None);
617+
},
618+
);
619+
620+
test.serial(
621+
"revertOverlayModeIfDiffInformedUnavailable: no-op when diff-informed analysis is available",
622+
(t) => {
623+
const config = createTestConfig({});
624+
config.overlayDatabaseMode = OverlayDatabaseMode.Overlay;
625+
const logger = getRunnerLogger(true);
626+
627+
revertOverlayModeIfDiffInformedUnavailable(config, true, true, logger);
628+
629+
t.is(config.overlayDatabaseMode, OverlayDatabaseMode.Overlay);
630+
},
631+
);
632+
633+
test.serial(
634+
"revertOverlayModeIfDiffInformedUnavailable: reverts to None when diff-informed analysis is unavailable",
635+
(t) => {
636+
const config = createTestConfig({});
637+
config.overlayDatabaseMode =
638+
OverlayDatabaseMode.Overlay as OverlayDatabaseMode;
639+
const logger = getRunnerLogger(true);
640+
641+
revertOverlayModeIfDiffInformedUnavailable(config, true, false, logger);
642+
t.is(config.overlayDatabaseMode, OverlayDatabaseMode.None);
643+
},
644+
);
645+
646+
test.serial(
647+
"revertOverlayModeIfDiffInformedUnavailable: no-op when diff-informed analysis is disabled",
648+
(t) => {
649+
const config = createTestConfig({});
650+
config.overlayDatabaseMode = OverlayDatabaseMode.Overlay;
651+
const logger = getRunnerLogger(true);
652+
653+
revertOverlayModeIfDiffInformedUnavailable(config, false, false, logger);
654+
t.is(config.overlayDatabaseMode, OverlayDatabaseMode.Overlay);
655+
},
656+
);

0 commit comments

Comments
 (0)