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

mbg · 2025-09-17T14:53:33Z

This PR modifies our implementation of dependency caching for C# to allow non-lock files to be used when computing a hash for the cache key.

The hash that's included in the cache key is derived from files which contain information about the project's dependency. The idea being that the hash changes only if the dependencies change. Currently, we only use packages.lock.json and paket.lock files for this. These files contain exact dependency information (including the exact versions that were resolved by the respective package manager). The resulting hashes we compute for the cache keys should therefore only change if the dependencies (including versions) change. This is the desired behaviour.

However, relatively few C# projects use lock files and fewer yet have them checked-in to the repository. This means that most C# projects do not benefit from our implementation of dependency caching at the moment.

This PR modifies the behaviour of our dependency caching implementation for C# as follows:

As before, we search for packages.lock.json or paket.lock files. If they exist, we use them to compute a hash for the cache key.
If no such files exist, and the new CsharpNewCacheKey feature is enabled (via FF or environment variable), we search for additional files that typically contain dependency information for C# projects. If they exist, we use them to compute the hash for the cache key.

Using the additional files is less precise, because they may not contain exact version information (only version ranges or no versions at all) and they contain information unrelated to dependencies. This means that the hash we compute can change, even if the dependencies don't change. However, dependency caching will benefit more C# projects.

This PR is best reviewed commit-by-commit. The first few commits just add the new Feature and refactor the code a bit to facilitate the changes.

Questions for reviewers:

Right now, disabling the Feature wouldn't avoid restoring caches that were produced with the Feature enabled. We likely want to do something similar as for Add feature flag to roll out JAR minimization in the Java extractor #3107 that marks caches produced with this Feature on.
Should we use this opportunity to also extend the list of paths that are included in the cache for C#? The setup-dotnet Action considers more paths and computes them dynamically. Now that we have getDependencyPaths as a function, it would be easy to do this.
~~If so, should that be gated behind the same FF?~~
~~Are there any additional files we should include for C# hashes?~~

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 enhances C# dependency caching support by allowing non-lock files to be used when computing cache keys. Currently, only packages.lock.json and paket.lock files are used for cache key computation, which limits caching benefits to projects that have these lock files checked in. The new implementation maintains backward compatibility while expanding support to more C# projects through an optional feature flag.

Key changes:

Adds a new CsharpNewCacheKey feature flag with environment variable support
Refactors cache configuration from static data structures to dynamic functions
Implements fallback logic to use .csproj, packages.config, and nuget.config files when lock files are unavailable

Reviewed Changes

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

Show a summary per file

