Add feature flag to roll out JAR minimization in the Java extractor #3107

nickrolfe · 2025-09-11T10:14:06Z

This adds a feature flag which, when enabled, will have two effects:

sets an environment variable to enable a feature in the Java extractor to rewrite the contents of dependency JARs so that they take up less space on disk.
modify the key used for Actions dependency caching. This means that, if we find any problems with the JAR-rewriting and need to disable the flag, we won't have to worry about clearing corrupted cached.

Risk assessment

For internal use only. Please select the risk level of this change:

Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Consider adding a changelog entry for this change.
Confirm the readme and docs have been updated if necessary.

Copilot

Pull Request Overview

This PR adds a feature flag to control JAR minimization in the Java extractor, which will reduce the size of dependency JARs stored in Actions caches. The feature is gated behind the JavaMinimizeDependencyJars flag and affects cache keys to ensure safe rollout and rollback capabilities.

Key changes:

Adds new feature flag JavaMinimizeDependencyJars with CodeQL version requirement 2.23.0
Updates dependency caching functions to accept and use the minimization flag in cache key generation
Sets Java extractor environment variable when feature is enabled

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/feature-flags.ts	Adds JavaMinimizeDependencyJars feature flag definition
src/dependency-caching.ts	Updates caching functions to support JAR minimization flag and modify cache keys
src/init-action.ts	Integrates feature flag evaluation and sets extractor environment variable
src/analyze-action.ts	Passes minimization flag to dependency cache upload function
CHANGELOG.md	Documents the rollout of JAR minimization feature

src/dependency-caching.ts

CHANGELOG.md

mbg

Thanks for putting this together! I have a few minor comments about how the code is organised that are mostly suggestions, and one major comment about how the FF affects the cache key. In particular, I think that disabling the FF wouldn't prevent a minified cache from being restored -- see my comment there for more details.

mbg · 2025-09-11T11:39:28Z

src/feature-flags.ts

+  [Feature.JavaMinimizeDependencyJars]: {
+    defaultValue: false,
+    envVar: "CODEQL_ACTION_JAVA_MINIMIZE_DEPENDENCY_JARS",
+    minimumVersion: "2.23.0",


Could this be a ToolsFeature returned by the CLI, rather than using minimumVersion here?

I considered that, and thought it better to use a minimum version for two reasons:

A ToolsFeature would be declared by the CLI, but the CLI plays no role here; it's an extractor feature.

There were several earlier releases where the extractor supported this feature (i.e. would respond to the env var), but those were buggy (or caused problems by deleting private fields). So we want a version that not only supports the feature but is also known to be safe to use.

No strong feelings here, but isn't setting minimumVersion sort of equivalent to adding a ToolsFeature in that there's some CLI version that's going to add the ToolsFeature which is the minimum? I appreciate it's an extractor feature, but either way we don't check the extractors.

There were several earlier releases where the extractor supported this feature (i.e. would respond to the env var), but those were buggy (or caused problems by deleting private fields). So we want a version that not only supports the feature but is also known to be safe to use.

This makes sense. Am I assuming that your implicit thinking here is that minimumVersion is easier to change, if we discover other bugs, than a ToolsFeature?

Right, if the flag rollout reveals problems that require a fix in the extractor and a new CLI release, we can just bump the minimumVersion.

If we used ToolsFeature, we'd have to change that feature name from something like JavaMinimizeJars to JavaMinimizeJars2 or JavaMinimizeJarsButActuallyWorking.

I don't have particularly strong feelings about this, but using ToolsFeature would require waiting for another CLI release.

If we used ToolsFeature, we'd have to change that feature name from something like JavaMinimizeJars to JavaMinimizeJars2 or JavaMinimizeJarsButActuallyWorking.

Yeah, we had to do that recently with something else.

Right, if the flag rollout reveals problems that require a fix in the extractor and a new CLI release, we can just bump the minimumVersion.

The tricky aspect here is always understanding all the possible permutations in which the Action, the CLI, and the FFs interact. The minimumVersion here prevents the scenario where the buggy extractor implementation would get used if someone uses an older CLI, the newest Action, and the FF is on. A ToolsFeature would accomplish the same, except require another CLI release.

If things go wrong, then bumping minimumVersion probably isn't going to help with the case where: someone has a version of the Action (but not the latest) that includes the changes from this PR with the original minimumVersion, some CLI version below the new minimumVersion, and we re-enable the FF.

To prevent that, we'd either need a different ToolFeature or a different FF.

Gotcha. Are you happy that we proceed using minimumVersion, which means we don't have to add a ToolsFeature and wait for another release? If there's a problem that requires a fix and a new release, then I would add a ToolsFeature at the same time and change the name of the feature flag so that folks using older versions of the Action (relying on minimumVersion) aren't at risk.

I am happy with that 👍🏻

src/dependency-caching.ts

mbg · 2025-09-11T11:51:42Z

src/init-action.ts

+      logger.debug(
+        "CODEQL_EXTRACTOR_JAVA_OPTION_MINIMIZE_DEPENDENCY_JARS is already set, so the Action will not override it.",
+      );


Minor: Do you see any value in including the existing value of CODEQL_EXTRACTOR_JAVA_OPTION_MINIMIZE_DEPENDENCY_JARS here?

mbg · 2025-09-11T11:52:44Z

src/init-action.ts

        core.exportVariable("CODEQL_EXTRACTOR_PYTHON_EXTRACT_STDLIB", "true");
      }
    }
