Skip to content

Commit ae098fa

Browse files
henrymercerhenrymercer
authored
Merge pull request #4242 from github/henrymercer/graduate-toolcache-cleanup
Enable cleaning up toolcache bundles by default
2 parents 0a458f1 + 4ebc48b commit ae098fa

6 files changed

Lines changed: 27 additions & 79 deletions

File tree

‎.github/workflows/__multi-language-autodetect.yml‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/entry-points.js‎

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

‎pr-checks/checks/multi-language-autodetect.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ operatingSystems:
1414
- stable-v2.21.4
1515
- stable-v2.22.4
1616
env:
17-
CODEQL_ACTION_CLEANUP_TOOLCACHE_BUNDLES: true
1817
CODEQL_ACTION_RESOLVE_SUPPORTED_LANGUAGES_USING_CLI: true
1918
installGo: true
2019
installDotNet: true

‎src/feature-flags.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,6 @@ export enum Feature {
7474
AllowMergeConfigFiles = "allow_merge_config_files",
7575
/** Controls whether we allow multiple values for the `analysis-kinds` input. */
7676
AllowMultipleAnalysisKinds = "allow_multiple_analysis_kinds",
77-
/**
78-
* Controls whether we delete CodeQL bundles that we are not going to use from the toolcache
79-
* before downloading a different bundle, in order to reclaim disk space.
80-
*/
81-
CleanupToolcacheBundles = "cleanup_toolcache_bundles",
8277
CleanupTrapCaches = "cleanup_trap_caches",
8378
/** Whether to allow the `config-file` input to be specified via a repository property. */
8479
ConfigFileRepositoryProperty = "config_file_repository_property",
@@ -221,11 +216,6 @@ export const featureConfig = {
221216
envVar: "CODEQL_ACTION_ALLOW_MULTIPLE_ANALYSIS_KINDS",
222217
minimumVersion: undefined,
223218
},
224-
[Feature.CleanupToolcacheBundles]: {
225-
defaultValue: false,
226-
envVar: "CODEQL_ACTION_CLEANUP_TOOLCACHE_BUNDLES",
227-
minimumVersion: undefined,
228-
},
229219
[Feature.CleanupTrapCaches]: {
230220
defaultValue: false,
231221
envVar: "CODEQL_ACTION_CLEANUP_TRAP_CACHES",

‎src/setup-codeql.test.ts‎

Lines changed: 20 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ test.serial(
596596
undefined, // rawLanguages
597597
undefined, // otherLanguagePacksReason
598598
false, // useOverlayAwareDefaultCliVersion
599-
createFeatures([Feature.CleanupToolcacheBundles]),
599+
createFeatures([]),
600600
getRunnerLogger(true),
601601
);
602602

@@ -1375,10 +1375,7 @@ for (const bundle of ["per-language", "combined", "fallback"] as const) {
13751375
};
13761376
});
13771377
const addDiagnostic = sinon.stub(diagnostics, "addNoLanguageDiagnostic");
1378-
const features = createFeatures([
1379-
Feature.PerLanguageBundles,
1380-
Feature.CleanupToolcacheBundles,
1381-
]);
1378+
const features = createFeatures([Feature.PerLanguageBundles]);
13821379

