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.

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

… is enabled

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

Copilot · 2025-09-17T14:54:48Z

src/dependency-caching.ts

+    return cacheConfig.getHashPatterns(codeql, features);
+  } catch (err) {
+    if (err instanceof NoMatchingFilesError) {


The checkHashPatterns function catches NoMatchingFilesError but the error message logged doesn't distinguish between the original error condition (no lock files found) and the fallback condition (feature enabled but no fallback files found). Consider providing more specific error messages to help users understand which files are missing.

Copilot · 2025-09-17T14:54:48Z

src/dependency-caching.ts

+  if (await features.getValue(Feature.CsharpNewCacheKey, codeql)) {
+    // These are less accurate for use in cache key calculations, because they:
+    //
+    // - Don't contain the exact versions used. They may only contain version ranges or none at all.
+    // - They contain information unrelated to dependencies, which we don't care about.
+    //
+    // As a result, the hash we compute from these files may change, even if
+    // the dependencies haven't changed.
+    return makePatternCheck([
+      "**/*.csproj",
+      "**/packages.config",
+      "**/nuget.config",
+    ]);
+  }


[nitpick] The comment explains the limitations of using non-lock files but doesn't mention the cache key compatibility concern raised in the PR description. Consider adding a comment about cache invalidation when this feature is enabled/disabled to ensure the behavior is documented in the code.

henrymercer

Looks reasonable from an Action perspective

henrymercer · 2025-09-17T18:00:33Z

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.
+  throw new NoMatchingFilesError();


I don't have all the context here, but perhaps it would be clearer to return undefined and have this path included in the types?

michaelnebel · 2025-09-18T07:47:12Z

src/dependency-caching.ts

+    // As a result, the hash we compute from these files may change, even if
+    // the dependencies haven't changed.
+    return makePatternCheck([
+      "**/*.csproj",


@hvitved : Can you think of other files that are relevant to include i cache-key computations?

It is probably worth noting that we might risk using "bad" caches when version (or wildcard) ranges are used (in case the available version(s) of a dependency changes in a NuGet feed). Is it correctly understood that the only impact of this is that the "right" package version is just not available in the cache (and will be downloaded)?

Related question: For how long are caches stored?

Perhaps **/global.json and **/*.sln?

Actually, as far as I can tell, our dependency fetching logic will look in all small non-binary files for lines like <PackageReference Include="..." />, and then try to restore those dependencies. Obviously, we cannot meaningfully include all files in the hash, so perhaps we will just have to ignore this case.

Yes, there is a balance here between capturing all changes to dependencies and the ideal case, where the hash only changes if there are dependencies added or removed, or the exact versions that are in use change. If we hashed all "small non-binary" files, for example, we'd probably be uploading a new cache for every run. The main problem with that is how much of the cache quota we'd end up using.

The main goal here is to produce dependency caches for more C# projects in (primarily) Default Setup, where it's not possible to add custom caching steps.

We could port the logic to check all "small non-binary" files for lines that look like they include dependency information into the Action to determine which files to hash for this, but I wouldn't be too keen on that duplication and it's probably beyond the scope of this change. For more advanced logic like that, it would probably make sense to revisit the parts of the original EDR for this that talk about having the CLI communicate the caching configuration to the Action.

I agree with all of the above, I just wanted to point out this corner case in the dependency fetching logic.

Yes, that was helpful! Thank you ❤️

I only wanted to add the extra context for everyone's benefit.

For **/global.json and **/*.sln, do you know off the top of your head how common it is for them to contain dependency information?

For global.json files, I have only seen them used to indicate the .NET version - which we wouldn't want to cache (that should ideally be handled by the toolcache) and which I don't think affects the dependencies that are downloaded? Although, I imagine it would be possible to conditionally include dependencies based on the .NET version in use.

For *.sln files, I have only seen those include other project files.

I think you are right; let's skip global.json files and *.sln files.

michaelnebel

Looks plausible to me!

mbg added 6 commits September 17, 2025 13:06

Add CsharpNewCacheKey FF

71654c1

Propagate features into cachePrefix function

8ea9aef

Change getDefaultCacheConfig to be a const by turning paths int…

d33b2c1

…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

889a052

Move check whether there are files for hashing into getHashPatterns

f6e3125

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

7f81d73

… is enabled

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

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

Copilot AI reviewed Sep 17, 2025

View reviewed changes

henrymercer reviewed Sep 17, 2025

View reviewed changes

michaelnebel reviewed Sep 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

mbg commented Sep 17, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

henrymercer left a comment

Uh oh!

henrymercer Sep 17, 2025

Uh oh!

michaelnebel Sep 18, 2025

Uh oh!

hvitved Sep 19, 2025

Uh oh!

mbg Sep 19, 2025

Uh oh!

hvitved Sep 19, 2025

Uh oh!

mbg Sep 19, 2025

Uh oh!

hvitved Sep 19, 2025

Uh oh!

michaelnebel left a comment

Uh oh!

Uh oh!

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

Uh oh!

Conversation

mbg commented Sep 17, 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!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

henrymercer left a comment

Choose a reason for hiding this comment

Uh oh!

henrymercer Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

michaelnebel Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

hvitved Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

mbg Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

hvitved Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

mbg Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

hvitved Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

michaelnebel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!