Skip to content

Commit 9a00c75

Browse files
mbgmbg
authored
Merge pull request #21236 from github/mbg/csharp/fix-registry-feeds
C#: Make sure `allFeeds` contains at least `explicitFeeds`
2 parents 5204255 + 454d13b commit 9a00c75

10 files changed

Lines changed: 132 additions & 0 deletions

File tree

‎csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)
158158

159159
bool IBuildActions.IsMacOs() => IsMacOs;
160160

161+
public bool IsLinux { get; set; }
162+
163+
bool IBuildActions.IsLinux() => IsLinux;
164+
161165
public bool IsRunningOnAppleSilicon { get; set; }
162166

163167
bool IBuildActions.IsRunningOnAppleSilicon() => IsRunningOnAppleSilicon;

‎csharp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)
146146

147147
bool IBuildActions.IsMacOs() => IsMacOs;
148148

149+
public bool IsLinux { get; set; }
150+
151+
bool IBuildActions.IsLinux() => IsLinux;
152+
149153
public bool IsRunningOnAppleSilicon { get; set; }
150154

151155
bool IBuildActions.IsRunningOnAppleSilicon() => IsRunningOnAppleSilicon;

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,43 @@ private IEnumerable<string> GetFeeds(Func<IList<string>> getNugetFeeds)
814814
private (HashSet<string> explicitFeeds, HashSet<string> allFeeds) GetAllFeeds()
815815
{
816816
var nugetConfigs = fileProvider.NugetConfigs;
817+
818+
// On systems with case-sensitive file systems (for simplicity, we assume that is Linux), the
819+
// filenames of NuGet configuration files must be named correctly. For compatibility with projects
820+
// that are typically built on Windows or macOS where this doesn't matter, we accept all variants
821+
// of `nuget.config` ourselves. However, `dotnet` does not. If we detect that incorrectly-named
822+
// files are present, we emit a diagnostic to warn the user.
823+
if (SystemBuildActions.Instance.IsLinux())
824+
{
825+
string[] acceptedNugetConfigNames = ["nuget.config", "NuGet.config", "NuGet.Config"];
826+
var invalidNugetConfigs = nugetConfigs
827+
.Where(path => !acceptedNugetConfigNames.Contains(Path.GetFileName(path)));
828+
829+
if (invalidNugetConfigs.Count() > 0)
830+
{
831+
this.logger.LogWarning(string.Format(
832+
"Found incorrectly named NuGet configuration files: {0}",
833+
string.Join(", ", invalidNugetConfigs)
834+
));
835+
this.diagnosticsWriter.AddEntry(new DiagnosticMessage(
836+
Language.CSharp,
837+
"buildless/case-sensitive-nuget-config",
838+
"Found NuGet configuration files which are not correctly named",
839+
visibility: new DiagnosticMessage.TspVisibility(statusPage: true, cliSummaryTable: true, telemetry: true),
840+
markdownMessage: string.Format(
841+
"On platforms with case-sensitive file systems, NuGet only accepts files with one of the following names: {0}.\n\n" +
842+
"CodeQL found the following files while performing an analysis on a platform with a case-sensitive file system:\n\n" +
843+
"{1}\n\n" +
844+
"To avoid unexpected results, rename these files to match the casing of one of the accepted filenames.",
845+
string.Join(", ", acceptedNugetConfigNames),
846+
string.Join("\n", invalidNugetConfigs.Select(path => string.Format("- `{0}`", path)))
847+
),
848+
severity: DiagnosticMessage.TspSeverity.Warning
849+
));
850+
}
851+
}
852+
853+
// Find feeds that are explicitly configured in the NuGet configuration files that we found.
817854
var explicitFeeds = nugetConfigs
818855
.SelectMany(config => GetFeeds(() => dotnet.GetNugetFeeds(config)))
819856
.ToHashSet();
@@ -849,6 +886,14 @@ private IEnumerable<string> GetFeeds(Func<IList<string>> getNugetFeeds)
849886
.Where(folder => folder != null)
850887
.SelectMany(folder => GetFeeds(() => dotnet.GetNugetFeedsFromFolder(folder!)))
851888
.ToHashSet();
889+
890+
// If we have discovered any explicit feeds, then we also expect these to be in the set of all feeds.
891+
// Normally, it is a safe assumption to make that `GetNugetFeedsFromFolder` will include the feeds configured
892+
// in a NuGet configuration file in the given directory. There is one exception: on a system with case-sensitive
893+
// file systems, we may discover a configuration file such as `Nuget.Config` which is not recognised by `dotnet nuget`.
894+
// In that case, our call to `GetNugetFeeds` will retrieve the feeds from that file (because it is accepted when
895+
// provided explicitly as `--configfile` argument), but the call to `GetNugetFeedsFromFolder` will not.
896+
allFeeds.UnionWith(explicitFeeds);
852897
}
853898
else
854899
{

‎csharp/extractor/Semmle.Util/BuildActions.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ public interface IBuildActions
119119
/// <returns>True if we are running on macOS.</returns>
120120
bool IsMacOs();
121121

122+
/// <summary>
123+
/// Gets a value indicating whether we are running on Linux.
124+
/// </summary>
125+
/// <returns>True if we are running on Linux.</returns>
126+
bool IsLinux();
127+
122128
/// <summary>
123129
/// Gets a value indicating whether we are running on Apple Silicon.
124130
/// </summary>
@@ -246,6 +252,8 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory,
246252

247253
bool IBuildActions.IsMacOs() => RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
248254

255+
bool IBuildActions.IsLinux() => RuntimeInformation.IsOSPlatform(OSPlatform.Linux);
256+
249257
bool IBuildActions.IsRunningOnAppleSilicon()
250258
{
251259
var thisBuildActions = (IBuildActions)this;

‎csharp/ql/integration-tests/linux/diag_nuget_config_casing/Program.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class Program
2+
{
3+
static void Main(string[] args)
4+
{
5+
}
6+
}

‎csharp/ql/integration-tests/linux/diag_nuget_config_casing/diagnostics.expected‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{
2+
"markdownMessage": "C# analysis with build-mode 'none' completed.",
3+
"severity": "unknown",
4+
"source": {
5+
"extractorName": "csharp",
6+
"id": "csharp/autobuilder/buildless/complete",
7+
"name": "C# analysis with build-mode 'none' completed"
8+
},
9+
"visibility": {
10+
"cliSummaryTable": true,
11+
"statusPage": false,
12+
"telemetry": true
13+
}
14+
}
15+
{
16+
"markdownMessage": "C# was extracted with build-mode set to 'none'. This means that all C# source in the working directory will be scanned, with build tools, such as NuGet and dotnet CLIs, only contributing information about external dependencies.",
17+
"severity": "note",
18+
"source": {
19+
"extractorName": "csharp",
20+
"id": "csharp/autobuilder/buildless/mode-active",
21+
"name": "C# was extracted with build-mode set to 'none'"
22+
},
23+
"visibility": {
24+
"cliSummaryTable": true,
25+
"statusPage": true,
26+
"telemetry": true
27+
}
28+
}
29+
{
30+
"markdownMessage": "On platforms with case-sensitive file systems, NuGet only accepts files with one of the following names: nuget.config, NuGet.config, NuGet.Config.\n\nCodeQL found the following files while performing an analysis on a platform with a case-sensitive file system:\n\n- `<test-root-directory>/sub-project/Nuget.Config`\n\nTo avoid unexpected results, rename these files to match the casing of one of the accepted filenames.",
31+
"severity": "warning",
32+
"source": {
33+
"extractorName": "csharp",
34+
"id": "csharp/autobuilder/buildless/case-sensitive-nuget-config",
35+
"name": "Found NuGet configuration files which are not correctly named"
36+
},
37+
"visibility": {
38+
"cliSummaryTable": true,
39+
"statusPage": true,
40+
"telemetry": true
41+
}
42+
}

‎csharp/ql/integration-tests/linux/diag_nuget_config_casing/global.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"sdk": {
3+
"version": "10.0.100"
4+
}
5+
}

‎csharp/ql/integration-tests/linux/diag_nuget_config_casing/sub-project/Nuget.Config‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<configuration>
3+
<packageSources>
4+
</packageSources>
5+
</configuration>

‎csharp/ql/integration-tests/linux/diag_nuget_config_casing/test.csproj‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<PropertyGroup>
4+
<OutputType>Exe</OutputType>
5+
<TargetFramework>net10.0</TargetFramework>
6+
</PropertyGroup>
7+
8+
</Project>

‎csharp/ql/integration-tests/linux/diag_nuget_config_casing/test.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import runs_on
2+
3+
@runs_on.linux
4+
def test(codeql, csharp):
5+
codeql.database.create(build_mode="none")

0 commit comments

Comments
 (0)