13831380
await withTmpDir(async (tmpDir) => {
13841381
setupActionsVars(tmpDir, tmpDir);
@@ -1826,11 +1823,15 @@ function createToolcacheEntry(
18261823
*/
18271824
async function runDownloadCodeQL(
18281825
toolcacheRoot: string,
1829-
features: Feature[],
18301826
bundleVersion: string | undefined,
1831-
env = getTestEnv({
1832-
[ActionsEnvVars.RUNNER_TOOL_CACHE]: toolcacheRoot,
1833-
}),
1827+
{
1828+
env = getTestEnv({
1829+
[ActionsEnvVars.RUNNER_ENVIRONMENT]: "github-hosted",
1830+
[ActionsEnvVars.RUNNER_TOOL_CACHE]: toolcacheRoot,
1831+
}),
1832+
}: {
1833+
env?: Env;
1834+
} = {},
18341835
): Promise<{
18351836
codeqlFolder: string;
18361837
cleanupDiagnostic: toolsDownload.ToolcacheCleanupResult | undefined;
@@ -1841,7 +1842,6 @@ async function runDownloadCodeQL(
18411842
const { codeqlFolder } = await setupCodeql.downloadCodeQLBundle(
18421843
initAllState({
18431844
env,
1844-
features: createFeatures(features),
18451845
}),
18461846
{
18471847
bundle: {
@@ -1879,11 +1879,9 @@ async function runDownloadCodeQL(
18791879
async function testToolcacheCleanup(
18801880
t: ExecutionContext<unknown>,
18811881
{
1882-
features,
18831882
runnerEnvironment,
18841883
setUp,
18851884
}: {
1886-
features: Feature[];
18871885
runnerEnvironment: string | undefined;
18881886
setUp?: (env: Env) => void;
18891887
},
@@ -1895,12 +1893,8 @@ async function testToolcacheCleanup(
18951893
) {
18961894
await withTmpDir(async (tmpDir) => {
18971895
setupActionsVars(tmpDir, tmpDir);
1898-
if (runnerEnvironment === undefined) {
1899-
delete process.env[ActionsEnvVars.RUNNER_ENVIRONMENT];
1900-
} else {
1901-
process.env[ActionsEnvVars.RUNNER_ENVIRONMENT] = runnerEnvironment;
1902-
}
19031896
const env = getTestEnv({
1897+
[ActionsEnvVars.RUNNER_ENVIRONMENT]: runnerEnvironment,
19041898
[ActionsEnvVars.RUNNER_TOOL_CACHE]: tmpDir,
19051899
});
19061900
setUp?.(env);
@@ -1920,9 +1914,8 @@ async function testToolcacheCleanup(
19201914

19211915
const { cleanupDiagnostic } = await runDownloadCodeQL(
19221916
tmpDir,
1923-
features,
19241917
CLEANUP_BUNDLE_VERSION,
1925-
env,
1918+
{ env },
19261919
);
19271920

19281921
t.true(
@@ -1934,28 +1927,12 @@ async function testToolcacheCleanup(
19341927
});
19351928
}
19361929

1937-
test.serial(
1938-
"downloadCodeQLBundle does not clean up the toolcache when the feature flag is disabled",
1939-
async (t) => {
1940-
await testToolcacheCleanup(
1941-
t,
1942-
{ features: [], runnerEnvironment: "github-hosted" },
1943-
({ cleanupDiagnostic, destinationDirectory, staleDirectory }) => {
1944-
t.true(fs.existsSync(staleDirectory));
1945-
t.true(fs.existsSync(destinationDirectory));
1946-
t.is(cleanupDiagnostic, undefined);
1947-
},
1948-
);
1949-
},
1950-
);
1951-
19521930
test.serial(
19531931
"downloadCodeQLBundle does not clean up the toolcache when the runner is not GitHub-hosted",
19541932
async (t) => {
19551933
await testToolcacheCleanup(
19561934
t,
19571935
{
1958-
features: [Feature.CleanupToolcacheBundles],
19591936
runnerEnvironment: "self-hosted",
19601937
},
19611938
({ cleanupDiagnostic, destinationDirectory, staleDirectory }) => {
@@ -1975,7 +1952,6 @@ test.serial(
19751952
await testToolcacheCleanup(
19761953
t,
19771954
{
1978-
features: [Feature.CleanupToolcacheBundles],
19791955
runnerEnvironment: undefined,
19801956
},
19811957
({ cleanupDiagnostic, destinationDirectory, staleDirectory }) => {
@@ -1988,12 +1964,11 @@ test.serial(
19881964
);
19891965

19901966
test.serial(
1991-
"downloadCodeQLBundle deletes other CodeQL bundles from the toolcache when enabled on a GitHub-hosted runner",
1967+
"downloadCodeQLBundle deletes other CodeQL bundles from the toolcache on a GitHub-hosted runner",
19921968
async (t) => {
19931969
await testToolcacheCleanup(
19941970
t,
19951971
{
1996-
features: [Feature.CleanupToolcacheBundles],
19971972
runnerEnvironment: "github-hosted",
19981973
},
19991974
({ cleanupDiagnostic, destinationDirectory, staleDirectory }) => {
@@ -2027,7 +2002,6 @@ test.serial(
20272002

20282003
const { cleanupDiagnostic } = await runDownloadCodeQL(
20292004
tmpDir,
2030-
[Feature.CleanupToolcacheBundles],
20312005
CLEANUP_BUNDLE_VERSION,
20322006
);
20332007

@@ -2057,7 +2031,6 @@ test.serial(
20572031

20582032
const { cleanupDiagnostic } = await runDownloadCodeQL(
20592033
tmpDir,
2060-
[Feature.CleanupToolcacheBundles],
20612034
CLEANUP_BUNDLE_VERSION,
20622035
);
20632036

@@ -2189,7 +2162,6 @@ test.serial(
21892162

21902163
const { cleanupDiagnostic } = await runDownloadCodeQL(
21912164
toolcacheRoot,
2192-
[Feature.CleanupToolcacheBundles],
21932165
CLEANUP_BUNDLE_VERSION,
21942166
);
21952167

@@ -2212,7 +2184,6 @@ test.serial(
22122184
await testToolcacheCleanup(
22132185
t,
22142186
{
2215-
features: [Feature.CleanupToolcacheBundles],
22162187
runnerEnvironment: "github-hosted",
22172188
setUp: (env) => {
22182189
env.set(EnvVar.HAS_SET_UP_CODEQL, "true");
@@ -2242,9 +2213,13 @@ test.serial(
22422213
);
22432214
const { codeqlFolder, cleanupDiagnostic } = await runDownloadCodeQL(
22442215
tmpDir,
2245-
[Feature.CleanupToolcacheBundles],
22462216
CLEANUP_BUNDLE_VERSION,
2247-
getTestEnv({ [EnvVar.HAS_SET_UP_CODEQL]: "true" }),
2217+
{
2218+
env: getTestEnv({
2219+
[ActionsEnvVars.RUNNER_ENVIRONMENT]: "github-hosted",
2220+
[EnvVar.HAS_SET_UP_CODEQL]: "true",
2221+
}),
2222+
},
22482223
);
22492224

22502225
t.true(fs.existsSync(staleDirectory));
@@ -2312,7 +2287,7 @@ test.serial(
23122287
undefined, // rawLanguages
23132288
undefined, // otherLanguagePacksReason
23142289
false, // useOverlayAwareDefaultCliVersion
2315-
createFeatures([Feature.CleanupToolcacheBundles]),
2290+
createFeatures([]),
23162291
getRunnerLogger(true),
23172292
),
23182293
{ is: error },
@@ -2347,7 +2322,6 @@ test.serial(
23472322

23482323
const { codeqlFolder, cleanupDiagnostic } = await runDownloadCodeQL(
23492324
tmpDir,
2350-
[Feature.CleanupToolcacheBundles],
23512325
undefined, // bundleVersion
23522326
);
23532327

@@ -2381,7 +2355,6 @@ test.serial(
23812355

23822356
const { cleanupDiagnostic } = await runDownloadCodeQL(
23832357
tmpDir,
2384-
[Feature.CleanupToolcacheBundles],
23852358
CLEANUP_BUNDLE_VERSION,
23862359
);
23872360

@@ -2417,7 +2390,6 @@ test.serial(
24172390

24182391
const { cleanupDiagnostic } = await runDownloadCodeQL(
24192392
tmpDir,
2420-
[Feature.CleanupToolcacheBundles],
24212393
CLEANUP_BUNDLE_VERSION,
24222394
);
24232395

@@ -2467,7 +2439,6 @@ test.serial(
24672439

24682440
const { cleanupDiagnostic } = await runDownloadCodeQL(
24692441
toolcacheRoot,
2470-
[Feature.CleanupToolcacheBundles],
24712442
CLEANUP_BUNDLE_VERSION,
24722443
);
24732444

‎src/setup-codeql.ts‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -839,17 +839,16 @@ function getToolcacheDestination(
839839
}
840840

841841
/**
842-
* Reclaims disk space by deleting the CodeQL tools from the toolcache, if enabled.
842+
* Reclaims disk space by deleting the CodeQL tools from the toolcache on GitHub-hosted runners.
843843
*
844844
* On GitHub-hosted runners the toolcache shares a filesystem with the workspace, so tools left in
845845
* the toolcache take up space that the analysis could use instead. This holds wherever we extract
846846
* the tools we are obtaining, since the toolcache is on that filesystem either way.
847847
*/
848848
async function tryDeleteToolcacheBundles({
849849
env,
850-
features,
851850
logger,
852-
}: ActionState<["Logger", "ReadOnlyEnv", "FeatureFlags"]>): Promise<void> {
851+
}: ActionState<["Logger", "ReadOnlyEnv"]>): Promise<void> {
853852
// A step that has already set up CodeQL may hand out a path into the toolcache that a later step
854853
// runs, so only the first step to set it up can know that nothing else relies on the toolcache.
855854
if (env.getOptional(EnvVar.HAS_SET_UP_CODEQL) !== undefined) {
@@ -860,11 +859,7 @@ async function tryDeleteToolcacheBundles({
860859
return;
861860
}
862861

863-
if (
864-
!isGitHubHostedRunner() ||
865-
!isToolcacheOnWorkspaceFilesystem(logger) ||
866-
!(await features.getValue(Feature.CleanupToolcacheBundles))
867-
) {
862+
if (!isGitHubHostedRunner(env) || !isToolcacheOnWorkspaceFilesystem(logger)) {
868863
return;
869864
}
870865

@@ -983,7 +978,7 @@ export async function setupCodeQLBundle(
983978
break;
984979
case "download": {
985980
const result = await downloadCodeQLBundle(
986-
{ env: getEnv(), features, logger },
981+
{ env: getEnv(), logger },
987982
source,
988983
apiDetails,
989984
zstdAvailability.version,
@@ -1019,7 +1014,7 @@ export async function setupCodeQLBundle(
10191014
* @returns The extraction directory and download timings.
10201015
*/
10211016
export async function downloadCodeQLBundle(
1022-
action: ActionState<["Logger", "ReadOnlyEnv", "FeatureFlags"]>,
1017+
action: ActionState<["Logger", "ReadOnlyEnv"]>,
10231018
source: CodeQLDownloadSource,
10241019
apiDetails: api.GitHubApiDetails,
10251020
tarVersion: tar.TarVersion | undefined,

0 commit comments

Comments
 (0)