File	Description
src/feature-flags.ts	Adds the new `CsharpNewCacheKey` feature flag configuration
src/dependency-caching.ts	Core refactoring to support dynamic hash pattern generation and C# fallback logic
src/init-action.ts	Updates function calls to pass CodeQL and features parameters
src/analyze-action.ts	Updates function calls to pass CodeQL and features parameters
lib/*	Generated JavaScript files reflecting the TypeScript changes

src/dependency-caching.ts

henrymercer

Looks reasonable from an Action perspective

src/dependency-caching.ts

michaelnebel

Looks plausible to me!

…o a function Changing `paths` to be a function is necessary to allow `getTemporaryDirectory` to be called

… is enabled

Add tests for `makePatternCheck` and `checkHashPatterns`

Copilot

Pull Request Overview

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

Copilot · 2025-11-05T17:55:13Z

src/dependency-caching.ts

+ * @param codeql The CodeQL instance to use.
+ * @param features Information about which FFs are enabled.
 * @param language The language being analyzed.
 * @param cacheConfig The cache configuration for the language.


The JSDoc parameter description for cacheConfig is incorrect. This parameter has been removed from the function signature but the documentation still references it. The function now receives patterns instead.

Suggested change

* @param cacheConfig The cache configuration for the language.

* @param patterns The file patterns to hash for the cache key.

Copilot · 2025-11-05T17:55:14Z

src/dependency-caching.ts

+
+  if (patterns === undefined) {
+    logger.info(
+      `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.`,


The log message says 'Skipping download' but the function checkHashPatterns is also used in the upload path. Consider making the message more generic or having separate messages for download vs upload contexts.

Suggested change

`Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.`,

`Skipping dependency cache for ${language} as we cannot calculate a hash for the cache key.`,

This seems valid, though "Skipping dependency caching" seems more natural.

michaelnebel

The new pattern for deciding which C# files to use in the cache hash LGTM!

Should we use this opportunity to also extend the list of paths that are included in the cache for C#? The setup-dotnet Action considers more paths and computes them dynamically. Now that we have getDependencyPaths as a function, it would be easy to do this.

Does this mean that we are "only" including "{working_dir}/.nuget/packages" in the cache?

The dependency manager for C# use the "scratch dir" for storing downloaded packages.

nickrolfe

I only reviewed the parts that generalise the mechanism to add a feature-flag-specific prefix to the cache key, but that looks good.

henrymercer

This generally looks good. The only blocking comment from me is the naming of the cache entries.

For PRs like this in the future, consider splitting up into a core functionality part (e.g. here, handling the addition of feature flags to the cache key), and a language-specific part (e.g. here, changing the cache key for C#).

src/caching-utils.ts

henrymercer · 2025-11-07T17:26:40Z

src/dependency-caching.ts

+  // If we get to this point, the `basePatterns` didn't find any files,
+  // and `Feature.CsharpNewCacheKey` is either not enabled or we didn't
+  // find any files using those patterns either.
+  return undefined;


The last bit isn't true since we'll return the result of makePatternCheck, even if it's undefined. It might be clearer to have a helper function that checks whether the patterns match anything, and use that here instead.

henrymercer · 2025-11-07T17:31:14Z

src/dependency-caching.ts

+
+  if (patterns === undefined) {
+    logger.info(
+      `Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.`,


This seems valid, though "Skipping dependency caching" seems more natural.

henrymercer · 2025-11-07T17:37:13Z

src/dependency-caching.ts


-  return `${prefix}-${CODEQL_DEPENDENCY_CACHE_VERSION}-${runnerOs}-${language}-`;
+  // Assemble the cache key.
+  return `${featurePrefix}${prefix}-${CODEQL_DEPENDENCY_CACHE_VERSION}-${runnerOs}-${language}-`;


If users list cache entries using the API, I think it'd be clearer to them what those cache entries are for if we keep codeql-dependencies- at the front.

mbg requested a review from a team as a code owner September 17, 2025 14:53

mbg requested review from Copilot, henrymercer and michaelnebel September 17, 2025 14:53

Copilot AI reviewed Sep 17, 2025

View reviewed changes

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

src/dependency-caching.ts Show resolved Hide resolved

henrymercer reviewed Sep 17, 2025

View reviewed changes

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

michaelnebel reviewed Sep 18, 2025

View reviewed changes

src/dependency-caching.ts Show resolved Hide resolved

michaelnebel reviewed Sep 18, 2025

View reviewed changes

jbj mentioned this pull request Nov 5, 2025

C#: Overlay extraction support. github/codeql#20425

Draft

mbg added 9 commits November 5, 2025 15:48

Add CsharpNewCacheKey FF

1ca20ab

Propagate features into cachePrefix function

2aa1f55

Change getDefaultCacheConfig to be a const by turning paths int…

2a7680f

…o a function Changing `paths` to be a function is necessary to allow `getTemporaryDirectory` to be called

Change hash to be a function that can use Features

ab1c842

Move check whether there are files for hashing into getHashPatterns

6b48207

Use additional files for C# key hashes if Feature.CsharpNewCacheKey…

0324490

… is enabled

Move createCacheKeyHash to caching-utils

0cbd930

Add and use getFeaturePrefix for dependency caching

03b2dc2

Use undefined instead of NoMatchingFilesError

2680455

Add tests for `makePatternCheck` and `checkHashPatterns`

mbg force-pushed the mbg/csharp/new-cache-key-calculation branch from 7f81d73 to 2680455 Compare November 5, 2025 17:34

github-actions bot added the size/L May be hard to review label Nov 5, 2025

mbg requested review from Copilot, henrymercer and nickrolfe November 5, 2025 17:35

Copilot AI reviewed Nov 5, 2025

View reviewed changes

michaelnebel reviewed Nov 7, 2025

View reviewed changes

nickrolfe reviewed Nov 7, 2025

View reviewed changes

henrymercer reviewed Nov 7, 2025

View reviewed changes

	* @param cacheConfig The cache configuration for the language.
	* @param patterns The file patterns to hash for the cache key.

	`Skipping download of dependency cache for ${language} as we cannot calculate a hash for the cache key.`,
	`Skipping dependency cache for ${language} as we cannot calculate a hash for the cache key.`,

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

Are you sure you want to change the base?

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

Conversation

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

Uh oh!

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!

henrymercer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

michaelnebel left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

henrymercer Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

michaelnebel left a comment • 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!

nickrolfe left a comment

Choose a reason for hiding this comment

Uh oh!

henrymercer left a comment • 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!

Uh oh!

henrymercer Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

henrymercer Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

henrymercer Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mbg commented Sep 17, 2025 •

edited

Loading

michaelnebel left a comment •

edited

Loading

henrymercer left a comment •

edited

Loading