From 159d31d494c52b11425dba7184dfcd2537c37c04 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Tue, 8 Apr 2025 13:25:14 +0200 Subject: [PATCH 1/4] Reenable problematic test --- .../all-platforms/diag_missing_project_files/test.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/csharp/ql/integration-tests/all-platforms/diag_missing_project_files/test.py b/csharp/ql/integration-tests/all-platforms/diag_missing_project_files/test.py index fa941f346df1..a2676d16d9c0 100644 --- a/csharp/ql/integration-tests/all-platforms/diag_missing_project_files/test.py +++ b/csharp/ql/integration-tests/all-platforms/diag_missing_project_files/test.py @@ -1,8 +1,2 @@ -import pytest -import runs_on - - -# Skipping the test on macos-15, as we're running into trouble. -@pytest.mark.only_if(not runs_on.macos_15) def test(codeql, csharp): codeql.database.create(_assert_failure=True) From 51388f24012fbf33d4321e74d2ed2a3c6806e9c1 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 11 Apr 2025 12:42:12 +0200 Subject: [PATCH 2/4] Do not try running mono when it's not available on the runner --- .../BuildScripts.cs | 35 ++++++++++++++++--- .../Semmle.Autobuild.Shared/MsBuildRule.cs | 20 +++++++---- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs b/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs index d277331b12e9..db445c591a52 100644 --- a/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs +++ b/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs @@ -46,6 +46,11 @@ bool IBuildActions.FileExists(string file) public IList RunProcessIn { get; } = new List(); public IDictionary RunProcess { get; } = new Dictionary(); + + /// + /// (process-exit code) pairs for commands that are executed during the assembly of the autobuild script. + /// + public IDictionary RunProcessExecuteDuring { get; } = new Dictionary(); public IDictionary RunProcessOut { get; } = new Dictionary(); public IDictionary RunProcessWorkingDirectory { get; } = new Dictionary(); public HashSet CreateDirectories { get; } = new HashSet(); @@ -66,7 +71,7 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, if (wd != workingDirectory) throw new ArgumentException($"Unexpected RunProcessWorkingDirectory, got {wd ?? "null"} expected {workingDirectory ?? "null"} in {pattern}"); - if (!RunProcess.TryGetValue(pattern, out var ret)) + if (!RunProcess.TryGetValue(pattern, out var ret) && !RunProcessExecuteDuring.TryGetValue(pattern, out ret)) throw new ArgumentException("Missing RunProcess " + pattern); return ret; @@ -81,7 +86,7 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, if (wd != workingDirectory) throw new ArgumentException($"Unexpected RunProcessWorkingDirectory, got {wd ?? "null"} expected {workingDirectory ?? "null"} in {pattern}"); - if (!RunProcess.TryGetValue(pattern, out var ret)) + if (!RunProcess.TryGetValue(pattern, out var ret) && !RunProcessExecuteDuring.TryGetValue(pattern, out ret)) throw new ArgumentException("Missing RunProcess " + pattern); return ret; @@ -797,11 +802,32 @@ public void TestDirsProjWindows() } [Fact] - public void TestDirsProjLinux() + public void TestDirsProjLinux_WithMono() { + actions.RunProcessExecuteDuring[@"mono --version"] = 0; + actions.RunProcess[@"nuget restore C:\Project/dirs.proj -DisableParallelProcessing"] = 1; actions.RunProcess[@"mono scratch/.nuget/nuget.exe restore C:\Project/dirs.proj -DisableParallelProcessing"] = 0; actions.RunProcess[@"msbuild C:\Project/dirs.proj /t:rebuild"] = 0; + + var autobuilder = TestDirsProjLinux(); + TestAutobuilderScript(autobuilder, 0, 3); + } + + [Fact] + public void TestDirsProjLinux_WithoutMono() + { + actions.RunProcessExecuteDuring[@"mono --version"] = 1; + + actions.RunProcess[@"dotnet msbuild /t:restore C:\Project/dirs.proj"] = 0; + actions.RunProcess[@"dotnet msbuild C:\Project/dirs.proj /t:rebuild"] = 0; + + var autobuilder = TestDirsProjLinux(); + TestAutobuilderScript(autobuilder, 0, 2); + } + + private CSharpAutobuilder TestDirsProjLinux() + { actions.FileExists["csharp.log"] = true; actions.FileExists[@"C:\Project/a/test.csproj"] = true; actions.FileExists[@"C:\Project/dirs.proj"] = true; @@ -830,8 +856,7 @@ public void TestDirsProjLinux() "); actions.LoadXml[@"C:\Project/dirs.proj"] = dirsproj; - var autobuilder = CreateAutoBuilder(false); - TestAutobuilderScript(autobuilder, 0, 3); + return CreateAutoBuilder(false); } [Fact] diff --git a/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs b/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs index 953f0884a44f..72ede13da42b 100644 --- a/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs +++ b/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs @@ -10,15 +10,13 @@ internal static class MsBuildCommandExtensions /// /// Appends a call to msbuild. /// - /// - /// /// - public static CommandBuilder MsBuildCommand(this CommandBuilder cmdBuilder, IAutobuilder builder) + public static CommandBuilder MsBuildCommand(this CommandBuilder cmdBuilder, IAutobuilder builder, bool preferDotnet) { // mono doesn't ship with `msbuild` on Arm-based Macs, but we can fall back to // msbuild that ships with `dotnet` which can be invoked with `dotnet msbuild` // perhaps we should do this on all platforms? - return builder.Actions.IsRunningOnAppleSilicon() + return builder.Actions.IsRunningOnAppleSilicon() || preferDotnet ? cmdBuilder.RunCommand("dotnet").Argument("msbuild") : cmdBuilder.RunCommand("msbuild"); } @@ -75,13 +73,21 @@ BuildScript GetNugetRestoreScript() => QuoteArgument(projectOrSolution.FullPath). Argument("-DisableParallelProcessing"). Script; + + BuildScript GetMonoVersionScript() => new CommandBuilder(builder.Actions). + RunCommand("mono"). + Argument("--version"). + Script; + + var preferDotnet = !builder.Actions.IsWindows() && GetMonoVersionScript().Run(builder.Actions, (_, _) => { }, (_, _, _) => { }) != 0; + var nugetRestore = GetNugetRestoreScript(); var msbuildRestoreCommand = new CommandBuilder(builder.Actions). - MsBuildCommand(builder). + MsBuildCommand(builder, preferDotnet). Argument("/t:restore"). QuoteArgument(projectOrSolution.FullPath); - if (builder.Actions.IsRunningOnAppleSilicon()) + if (builder.Actions.IsRunningOnAppleSilicon() || preferDotnet) { // On Apple Silicon, only try package restore with `dotnet msbuild /t:restore` ret &= BuildScript.Try(msbuildRestoreCommand.Script); @@ -119,7 +125,7 @@ BuildScript GetNugetRestoreScript() => command.RunCommand("set Platform=&& type NUL", quoteExe: false); } - command.MsBuildCommand(builder); + command.MsBuildCommand(builder, preferDotnet); command.QuoteArgument(projectOrSolution.FullPath); var target = "rebuild"; From 91daca1a6babb9bef353f8c54626c6b4c15bd696 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 14 Apr 2025 13:51:30 +0200 Subject: [PATCH 3/4] Improve code quality based on PR review --- .../BuildScripts.cs | 17 +++++++------ .../BuildScripts.cs | 4 ++++ .../Semmle.Autobuild.Shared/MsBuildRule.cs | 7 +----- csharp/extractor/Semmle.Util/BuildActions.cs | 24 +++++++++++++++++++ 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs b/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs index db445c591a52..eb75af79082f 100644 --- a/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs +++ b/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs @@ -46,11 +46,6 @@ bool IBuildActions.FileExists(string file) public IList RunProcessIn { get; } = new List(); public IDictionary RunProcess { get; } = new Dictionary(); - - /// - /// (process-exit code) pairs for commands that are executed during the assembly of the autobuild script. - /// - public IDictionary RunProcessExecuteDuring { get; } = new Dictionary(); public IDictionary RunProcessOut { get; } = new Dictionary(); public IDictionary RunProcessWorkingDirectory { get; } = new Dictionary(); public HashSet CreateDirectories { get; } = new HashSet(); @@ -71,7 +66,7 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, if (wd != workingDirectory) throw new ArgumentException($"Unexpected RunProcessWorkingDirectory, got {wd ?? "null"} expected {workingDirectory ?? "null"} in {pattern}"); - if (!RunProcess.TryGetValue(pattern, out var ret) && !RunProcessExecuteDuring.TryGetValue(pattern, out ret)) + if (!RunProcess.TryGetValue(pattern, out var ret)) throw new ArgumentException("Missing RunProcess " + pattern); return ret; @@ -86,7 +81,7 @@ int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, if (wd != workingDirectory) throw new ArgumentException($"Unexpected RunProcessWorkingDirectory, got {wd ?? "null"} expected {workingDirectory ?? "null"} in {pattern}"); - if (!RunProcess.TryGetValue(pattern, out var ret) && !RunProcessExecuteDuring.TryGetValue(pattern, out ret)) + if (!RunProcess.TryGetValue(pattern, out var ret)) throw new ArgumentException("Missing RunProcess " + pattern); return ret; @@ -167,6 +162,10 @@ IEnumerable IBuildActions.EnumerateDirectories(string dir) bool IBuildActions.IsRunningOnAppleSilicon() => IsRunningOnAppleSilicon; + public bool IsMonoInstalled { get; set; } + + bool IBuildActions.IsMonoInstalled() => IsMonoInstalled; + public string PathCombine(params string[] parts) { return string.Join(IsWindows ? '\\' : '/', parts.Where(p => !string.IsNullOrWhiteSpace(p))); @@ -804,7 +803,7 @@ public void TestDirsProjWindows() [Fact] public void TestDirsProjLinux_WithMono() { - actions.RunProcessExecuteDuring[@"mono --version"] = 0; + actions.IsMonoInstalled = true; actions.RunProcess[@"nuget restore C:\Project/dirs.proj -DisableParallelProcessing"] = 1; actions.RunProcess[@"mono scratch/.nuget/nuget.exe restore C:\Project/dirs.proj -DisableParallelProcessing"] = 0; @@ -817,7 +816,7 @@ public void TestDirsProjLinux_WithMono() [Fact] public void TestDirsProjLinux_WithoutMono() { - actions.RunProcessExecuteDuring[@"mono --version"] = 1; + actions.IsMonoInstalled = false; actions.RunProcess[@"dotnet msbuild /t:restore C:\Project/dirs.proj"] = 0; actions.RunProcess[@"dotnet msbuild C:\Project/dirs.proj /t:rebuild"] = 0; diff --git a/csharp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs b/csharp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs index c74692bed750..afa4ea4b41c3 100644 --- a/csharp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs +++ b/csharp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs @@ -150,6 +150,10 @@ IEnumerable IBuildActions.EnumerateDirectories(string dir) bool IBuildActions.IsRunningOnAppleSilicon() => IsRunningOnAppleSilicon; + public bool IsMonoInstalled { get; set; } + + bool IBuildActions.IsMonoInstalled() => IsMonoInstalled; + string IBuildActions.PathCombine(params string[] parts) { return string.Join(IsWindows ? '\\' : '/', parts.Where(p => !string.IsNullOrWhiteSpace(p))); diff --git a/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs b/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs index 72ede13da42b..4295f71df9d4 100644 --- a/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs +++ b/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs @@ -74,12 +74,7 @@ BuildScript GetNugetRestoreScript() => Argument("-DisableParallelProcessing"). Script; - BuildScript GetMonoVersionScript() => new CommandBuilder(builder.Actions). - RunCommand("mono"). - Argument("--version"). - Script; - - var preferDotnet = !builder.Actions.IsWindows() && GetMonoVersionScript().Run(builder.Actions, (_, _) => { }, (_, _, _) => { }) != 0; + var preferDotnet = !builder.Actions.IsWindows() && !builder.Actions.IsMonoInstalled(); var nugetRestore = GetNugetRestoreScript(); var msbuildRestoreCommand = new CommandBuilder(builder.Actions). diff --git a/csharp/extractor/Semmle.Util/BuildActions.cs b/csharp/extractor/Semmle.Util/BuildActions.cs index 507af96d13ca..38210402945b 100644 --- a/csharp/extractor/Semmle.Util/BuildActions.cs +++ b/csharp/extractor/Semmle.Util/BuildActions.cs @@ -125,6 +125,11 @@ public interface IBuildActions /// True if we are running on Apple Silicon. bool IsRunningOnAppleSilicon(); + /// + /// Checks if Mono is installed. + /// + bool IsMonoInstalled(); + /// /// Combine path segments, Path.Combine(). /// @@ -261,6 +266,25 @@ bool IBuildActions.IsRunningOnAppleSilicon() } } + bool IBuildActions.IsMonoInstalled() + { + var thisBuildActions = (IBuildActions)this; + + if (thisBuildActions.IsWindows()) + { + return false; + } + + try + { + return 0 == thisBuildActions.RunProcess("mono", "--version", workingDirectory: null, env: null); + } + catch (Exception) + { + return false; + } + } + string IBuildActions.PathCombine(params string[] parts) => Path.Combine(parts); void IBuildActions.WriteAllText(string filename, string contents) => File.WriteAllText(filename, contents); From a70536f002d5de3e8c8a57bf62b7c678d165a7ce Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 14 Apr 2025 14:54:56 +0200 Subject: [PATCH 4/4] Improve code quality --- csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs b/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs index 4295f71df9d4..748a22fb9d3c 100644 --- a/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs +++ b/csharp/autobuilder/Semmle.Autobuild.Shared/MsBuildRule.cs @@ -16,7 +16,9 @@ public static CommandBuilder MsBuildCommand(this CommandBuilder cmdBuilder, IAut // mono doesn't ship with `msbuild` on Arm-based Macs, but we can fall back to // msbuild that ships with `dotnet` which can be invoked with `dotnet msbuild` // perhaps we should do this on all platforms? - return builder.Actions.IsRunningOnAppleSilicon() || preferDotnet + // Similarly, there's no point in trying to rely on mono if it's not installed. + // In which case we can still fall back to `dotnet msbuild`. + return preferDotnet ? cmdBuilder.RunCommand("dotnet").Argument("msbuild") : cmdBuilder.RunCommand("msbuild"); } @@ -74,7 +76,7 @@ BuildScript GetNugetRestoreScript() => Argument("-DisableParallelProcessing"). Script; - var preferDotnet = !builder.Actions.IsWindows() && !builder.Actions.IsMonoInstalled(); + var preferDotnet = builder.Actions.IsRunningOnAppleSilicon() || !builder.Actions.IsWindows() && !builder.Actions.IsMonoInstalled(); var nugetRestore = GetNugetRestoreScript(); var msbuildRestoreCommand = new CommandBuilder(builder.Actions). @@ -82,7 +84,7 @@ BuildScript GetNugetRestoreScript() => Argument("/t:restore"). QuoteArgument(projectOrSolution.FullPath); - if (builder.Actions.IsRunningOnAppleSilicon() || preferDotnet) + if (preferDotnet) { // On Apple Silicon, only try package restore with `dotnet msbuild /t:restore` ret &= BuildScript.Try(msbuildRestoreCommand.Script);