robojerk/codeql-action

Address review comments from @henrymercer

Browse source
This commit is contained in:
Edoardo Pirovano 2022-08-09 18:37:22 +01:00
parent 8f867dcb21
commit 6df93613d7
No known key found for this signature in database
GPG key ID: 047556B5D93FFE28
15 changed files with 125 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

16
src/config-utils.ts
View file

@ -151,7 +151,7 @@ export interface Config {
injectedMlQueries: boolean;
/**
* Partial map from languages to locations of TRAP caches for that language.
* A key being omitted means TRAP caching should not be used for that language.
* If a key is omitted, then TRAP caching should not be used for that language.
*/
trapCaches: Partial<Record<Language, string>>;
}
@ -884,7 +884,7 @@ export async function getDefaultConfig(
queriesInput: string | undefined,
packsInput: string | undefined,
dbLocation: string | undefined,
trapCaching: boolean,
trapCachingEnabled: boolean,
debugMode: boolean,
debugArtifactName: string,
debugDatabaseName: string,
@ -944,7 +944,7 @@ export async function getDefaultConfig(
debugArtifactName,
debugDatabaseName,
injectedMlQueries,
trapCaches: trapCaching
trapCaches: trapCachingEnabled
? await downloadTrapCaches(codeQL, languages, logger)
: {},
};
@ -959,7 +959,7 @@ async function loadConfig(
packsInput: string | undefined,
configFile: string,
dbLocation: string | undefined,
trapCaching: boolean,
trapCachingEnabled: boolean,
debugMode: boolean,
debugArtifactName: string,
debugDatabaseName: string,
@ -1128,7 +1128,7 @@ async function loadConfig(
debugArtifactName,
debugDatabaseName,
injectedMlQueries,
trapCaches: trapCaching
trapCaches: trapCachingEnabled
? await downloadTrapCaches(codeQL, languages, logger)
: {},
};
@ -1372,7 +1372,7 @@ export async function initConfig(
packsInput: string | undefined,
configFile: string | undefined,
dbLocation: string | undefined,
trapCaching: boolean,
trapCachingEnabled: boolean,
debugMode: boolean,
debugArtifactName: string,
debugDatabaseName: string,
@ -1395,7 +1395,7 @@ export async function initConfig(
queriesInput,
packsInput,
dbLocation,
trapCaching,
trapCachingEnabled,
debugMode,
debugArtifactName,
debugDatabaseName,
@ -1415,7 +1415,7 @@ export async function initConfig(
packsInput,
configFile,
dbLocation,
trapCaching,
trapCachingEnabled,
debugMode,
debugArtifactName,
debugDatabaseName,

9
src/init-action.ts
View file

@ -300,11 +300,12 @@ async function run() {
await sendSuccessStatusReport(startedAt, config, toolsVersion);
}
function getTrapCachingEnabled(featureFlags: FeatureFlags): Promise<boolean> {
async function getTrapCachingEnabled(
featureFlags: FeatureFlags
): Promise<boolean> {
const trapCaching = getOptionalInput("trap-caching");
if (trapCaching !== undefined)
return Promise.resolve(trapCaching.toLowerCase() === "true");
return featureFlags.getValue(FeatureFlag.TrapCachingEnabled);
if (trapCaching !== undefined) return trapCaching === "true";
return await featureFlags.getValue(FeatureFlag.TrapCachingEnabled);
}
async function runWrapper() {

4
src/init.ts
View file

@ -42,7 +42,7 @@ export async function initConfig(
packsInput: string | undefined,
configFile: string | undefined,
dbLocation: string | undefined,
trapCaching: boolean,
trapCachingEnabled: boolean,
debugMode: boolean,
debugArtifactName: string,
debugDatabaseName: string,
@ -62,7 +62,7 @@ export async function initConfig(
packsInput,
configFile,
dbLocation,
trapCaching,
trapCachingEnabled,
debugMode,
debugArtifactName,
debugDatabaseName,

11
src/trap-caching.test.ts
View file

@ -63,7 +63,7 @@ const stubCodeql = setCodeQL({
},
});
const stubConfig: Config = {
const testConfigWithoutTmpDir: Config = {
languages: [Language.javascript, Language.cpp],
queries: {},
pathsIgnore: [],
@ -85,7 +85,7 @@ const stubConfig: Config = {
},
};
function getTestConfig(tmpDir: string): configUtils.Config {
function getTestConfigWithTempDir(tmpDir: string): configUtils.Config {
return {
languages: [Language.javascript, Language.ruby],
queries: {},
@ -110,7 +110,7 @@ function getTestConfig(tmpDir: string): configUtils.Config {
test("check flags for JS, analyzing default branch", async (t) => {
await util.withTmpDir(async (tmpDir) => {
const config = getTestConfig(tmpDir);
const config = getTestConfigWithTempDir(tmpDir);
sinon.stub(actionsUtil, "isAnalyzingDefaultBranch").resolves(true);
const result = await getTrapCachingExtractorConfigArgsForLang(
config,
@ -126,7 +126,7 @@ test("check flags for JS, analyzing default branch", async (t) => {
test("check flags for all, not analyzing default branch", async (t) => {
await util.withTmpDir(async (tmpDir) => {
const config = getTestConfig(tmpDir);
const config = getTestConfigWithTempDir(tmpDir);
sinon.stub(actionsUtil, "isAnalyzingDefaultBranch").resolves(false);
const result = await getTrapCachingExtractorConfigArgs(config);
t.deepEqual(result, [
@ -157,7 +157,7 @@ test("upload cache key contains right fields", async (t) => {
sinon.stub(actionsUtil, "isAnalyzingDefaultBranch").resolves(true);
const stubSave = sinon.stub(cache, "saveCache");
process.env.GITHUB_SHA = "somesha";
await uploadTrapCaches(stubCodeql, stubConfig, logger);
await uploadTrapCaches(stubCodeql, testConfigWithoutTmpDir, logger);
t.assert(
stubSave.calledOnceWith(
sinon.match.array.contains(["/some/cache/dir"]),
@ -177,6 +177,7 @@ test("download cache looks for the right key and creates dir", async (t) => {
sinon.stub(actionsUtil, "isAnalyzingDefaultBranch").resolves(false);
const stubRestore = sinon.stub(cache, "restoreCache").resolves("found");
const eventFile = path.resolve(tmpDir, "event.json");
process.env.GITHUB_EVENT_NAME = "pull_request";
process.env.GITHUB_EVENT_PATH = eventFile;
fs.writeFileSync(
eventFile,

60
src/trap-caching.ts
View file

@ -17,15 +17,18 @@ import { codeQlVersionAbove } from "./util";
// goes into the cache key.
const CACHE_VERSION = 1;
// This constant sets the size of each TRAP cache in megabytes.
const CACHE_SIZE_MB = 1024;
export async function getTrapCachingExtractorConfigArgs(
config: Config
): Promise<string[]> {
const result: string[] = [];
const result: string[][] = [];
for (const language of config.languages)
result.push(
...(await getTrapCachingExtractorConfigArgsForLang(config, language))
await getTrapCachingExtractorConfigArgsForLang(config, language)
);
return result;
return result.flat();
}
export async function getTrapCachingExtractorConfigArgsForLang(
@ -37,11 +40,19 @@ export async function getTrapCachingExtractorConfigArgsForLang(
const write = await actionsUtil.isAnalyzingDefaultBranch();
return [
`-O=${language}.trap.cache.dir=${cacheDir}`,
`-O=${language}.trap.cache.bound=1024`,
`-O=${language}.trap.cache.bound=${CACHE_SIZE_MB}`,
`-O=${language}.trap.cache.write=${write}`,
];
}
/**
* Download TRAP caches from the Actions cache.
* @param codeql The CodeQL instance to use.
* @param languages The languages being analyzed.
* @param logger A logger to record some informational messages to.
* @returns A partial map from languages to TRAP cache paths on disk, with
* languages for which we shouldn't use TRAP caching omitted.
*/
export async function downloadTrapCaches(
codeql: CodeQL,
languages: Language[],
@ -77,23 +88,28 @@ export async function downloadTrapCaches(
let baseSha = "unknown";
const eventPath = process.env.GITHUB_EVENT_PATH;
if (eventPath !== undefined) {
if (
process.env.GITHUB_EVENT_NAME === "pull_request" &&
eventPath !== undefined
) {
const event = JSON.parse(fs.readFileSync(path.resolve(eventPath), "utf-8"));
baseSha = event.pull_request?.base?.sha || "unknown";
baseSha = event.pull_request?.base?.sha || baseSha;
}
for (const language of languages) {
const cacheDir = result[language];
if (cacheDir === undefined) continue;
// The SHA from the base of the PR is the most similar commit we might have a cache for
const preferredKey = `codeql-trap-${CACHE_VERSION}-${await codeql.getVersion()}-${language}-${baseSha}`;
const preferredKey = await cacheKey(codeql, language, baseSha);
logger.info(
`Looking in Actions cache for TRAP cache with key ${preferredKey}`
);
const found = await cache.restoreCache([cacheDir], preferredKey, [
`codeql-trap-${CACHE_VERSION}-${await codeql.getVersion()}-${language}-`, // Fall back to any cache with the right key prefix
await cachePrefix(codeql, language), // Fall back to any cache with the right key prefix
]);
if (found === undefined) {
// Didn't find any cache, let's unset to avoid lookups in an empty directory
// We didn't find a TRAP cache in the Actions cache, so the directory on disk is
// still just an empty directory. There's no reason to tell the extractor to use it,
// so let's unset the entry in the map so we don't set any extractor options.
logger.info(`No TRAP cache found in Actions cache for ${language}`);
result[language] = undefined;
}
@ -109,15 +125,19 @@ export async function uploadTrapCaches(
): Promise<void> {
if (!(await actionsUtil.isAnalyzingDefaultBranch())) return; // Only upload caches from the default branch
const toAwait: Array<Promise<number>> = [];
for (const language of config.languages) {
const cacheDir = config.trapCaches[language];
if (cacheDir === undefined) continue;
const key = `codeql-trap-${CACHE_VERSION}-${await codeql.getVersion()}-${language}-${
const key = await cacheKey(
codeql,
language,
process.env.GITHUB_SHA || "unknown"
}`;
);
logger.info(`Uploading TRAP cache to Actions cache with key ${key}`);
await cache.saveCache([cacheDir], key);
toAwait.push(cache.saveCache([cacheDir], key));
}
await Promise.all(toAwait);
}
export async function getLanguagesSupportingCaching(
@ -132,6 +152,7 @@ export async function getLanguagesSupportingCaching(
return result;
const resolveResult = await codeql.betterResolveLanguages();
outer: for (const lang of languages) {
if (resolveResult.extractors[lang].length !== 1) continue;
const extractor = resolveResult.extractors[lang][0];
const trapCacheOptions =
extractor.extractor_options?.trap?.properties?.cache?.properties;
@ -153,3 +174,18 @@ export async function getLanguagesSupportingCaching(
}
return result;
}
async function cacheKey(
codeql: CodeQL,
language: Language,
baseSha: string
): Promise<string> {
return `${await cachePrefix(codeql, language)}-${baseSha}`;
}
async function cachePrefix(
codeql: CodeQL,
language: Language
): Promise<string> {
return `codeql-trap-${CACHE_VERSION}-${await codeql.getVersion()}-${language}-`;
}