C#: Only use nuget.exe on Windows or machines with Mono.#21993
Open
michaelnebel wants to merge 7 commits into
Open
C#: Only use nuget.exe on Windows or machines with Mono.#21993michaelnebel wants to merge 7 commits into
nuget.exe on Windows or machines with Mono.#21993michaelnebel wants to merge 7 commits into
Conversation
michaelnebel
force-pushed
the
csharp/dropmono
branch
from
94cac45 to
892517e
Compare
michaelnebel
changed the title
nuget.exe on Windows or machines with Mono.
michaelnebel
changed the title
nuget.exe on Windows or machines with Mono.nuget.exe on Windows or machines with Mono.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
michaelnebel
force-pushed
the
csharp/dropmono
branch
from
892517e to
e23819d
Compare
Comment on lines
+115
to
+118
| catch (Exception e) | ||
| { | ||
| logger.LogError($"Failed to add default package source to {nugetConfigPath}: {e}"); | ||
| } |
Comment on lines
+143
to
+146
| catch (Exception exc) | ||
| { | ||
| logger.LogInfo($"Download of nuget.exe failed: {exc.Message}"); | ||
| } |
Comment on lines
+269
to
+273
| catch (Exception e) | ||
| { | ||
| logger.LogWarning($"Failed to check if default package source is added: {e}"); | ||
| return true; | ||
| } |
Comment on lines
+337
to
+340
| catch (Exception exc) | ||
| { | ||
| logger.LogError($"Failed to restore original nuget.config file: {exc}"); | ||
| } |
michaelnebel
force-pushed
the
csharp/dropmono
branch
2 times, most recently
from
042ed54 to
2675042
Compare
…ry method for constructing package config restorers.
michaelnebel
force-pushed
the
csharp/dropmono
branch
from
2675042 to
c747352
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the C# extractor’s packages.config restore flow to avoid attempting nuget.exe restores on platforms where it cannot run (Linux/macOS without Mono), aligning the dependency-fetching behavior with Mono deprecation on hosted runners.
Changes:
- Introduces
PackagesConfigRestoreFactorythat selects between anuget.exe-based restorer and a no-op implementation based on platform/Mono availability. - Updates
NugetPackageRestorerto use the new factory abstraction instead of directly instantiatingNugetExeWrapper. - Removes the standalone
NugetExeWrapper.csfile (its logic is now encapsulated in the new factory file).
Show a summary per file
| File | Description |
|---|---|
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/PackagesConfigRestorer.cs | Adds factory + interface + no-op path, and embeds the previous NugetExeWrapper logic behind a platform/Mono gate. |
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs | Switches packages.config restore to use PackagesConfigRestoreFactory.Create(...). |
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs | Deleted; implementation moved/encapsulated within the new restore factory file. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 1
Comment on lines
+26
to
+29
| /// It is worth noting that for MacOS and Linux, nuget.exe is used with mono. However, mono is being deprecated and the last images to contain | ||
| /// mono are | ||
| /// - Ubuntu 22.04 | ||
| /// - MacOS 14 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As mono is being deprecated (phased out) on the MacOS and Linux runners, we need to adjust the dependency manager implementation to only attempt to use
nuget.exeon Windows or machines wheremonois installed asnuget.execan't be run natively on Linux or MacOS (it requiresmono). At the momentnuget.exeis downloaded, but an exception is thrown whenmonois called (which is the caught and extraction doesn't fail).DCA looks good.
Review on a commit by commit basis is encouraged.