+    if (process.env["CODEQL_EXTRACTOR_JAVA_OPTION_MINIMIZE_DEPENDENCY_JARS"]) {


Consider adding CODEQL_EXTRACTOR_JAVA_OPTION_MINIMIZE_DEPENDENCY_JARS to EnvVar in environment.ts and then use that everywhere.

mbg · 2025-09-11T11:55:43Z

src/init-action.ts

+      logger.debug(
+        "CODEQL_EXTRACTOR_JAVA_OPTION_MINIMIZE_DEPENDENCY_JARS is already set, so the Action will not override it.",
+      );
+    } else if (await minimizeJavaDependencyJars(config, features, codeql)) {


Another alternative with minimizeJavaDependencyJars would be to just inline it here, since it is the only place where it's used. You could re-use the result of querying Feature.JavaMinimizeDependencyJars from earlier in this function and combine it with the language + build-mode checks.

I went with this approach.

mbg · 2025-09-11T12:03:39Z

src/dependency-caching.ts


+  // To ensure a safe rollout of JAR minimization, we change the key when the feature is enabled.
+  if (language === KnownLanguage.java && minimizeJavaJars) {
+    prefix = `${prefix}-minify`;


One concern I have is the following chain of events:

The FF is enabled and we create a cache with a key like codeql-dependencies-minify${hash}.

Something goes wrong and the cache is horribly broken.

We decide to disable the FF again.

cachePrefix(..) will return just codeql-dependencies- for the next run.

Because we use the result of cachePrefix(..) as a possible restore key in the actionsCache.restoreCache call if we don't get an exact match for the primary cache key, we would restore the broken codeql-dependencies-minify ${hash} cache.

The user now has to manually delete the problematic cache.

That sort of defeats the point of having a FF in the first place. We should ensure that we don't restore a minified cache if the FF is disabled. Maybe the easiest way of accomplishing this would be to add minify or whatever at the start of the cache key prefix?

Alternatively, we could include different symbols after the main prefix depending on whether minimising is enabled or not.

Good catch, thanks. I was vaguely aware that the Actions cache worked by matching a prefix, and not necessarily a full string, but I hadn't thought through the consequences. I think I'll just change it to minify-${prefix}.

mbg

Generally looks good, thanks for making those changes!

I have added a few more suggestions, but none are very major.

mbg · 2025-09-12T10:47:31Z

CHANGELOG.md

 ## [UNRELEASED]

-No user facing changes.
+- We are rolling out a change in September 2025 that will reduce the size of Actions dependency caches for Java analyses. [#3107](https://github.com/github/codeql-action/pull/3107)


How about, rather than having a date here that might change, we could word this to be about the support for it. Something like:

Suggested change

- We are rolling out a change in September 2025 that will reduce the size of Actions dependency caches for Java analyses. [#3107](https://github.com/github/codeql-action/pull/3107)

- We added support for reducing the size of dependency caches for Java analyses, which will reduce cache usage and speed up workflows. This will be enabled automatically at a later time. [#3107](https://github.com/github/codeql-action/pull/3107)

mbg · 2025-09-12T10:48:36Z

src/init-action.ts


+    // If the feature flag to minimize Java dependency jars is enabled, and we are doing a Java
+    // `build-mode: none` analysis (i.e. the flag is relevant), then set the environment variable
+    // that enables the corresponding option int he Java extractor.


Suggested change

// that enables the corresponding option int he Java extractor.

// that enables the corresponding option in the Java extractor.

mbg · 2025-09-12T10:49:13Z

src/init-action.ts

+    // that enables the corresponding option int he Java extractor.
+    if (process.env[EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS]) {
+      logger.debug(
+        `${EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS} is already set to ${process.env[EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS]}, so the Action will not override it.`,


Just to make it clearer what the value is, in case it's set to something silly that blends into the rest of the sentence.

Suggested change

`${EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS} is already set to ${process.env[EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS]}, so the Action will not override it.`,

`${EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS} is already set to '${process.env[EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS]}', so the Action will not override it.`,

mbg · 2025-09-12T10:54:56Z

src/feature-flags.ts

+  [Feature.JavaMinimizeDependencyJars]: {
+    defaultValue: false,
+    envVar: "CODEQL_ACTION_JAVA_MINIMIZE_DEPENDENCY_JARS",
+    minimumVersion: "2.23.0",


No strong feelings here, but isn't setting minimumVersion sort of equivalent to adding a ToolsFeature in that there's some CLI version that's going to add the ToolsFeature which is the minimum? I appreciate it's an extractor feature, but either way we don't check the extractors.

There were several earlier releases where the extractor supported this feature (i.e. would respond to the env var), but those were buggy (or caused problems by deleting private fields). So we want a version that not only supports the feature but is also known to be safe to use.

This makes sense. Am I assuming that your implicit thinking here is that minimumVersion is easier to change, if we discover other bugs, than a ToolsFeature?

mbg · 2025-09-12T10:58:28Z

I think you'll also need to rebuild the JS files (possibly requiring changes from main) and resolve the merge conflict in the changelog.

nickrolfe marked this pull request as ready for review September 11, 2025 10:54

nickrolfe requested a review from a team as a code owner September 11, 2025 10:54

Copilot AI review requested due to automatic review settings September 11, 2025 10:54

Copilot AI reviewed Sep 11, 2025

View reviewed changes

src/dependency-caching.ts Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

nickrolfe force-pushed the nickrolfe/minimize-jars branch from a9e4902 to f14c640 Compare September 11, 2025 11:00

mbg reviewed Sep 11, 2025

View reviewed changes

nickrolfe force-pushed the nickrolfe/minimize-jars branch from f14c640 to df56f59 Compare September 12, 2025 10:35

mbg previously approved these changes Sep 12, 2025

View reviewed changes

nickrolfe dismissed mbg’s stale review via c5c7736 September 12, 2025 11:09

nickrolfe force-pushed the nickrolfe/minimize-jars branch from df56f59 to c5c7736 Compare September 12, 2025 11:09

nickrolfe added 2 commits September 12, 2025 12:09

Add feature flag to roll out JAR minimization in the Java extractor

0abf548

Add changelog entry for Java dependency minimization rollout

3ca9525

nickrolfe force-pushed the nickrolfe/minimize-jars branch from c5c7736 to 3ca9525 Compare September 12, 2025 11:10

mbg approved these changes Sep 12, 2025

View reviewed changes

nickrolfe merged commit 185266a into main Sep 12, 2025
290 checks passed

nickrolfe deleted the nickrolfe/minimize-jars branch September 12, 2025 12:09

nickrolfe mentioned this pull request Sep 15, 2025

Only enable Java dependency minimisation when caching is enabled #3113

Merged

mbg mentioned this pull request Sep 17, 2025

Support non-lock files for C# cache key computation #3117

Open

github-actions bot mentioned this pull request Sep 25, 2025

Merge main into releases/v3 #3149

Merged

8 tasks

	- We are rolling out a change in September 2025 that will reduce the size of Actions dependency caches for Java analyses. [#3107](https://github.com/github/codeql-action/pull/3107)
	- We added support for reducing the size of dependency caches for Java analyses, which will reduce cache usage and speed up workflows. This will be enabled automatically at a later time. [#3107](https://github.com/github/codeql-action/pull/3107)

	// that enables the corresponding option int he Java extractor.
	// that enables the corresponding option in the Java extractor.

	`${EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS} is already set to ${process.env[EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS]}, so the Action will not override it.`,
	`${EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS} is already set to '${process.env[EnvVar.JAVA_EXTRACTOR_MINIMIZE_DEPENDENCY_JARS]}', so the Action will not override it.`,

Add feature flag to roll out JAR minimization in the Java extractor #3107

Add feature flag to roll out JAR minimization in the Java extractor #3107

Conversation

nickrolfe commented Sep 11, 2025

Risk assessment

Merge / deployment checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickrolfe Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbg Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbg commented Sep 12, 2025

Uh oh!

Uh oh!

Uh oh!

nickrolfe Sep 12, 2025 •

edited

Loading

mbg Sep 11, 2025 •

edited

Loading