diff --git a/lib/analyze-action.js b/lib/analyze-action.js index 4ee0d87e58..6453b000fe 100644 --- a/lib/analyze-action.js +++ b/lib/analyze-action.js @@ -91155,6 +91155,11 @@ async function uploadDependencyCaches(codeql, features, config, logger) { status.push({ language, result: "no-hash" /* NoHash */ }); continue; } + const key = await cacheKey2(codeql, features, language, patterns); + if (config.dependencyCachingRestoredKeys.includes(key)) { + status.push({ language, result: "duplicate" /* Duplicate */ }); + continue; + } const size = await getTotalCacheSize( cacheConfig.getDependencyPaths(), logger, @@ -91167,7 +91172,6 @@ async function uploadDependencyCaches(codeql, features, config, logger) { ); continue; } - const key = await cacheKey2(codeql, features, language, patterns); logger.info( `Uploading cache of size ${size} for ${language} with key ${key}...` ); diff --git a/lib/init-action.js b/lib/init-action.js index 6c92b83a7b..5dad5fc063 100644 --- a/lib/init-action.js +++ b/lib/init-action.js @@ -86824,6 +86824,7 @@ async function initActionState({ trapCaches, trapCacheDownloadTime, dependencyCachingEnabled: getCachingKind(dependencyCachingEnabled), + dependencyCachingRestoredKeys: [], extraQueryExclusions: [], overlayDatabaseMode: "none" /* None */, useOverlayDatabaseCaching: false, @@ -87317,6 +87318,7 @@ async function checkHashPatterns(codeql, features, language, cacheConfig, checkT } async function downloadDependencyCaches(codeql, features, languages, logger) { const status = []; + const restoredKeys = []; for (const language of languages) { const cacheConfig = defaultCacheConfigs[language]; if (cacheConfig === void 0) { @@ -87355,14 +87357,22 @@ async function downloadDependencyCaches(codeql, features, languages, logger) { const download_duration_ms = Math.round(performance.now() - start); if (hitKey !== void 0) { logger.info(`Cache hit on key ${hitKey} for ${language}.`); - const hit_kind = hitKey === primaryKey ? "exact" /* Exact */ : "partial" /* Partial */; - status.push({ language, hit_kind, download_duration_ms }); + let hit_kind = "partial" /* Partial */; + if (hitKey === primaryKey) { + hit_kind = "exact" /* Exact */; + } + status.push({ + language, + hit_kind, + download_duration_ms + }); + restoredKeys.push(hitKey); } else { status.push({ language, hit_kind: "miss" /* Miss */ }); logger.info(`No suitable cache found for ${language}.`); } } - return status; + return { statusReport: status, restoredKeys }; } async function cacheKey2(codeql, features, language, patterns) { const hash = await glob.hashFiles(patterns.join("\n")); @@ -89994,7 +90004,7 @@ async function run() { return; } let overlayBaseDatabaseStats; - let dependencyCachingResults; + let dependencyCachingStatus; try { if (config.overlayDatabaseMode === "overlay" /* Overlay */ && config.useOverlayDatabaseCaching) { overlayBaseDatabaseStats = await downloadOverlayBaseDatabaseFromCache( @@ -90135,12 +90145,14 @@ exec ${goBinaryPath} "$@"` } } if (shouldRestoreCache(config.dependencyCachingEnabled)) { - dependencyCachingResults = await downloadDependencyCaches( + const dependencyCachingResult = await downloadDependencyCaches( codeql, features, config.languages, logger ); + dependencyCachingStatus = dependencyCachingResult.statusReport; + config.dependencyCachingRestoredKeys = dependencyCachingResult.restoredKeys; } if (await codeQlVersionAtLeast(codeql, "2.17.1")) { } else { @@ -90241,7 +90253,7 @@ exec ${goBinaryPath} "$@"` toolsSource, toolsVersion, overlayBaseDatabaseStats, - dependencyCachingResults, + dependencyCachingStatus, logger, error4 ); @@ -90259,7 +90271,7 @@ exec ${goBinaryPath} "$@"` toolsSource, toolsVersion, overlayBaseDatabaseStats, - dependencyCachingResults, + dependencyCachingStatus, logger ); } diff --git a/src/config-utils.test.ts b/src/config-utils.test.ts index da58dd8b1b..906336d26d 100644 --- a/src/config-utils.test.ts +++ b/src/config-utils.test.ts @@ -200,12 +200,9 @@ test("load code quality config", async (t) => { ); // And the config we expect it to result in - const expectedConfig: configUtils.Config = { - version: actionsUtil.getActionVersion(), + const expectedConfig = createTestConfig({ analysisKinds: [AnalysisKind.CodeQuality], languages: [KnownLanguage.actions], - buildMode: undefined, - originalUserInput: {}, // This gets set because we only have `AnalysisKind.CodeQuality` computedConfig: { "disable-default-queries": true, @@ -219,14 +216,7 @@ test("load code quality config", async (t) => { debugMode: false, debugArtifactName: "", debugDatabaseName: "", - trapCaches: {}, - trapCacheDownloadTime: 0, - dependencyCachingEnabled: CachingKind.None, - extraQueryExclusions: [], - overlayDatabaseMode: OverlayDatabaseMode.None, - useOverlayDatabaseCaching: false, - repositoryProperties: {}, - }; + }); t.deepEqual(config, expectedConfig); }); @@ -507,9 +497,7 @@ test("load non-empty input", async (t) => { }; // And the config we expect it to parse to - const expectedConfig: configUtils.Config = { - version: actionsUtil.getActionVersion(), - analysisKinds: [AnalysisKind.CodeScanning], + const expectedConfig = createTestConfig({ languages: [KnownLanguage.javascript], buildMode: BuildMode.None, originalUserInput: userConfig, @@ -521,14 +509,7 @@ test("load non-empty input", async (t) => { debugMode: false, debugArtifactName: "my-artifact", debugDatabaseName: "my-db", - trapCaches: {}, - trapCacheDownloadTime: 0, - dependencyCachingEnabled: CachingKind.None, - extraQueryExclusions: [], - overlayDatabaseMode: OverlayDatabaseMode.None, - useOverlayDatabaseCaching: false, - repositoryProperties: {}, - }; + }); const languagesInput = "javascript"; const configFilePath = createConfigFile(inputFileContents, tempDir); diff --git a/src/config-utils.ts b/src/config-utils.ts index fa88d4b4a5..03608fb1ad 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -148,6 +148,9 @@ export interface Config { /** A value indicating how dependency caching should be used. */ dependencyCachingEnabled: CachingKind; + /** The keys of caches that we restored, if any. */ + dependencyCachingRestoredKeys: string[]; + /** * Extra query exclusions to append to the config. */ @@ -496,6 +499,7 @@ export async function initActionState( trapCaches, trapCacheDownloadTime, dependencyCachingEnabled: getCachingKind(dependencyCachingEnabled), + dependencyCachingRestoredKeys: [], extraQueryExclusions: [], overlayDatabaseMode: OverlayDatabaseMode.None, useOverlayDatabaseCaching: false, diff --git a/src/dependency-caching.test.ts b/src/dependency-caching.test.ts index eefb8504cd..a9b9e6210f 100644 --- a/src/dependency-caching.test.ts +++ b/src/dependency-caching.test.ts @@ -7,6 +7,7 @@ import test from "ava"; import * as sinon from "sinon"; import { cacheKeyHashLength } from "./caching-utils"; +import * as cachingUtils from "./caching-utils"; import { createStubCodeQL } from "./codeql"; import { CacheConfig, @@ -20,6 +21,8 @@ import { downloadDependencyCaches, CacheHitKind, cacheKey, + uploadDependencyCaches, + CacheStoreResult, } from "./dependency-caching"; import { Feature } from "./feature-flags"; import { KnownLanguage } from "./languages"; @@ -29,6 +32,7 @@ import { getRecordingLogger, checkExpectedLogMessages, LoggedMessage, + createTestConfig, } from "./testing-utils"; import { withTmpDir } from "./util"; @@ -237,15 +241,17 @@ test("downloadDependencyCaches - does not restore caches with feature keys if no .resolves(CSHARP_BASE_PATTERNS); makePatternCheckStub.withArgs(CSHARP_EXTRA_PATTERNS).resolves(undefined); - const results = await downloadDependencyCaches( + const result = await downloadDependencyCaches( codeql, createFeatures([]), [KnownLanguage.csharp], logger, ); - t.is(results.length, 1); - t.is(results[0].language, KnownLanguage.csharp); - t.is(results[0].hit_kind, CacheHitKind.Miss); + const statusReport = result.statusReport; + t.is(statusReport.length, 1); + t.is(statusReport[0].language, KnownLanguage.csharp); + t.is(statusReport[0].hit_kind, CacheHitKind.Miss); + t.deepEqual(result.restoredKeys, []); t.assert(restoreCacheStub.calledOnce); }); @@ -257,7 +263,8 @@ test("downloadDependencyCaches - restores caches with feature keys if features a const logger = getRecordingLogger(messages); const features = createFeatures([Feature.CsharpNewCacheKey]); - sinon.stub(glob, "hashFiles").resolves("abcdef"); + const mockHash = "abcdef"; + sinon.stub(glob, "hashFiles").resolves(mockHash); const keyWithFeature = await cacheKey( codeql, @@ -277,15 +284,28 @@ test("downloadDependencyCaches - restores caches with feature keys if features a .resolves(CSHARP_BASE_PATTERNS); makePatternCheckStub.withArgs(CSHARP_EXTRA_PATTERNS).resolves(undefined); - const results = await downloadDependencyCaches( + const result = await downloadDependencyCaches( codeql, features, [KnownLanguage.csharp], logger, ); - t.is(results.length, 1); - t.is(results[0].language, KnownLanguage.csharp); - t.is(results[0].hit_kind, CacheHitKind.Exact); + + // Check that the status report for telemetry indicates that one cache was restored with an exact match. + const statusReport = result.statusReport; + t.is(statusReport.length, 1); + t.is(statusReport[0].language, KnownLanguage.csharp); + t.is(statusReport[0].hit_kind, CacheHitKind.Exact); + + // Check that the restored key has been returned. + const restoredKeys = result.restoredKeys; + t.is(restoredKeys.length, 1); + t.assert( + restoredKeys[0].endsWith(mockHash), + "Expected restored key to end with hash returned by `hashFiles`", + ); + + // `restoreCache` should have been called exactly once. t.assert(restoreCacheStub.calledOnce); }); @@ -297,8 +317,14 @@ test("downloadDependencyCaches - restores caches with feature keys if features a const logger = getRecordingLogger(messages); const features = createFeatures([Feature.CsharpNewCacheKey]); + // We expect two calls to `hashFiles`: the first by the call to `cacheKey` below, + // and the second by `downloadDependencyCaches`. We use the result of the first + // call as part of the cache key that identifies a mock, existing cache. The result + // of the second call is for the primary restore key, which we don't want to match + // the first key so that we can test the restore keys logic. + const restoredHash = "abcdef"; const hashFilesStub = sinon.stub(glob, "hashFiles"); - hashFilesStub.onFirstCall().resolves("abcdef"); + hashFilesStub.onFirstCall().resolves(restoredHash); hashFilesStub.onSecondCall().resolves("123456"); const keyWithFeature = await cacheKey( @@ -319,18 +345,230 @@ test("downloadDependencyCaches - restores caches with feature keys if features a .resolves(CSHARP_BASE_PATTERNS); makePatternCheckStub.withArgs(CSHARP_EXTRA_PATTERNS).resolves(undefined); - const results = await downloadDependencyCaches( + const result = await downloadDependencyCaches( codeql, features, [KnownLanguage.csharp], logger, ); - t.is(results.length, 1); - t.is(results[0].language, KnownLanguage.csharp); - t.is(results[0].hit_kind, CacheHitKind.Partial); + + // Check that the status report for telemetry indicates that one cache was restored with a partial match. + const statusReport = result.statusReport; + t.is(statusReport.length, 1); + t.is(statusReport[0].language, KnownLanguage.csharp); + t.is(statusReport[0].hit_kind, CacheHitKind.Partial); + + // Check that the restored key has been returned. + const restoredKeys = result.restoredKeys; + t.is(restoredKeys.length, 1); + t.assert( + restoredKeys[0].endsWith(restoredHash), + "Expected restored key to end with hash returned by `hashFiles`", + ); + t.assert(restoreCacheStub.calledOnce); }); +test("uploadDependencyCaches - skips upload for a language with no cache config", async (t) => { + const codeql = createStubCodeQL({}); + const messages: LoggedMessage[] = []; + const logger = getRecordingLogger(messages); + const features = createFeatures([]); + const config = createTestConfig({ + languages: [KnownLanguage.actions], + }); + + const result = await uploadDependencyCaches(codeql, features, config, logger); + t.is(result.length, 0); + checkExpectedLogMessages(t, messages, [ + "Skipping upload of dependency cache for actions", + ]); +}); + +test("uploadDependencyCaches - skips upload if no files for the hash exist", async (t) => { + const codeql = createStubCodeQL({}); + const messages: LoggedMessage[] = []; + const logger = getRecordingLogger(messages); + const features = createFeatures([]); + const config = createTestConfig({ + languages: [KnownLanguage.go], + }); + + const makePatternCheckStub = sinon.stub(internal, "makePatternCheck"); + makePatternCheckStub.resolves(undefined); + + const result = await uploadDependencyCaches(codeql, features, config, logger); + t.is(result.length, 1); + t.is(result[0].language, KnownLanguage.go); + t.is(result[0].result, CacheStoreResult.NoHash); +}); + +test("uploadDependencyCaches - skips upload if we know the cache already exists", async (t) => { + process.env["RUNNER_OS"] = "Linux"; + + const codeql = createStubCodeQL({}); + const messages: LoggedMessage[] = []; + const logger = getRecordingLogger(messages); + const features = createFeatures([]); + + const mockHash = "abcdef"; + sinon.stub(glob, "hashFiles").resolves(mockHash); + + const makePatternCheckStub = sinon.stub(internal, "makePatternCheck"); + makePatternCheckStub + .withArgs(CSHARP_BASE_PATTERNS) + .resolves(CSHARP_BASE_PATTERNS); + + const primaryCacheKey = await cacheKey( + codeql, + features, + KnownLanguage.csharp, + CSHARP_BASE_PATTERNS, + ); + + const config = createTestConfig({ + languages: [KnownLanguage.csharp], + dependencyCachingRestoredKeys: [primaryCacheKey], + }); + + const result = await uploadDependencyCaches(codeql, features, config, logger); + t.is(result.length, 1); + t.is(result[0].language, KnownLanguage.csharp); + t.is(result[0].result, CacheStoreResult.Duplicate); +}); + +test("uploadDependencyCaches - skips upload if cache size is 0", async (t) => { + process.env["RUNNER_OS"] = "Linux"; + + const codeql = createStubCodeQL({}); + const messages: LoggedMessage[] = []; + const logger = getRecordingLogger(messages); + const features = createFeatures([]); + + const mockHash = "abcdef"; + sinon.stub(glob, "hashFiles").resolves(mockHash); + + const makePatternCheckStub = sinon.stub(internal, "makePatternCheck"); + makePatternCheckStub + .withArgs(CSHARP_BASE_PATTERNS) + .resolves(CSHARP_BASE_PATTERNS); + + sinon.stub(cachingUtils, "getTotalCacheSize").resolves(0); + + const config = createTestConfig({ + languages: [KnownLanguage.csharp], + }); + + const result = await uploadDependencyCaches(codeql, features, config, logger); + t.is(result.length, 1); + t.is(result[0].language, KnownLanguage.csharp); + t.is(result[0].result, CacheStoreResult.Empty); + + checkExpectedLogMessages(t, messages, [ + "Skipping upload of dependency cache", + ]); +}); + +test("uploadDependencyCaches - uploads caches when all requirements are met", async (t) => { + process.env["RUNNER_OS"] = "Linux"; + + const codeql = createStubCodeQL({}); + const messages: LoggedMessage[] = []; + const logger = getRecordingLogger(messages); + const features = createFeatures([]); + + const mockHash = "abcdef"; + sinon.stub(glob, "hashFiles").resolves(mockHash); + + const makePatternCheckStub = sinon.stub(internal, "makePatternCheck"); + makePatternCheckStub + .withArgs(CSHARP_BASE_PATTERNS) + .resolves(CSHARP_BASE_PATTERNS); + + sinon.stub(cachingUtils, "getTotalCacheSize").resolves(1024); + sinon.stub(actionsCache, "saveCache").resolves(); + + const config = createTestConfig({ + languages: [KnownLanguage.csharp], + }); + + const result = await uploadDependencyCaches(codeql, features, config, logger); + t.is(result.length, 1); + t.is(result[0].language, KnownLanguage.csharp); + t.is(result[0].result, CacheStoreResult.Stored); + t.is(result[0].upload_size_bytes, 1024); + + checkExpectedLogMessages(t, messages, ["Uploading cache of size"]); +}); + +test("uploadDependencyCaches - catches `ReserveCacheError` exceptions", async (t) => { + process.env["RUNNER_OS"] = "Linux"; + + const codeql = createStubCodeQL({}); + const messages: LoggedMessage[] = []; + const logger = getRecordingLogger(messages); + const features = createFeatures([]); + + const mockHash = "abcdef"; + sinon.stub(glob, "hashFiles").resolves(mockHash); + + const makePatternCheckStub = sinon.stub(internal, "makePatternCheck"); + makePatternCheckStub + .withArgs(CSHARP_BASE_PATTERNS) + .resolves(CSHARP_BASE_PATTERNS); + + sinon.stub(cachingUtils, "getTotalCacheSize").resolves(1024); + sinon + .stub(actionsCache, "saveCache") + .throws(new actionsCache.ReserveCacheError("Already in use")); + + const config = createTestConfig({ + languages: [KnownLanguage.csharp], + }); + + await t.notThrowsAsync(async () => { + const result = await uploadDependencyCaches( + codeql, + features, + config, + logger, + ); + t.is(result.length, 1); + t.is(result[0].language, KnownLanguage.csharp); + t.is(result[0].result, CacheStoreResult.Duplicate); + + checkExpectedLogMessages(t, messages, ["Not uploading cache for"]); + }); +}); + +test("uploadDependencyCaches - throws other exceptions", async (t) => { + process.env["RUNNER_OS"] = "Linux"; + + const codeql = createStubCodeQL({}); + const messages: LoggedMessage[] = []; + const logger = getRecordingLogger(messages); + const features = createFeatures([]); + + const mockHash = "abcdef"; + sinon.stub(glob, "hashFiles").resolves(mockHash); + + const makePatternCheckStub = sinon.stub(internal, "makePatternCheck"); + makePatternCheckStub + .withArgs(CSHARP_BASE_PATTERNS) + .resolves(CSHARP_BASE_PATTERNS); + + sinon.stub(cachingUtils, "getTotalCacheSize").resolves(1024); + sinon.stub(actionsCache, "saveCache").throws(); + + const config = createTestConfig({ + languages: [KnownLanguage.csharp], + }); + + await t.throwsAsync(async () => { + await uploadDependencyCaches(codeql, features, config, logger); + }); +}); + test("getFeaturePrefix - returns empty string if no features are enabled", async (t) => { const codeql = createStubCodeQL({}); const features = createFeatures([]); diff --git a/src/dependency-caching.ts b/src/dependency-caching.ts index 220f1d5bab..9f6aa2afca 100644 --- a/src/dependency-caching.ts +++ b/src/dependency-caching.ts @@ -193,6 +193,14 @@ export interface DependencyCacheRestoreStatus { /** An array of `DependencyCacheRestoreStatus` objects for each analysed language with a caching configuration. */ export type DependencyCacheRestoreStatusReport = DependencyCacheRestoreStatus[]; +/** Represents the results of `downloadDependencyCaches`. */ +export interface DownloadDependencyCachesResult { + /** The status report for telemetry */ + statusReport: DependencyCacheRestoreStatusReport; + /** An array of cache keys that we have restored and therefore know to exist. */ + restoredKeys: string[]; +} + /** * A wrapper around `cacheConfig.getHashPatterns` which logs when there are no files to calculate * a hash for the cache key from. @@ -239,8 +247,9 @@ export async function downloadDependencyCaches( features: FeatureEnablement, languages: Language[], logger: Logger, -): Promise { +): Promise { const status: DependencyCacheRestoreStatusReport = []; + const restoredKeys: string[] = []; for (const language of languages) { const cacheConfig = defaultCacheConfigs[language]; @@ -288,16 +297,27 @@ export async function downloadDependencyCaches( if (hitKey !== undefined) { logger.info(`Cache hit on key ${hitKey} for ${language}.`); - const hit_kind = - hitKey === primaryKey ? CacheHitKind.Exact : CacheHitKind.Partial; - status.push({ language, hit_kind, download_duration_ms }); + + // We have a partial cache hit, unless the key of the restored cache matches the + // primary restore key. + let hit_kind = CacheHitKind.Partial; + if (hitKey === primaryKey) { + hit_kind = CacheHitKind.Exact; + } + + status.push({ + language, + hit_kind, + download_duration_ms, + }); + restoredKeys.push(hitKey); } else { status.push({ language, hit_kind: CacheHitKind.Miss }); logger.info(`No suitable cache found for ${language}.`); } } - return status; + return { statusReport: status, restoredKeys }; } /** Enumerates possible outcomes for storing caches. */ @@ -365,6 +385,18 @@ export async function uploadDependencyCaches( continue; } + // Now that we have verified that there are suitable files, compute the hash for the cache key. + const key = await cacheKey(codeql, features, language, patterns); + + // Check that we haven't previously restored this exact key. If a cache with this key + // already exists in the Actions Cache, performing the next steps is pointless as the cache + // will not get overwritten. We can therefore skip the expensive work of measuring the size + // of the cache contents and attempting to upload it if we know that the cache already exists. + if (config.dependencyCachingRestoredKeys.includes(key)) { + status.push({ language, result: CacheStoreResult.Duplicate }); + continue; + } + // Calculate the size of the files that we would store in the cache. We use this to determine whether the // cache should be saved or not. For example, if there are no files to store, then we skip creating the // cache. In the future, we could also: @@ -390,8 +422,6 @@ export async function uploadDependencyCaches( continue; } - const key = await cacheKey(codeql, features, language, patterns); - logger.info( `Uploading cache of size ${size} for ${language} with key ${key}...`, ); diff --git a/src/init-action.ts b/src/init-action.ts index 3512520c2c..689ded2fc1 100644 --- a/src/init-action.ts +++ b/src/init-action.ts @@ -371,7 +371,7 @@ async function run() { } let overlayBaseDatabaseStats: OverlayBaseDatabaseDownloadStats | undefined; - let dependencyCachingResults: DependencyCacheRestoreStatusReport | undefined; + let dependencyCachingStatus: DependencyCacheRestoreStatusReport | undefined; try { if ( config.overlayDatabaseMode === OverlayDatabaseMode.Overlay && @@ -579,12 +579,15 @@ async function run() { // Restore dependency cache(s), if they exist. if (shouldRestoreCache(config.dependencyCachingEnabled)) { - dependencyCachingResults = await downloadDependencyCaches( + const dependencyCachingResult = await downloadDependencyCaches( codeql, features, config.languages, logger, ); + dependencyCachingStatus = dependencyCachingResult.statusReport; + config.dependencyCachingRestoredKeys = + dependencyCachingResult.restoredKeys; } // Suppress warnings about disabled Python library extraction. @@ -732,7 +735,7 @@ async function run() { toolsSource, toolsVersion, overlayBaseDatabaseStats, - dependencyCachingResults, + dependencyCachingStatus, logger, error, ); @@ -755,7 +758,7 @@ async function run() { toolsSource, toolsVersion, overlayBaseDatabaseStats, - dependencyCachingResults, + dependencyCachingStatus, logger, ); } diff --git a/src/testing-utils.ts b/src/testing-utils.ts index 12193309bb..eaec11e2c4 100644 --- a/src/testing-utils.ts +++ b/src/testing-utils.ts @@ -392,6 +392,7 @@ export function createTestConfig(overrides: Partial): Config { trapCaches: {}, trapCacheDownloadTime: 0, dependencyCachingEnabled: CachingKind.None, + dependencyCachingRestoredKeys: [], extraQueryExclusions: [], overlayDatabaseMode: OverlayDatabaseMode.None, useOverlayDatabaseCaching: false,