C#: Add extractor option for the dependency directory in BMN. #20832
+139
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyDirectory.cs
Fixed
Show fixed
Hide fixed
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyDirectory.cs
Dismissed
Show dismissed
Hide dismissed
michaelnebel
force-pushed
the
csharp/dependencycaching
branch
4 times, most recently
from
9126705 to
7223270
Compare
michaelnebel
changed the title
Copilot finished reviewing on behalf of
michaelnebel
November 17, 2025 12:10
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds an extractor option for setting the dependency directory in buildless mode (standalone) extraction, enabling dependency caching. This aligns with the equivalent Java implementation for consistency across languages.
Key changes:
- Introduced a new
DependencyDirectoryclass that supports both cached and temporary directory modes - Added
GetBuildlessDependencyDir()method to read the newCODEQL_EXTRACTOR_CSHARP_OPTION_BUILDLESS_DEPENDENCY_DIRenvironment variable - Replaced
TemporaryDirectorywithDependencyDirectoryin NuGet package restoration classes
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyDirectory.cs | New class that manages dependency directories, supporting both cached and temporary modes based on environment variable |
| csharp/extractor/Semmle.Util/EnvironmentVariables.cs | Added method to retrieve the buildless dependency directory from environment variables |
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs | Updated to use DependencyDirectory instead of TemporaryDirectory for package storage |
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs | Updated to use DependencyDirectory and revised comment to reflect cached or temp location |
| csharp/codeql-extractor.yml | Added buildless_dependency_dir option definition for the extractor configuration |
| csharp/ql/integration-tests/all-platforms/standalone_dependency_dir/test.py | Integration test that verifies the dependency directory option works correctly |
| csharp/ql/integration-tests/all-platforms/standalone_dependency_dir/proj/standalone.csproj | Test project file with Newtonsoft.Json dependency |
| csharp/ql/integration-tests/all-platforms/standalone_dependency_dir/proj/global.json | SDK version specification for the test project |
| csharp/ql/integration-tests/all-platforms/standalone_dependency_dir/proj/Program.cs | Minimal C# program for the integration test |
| csharp/ql/integration-tests/all-platforms/standalone_dependency_dir/Assemblies.ql | Query to verify that dependencies are found in the expected directory |
| csharp/ql/integration-tests/all-platforms/standalone_dependency_dir/Assemblies.expected | Expected output showing dependency in the configured directory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyDirectory.cs
Outdated
Show resolved
Hide resolved
mbg
reviewed
Nov 17, 2025
Member
mbg
left a comment
mbg
left a comment
There was a problem hiding this comment.
Thank you for taking the time to add support for this! Looks good from a consumer perspective. I added a few minor comments.
I don't know enough about the BMN extractor to know whether this change to NugetPackageRestorer covers all necessary cases, so someone more familiar with it should ideally review this too.
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyDirectory.cs
Show resolved
Hide resolved
csharp/ql/integration-tests/all-platforms/standalone_dependency_dir/test.py
Show resolved
Hide resolved
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs
Outdated
Show resolved
Hide resolved
michaelnebel
force-pushed
the
csharp/dependencycaching
branch
from
79ecb0e to
89d8c45
Compare
michaelnebel
force-pushed
the
csharp/dependencycaching
branch
from
89d8c45 to
eb23d31
Compare
mbg
previously approved these changes
Nov 17, 2025
hvitved
previously approved these changes
Nov 21, 2025
| # dependency directory set above. | ||
| codeql.database.create(source_root="proj", build_mode="none") | ||
|
|
||
| # Check that the packages directory has been created in the dependecies folder. |
michaelnebel
force-pushed
the
csharp/dependencycaching
branch
from
2440063 to
5c454d2
Compare
Contributor
Author
|
@hvitved : Had to rebase due to failing integration test (unrelated to this PR) - fixed the typo (this is the only change since the last review). |
hvitved
approved these changes
Nov 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the C# equivalent of this PR.
In this PR we add an extractor option for setting the directory for storing dependencies in BMN.
This is needed as a part of the Overlay work related to caching. Even though the action uses the environment variable directly for setting the directory, we still introduce this as an extractor option to streamline the implementation with Java.