Skip to content

Commit b034b2f

Browse files
mbgmbg
committed
Refactor autobuild logic into an IBuildRule 
1 parent 93b7a2b commit b034b2f

File tree

3 files changed

+105
-80
lines changed

3 files changed

+105
-80
lines changed

‎csharp/autobuilder/Semmle.Autobuild.CSharp/AutoBuildRule.cs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
using Semmle.Autobuild.Shared;
2+
using Semmle.Extraction.CSharp;
3+
4+
namespace Semmle.Autobuild.CSharp
5+
{
6+
internal class AutoBuildRule : IBuildRule<CSharpAutobuildOptions>
7+
{
8+
private readonly CSharpAutobuilder autobuilder;
9+
10+
public DotNetRule DotNetRule { get; }
11+
12+
public MsBuildRule MsBuildRule { get; }
13+
14+
public BuildCommandAutoRule BuildCommandAutoRule { get; }
15+
16+
public AutoBuildRule(CSharpAutobuilder autobuilder)
17+
{
18+
this.autobuilder = autobuilder;
19+
this.DotNetRule = new DotNetRule();
20+
this.MsBuildRule = new MsBuildRule();
21+
this.BuildCommandAutoRule = new BuildCommandAutoRule(DotNetRule.WithDotNet);
22+
}
23+
24+
public BuildScript Analyse(IAutobuilder<CSharpAutobuildOptions> builder, bool auto)
25+
{
26+
var cleanTrapFolder =
27+
BuildScript.DeleteDirectory(this.autobuilder.TrapDir);
28+
var cleanSourceArchive =
29+
BuildScript.DeleteDirectory(this.autobuilder.SourceArchiveDir);
30+
var tryCleanExtractorArgsLogs =
31+
BuildScript.Create(actions =>
32+
{
33+
foreach (var file in Extractor.GetCSharpArgsLogs())
34+
{
35+
try
36+
{
37+
actions.FileDelete(file);
38+
}
39+
catch // lgtm[cs/catch-of-all-exceptions] lgtm[cs/empty-catch-block]
40+
{ }
41+
}
42+
43+
return 0;
44+
});
45+
var attemptExtractorCleanup =
46+
BuildScript.Try(cleanTrapFolder) &
47+
BuildScript.Try(cleanSourceArchive) &
48+
tryCleanExtractorArgsLogs &
49+
BuildScript.DeleteFile(Extractor.GetCSharpLogPath());
50+
51+
/// <summary>
52+
/// Execute script `s` and check that the C# extractor has been executed.
53+
/// If either fails, attempt to cleanup any artifacts produced by the extractor,
54+
/// and exit with code 1, in order to proceed to the next attempt.
55+
/// </summary>
56+
BuildScript IntermediateAttempt(BuildScript s) =>
57+
(s & this.autobuilder.CheckExtractorRun(false)) |
58+
(attemptExtractorCleanup & BuildScript.Failure);
59+
60+
return
61+
// First try .NET Core
62+
IntermediateAttempt(this.DotNetRule.Analyse(this.autobuilder, true)) |
63+
// Then MSBuild
64+
(() => IntermediateAttempt(this.MsBuildRule.Analyse(this.autobuilder, true))) |
65+
// And finally look for a script that might be a build script
66+
(() => this.BuildCommandAutoRule.Analyse(this.autobuilder, true) & this.autobuilder.CheckExtractorRun(true));
67+
}
68+
}
69+
}

‎csharp/autobuilder/Semmle.Autobuild.CSharp/CSharpAutobuilder.cs

Lines changed: 33 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -34,36 +34,20 @@ public class CSharpAutobuilder : Autobuilder<CSharpAutobuildOptions>
3434
private const string buildCommandDocsUrl =
3535
"https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages";
3636

37-
private DotNetRule? dotNetRule;
38-
39-
private MsBuildRule? msBuildRule;
40-
41-
private BuildCommandAutoRule? buildCommandAutoRule;
37+
private readonly AutoBuildRule autoBuildRule;
4238

4339
private readonly DiagnosticClassifier diagnosticClassifier;
4440

4541
protected override DiagnosticClassifier DiagnosticClassifier => diagnosticClassifier;
4642

47-
public CSharpAutobuilder(IBuildActions actions, CSharpAutobuildOptions options) : base(actions, options) =>
48-
diagnosticClassifier = new CSharpDiagnosticClassifier();
43+
public CSharpAutobuilder(IBuildActions actions, CSharpAutobuildOptions options) : base(actions, options)
44+
{
45+
this.autoBuildRule = new AutoBuildRule(this);
46+
this.diagnosticClassifier = new CSharpDiagnosticClassifier();
47+
}
4948

5049
public override BuildScript GetBuildScript()
5150
{
52-
/// <summary>
53-
/// A script that checks that the C# extractor has been executed.
54-
/// </summary>
55-
BuildScript CheckExtractorRun(bool warnOnFailure) =>
56-
BuildScript.Create(actions =>
57-
{
58-
if (actions.FileExists(Extractor.GetCSharpLogPath()))
59-
return 0;
60-
61-
if (warnOnFailure)
62-
Log(Severity.Error, "No C# code detected during build.");
63-
64-
return 1;
65-
});
66-
6751
var attempt = BuildScript.Failure;
6852
switch (GetCSharpBuildStrategy())
6953
{
@@ -81,51 +65,9 @@ BuildScript CheckExtractorRun(bool warnOnFailure) =>
8165
attempt = new DotNetRule().Analyse(this, false) & CheckExtractorRun(true);
8266
break;
8367
case CSharpBuildStrategy.Auto:
84-
var cleanTrapFolder =
85-
BuildScript.DeleteDirectory(TrapDir);
86-
var cleanSourceArchive =
87-
BuildScript.DeleteDirectory(SourceArchiveDir);
88-
var tryCleanExtractorArgsLogs =
89-
BuildScript.Create(actions =>
90-
{
91-
foreach (var file in Extractor.GetCSharpArgsLogs())
92-
{
93-
try
94-
{
95-
actions.FileDelete(file);
96-
}
97-
catch // lgtm[cs/catch-of-all-exceptions] lgtm[cs/empty-catch-block]
98-
{ }
99-
}
100-
101-
return 0;
102-
});
103-
var attemptExtractorCleanup =
104-
BuildScript.Try(cleanTrapFolder) &
105-
BuildScript.Try(cleanSourceArchive) &
106-
tryCleanExtractorArgsLogs &
107-
BuildScript.DeleteFile(Extractor.GetCSharpLogPath());
108-
109-
/// <summary>
110-
/// Execute script `s` and check that the C# extractor has been executed.
111-
/// If either fails, attempt to cleanup any artifacts produced by the extractor,
112-
/// and exit with code 1, in order to proceed to the next attempt.
113-
/// </summary>
114-
BuildScript IntermediateAttempt(BuildScript s) =>
115-
(s & CheckExtractorRun(false)) |
116-
(attemptExtractorCleanup & BuildScript.Failure);
117-
118-
this.dotNetRule = new DotNetRule();
119-
this.msBuildRule = new MsBuildRule();
120-
this.buildCommandAutoRule = new BuildCommandAutoRule(DotNetRule.WithDotNet);
121-
12268
attempt =
123-
// First try .NET Core
124-
IntermediateAttempt(dotNetRule.Analyse(this, true)) |
125-
// Then MSBuild
126-
(() => IntermediateAttempt(msBuildRule.Analyse(this, true))) |
127-
// And finally look for a script that might be a build script
128-
(() => this.buildCommandAutoRule.Analyse(this, true) & CheckExtractorRun(true)) |
69+
// Attempt a few different build strategies to see if one works
70+
this.autoBuildRule.Analyse(this, true) |
12971
// All attempts failed: print message
13072
AutobuildFailure();
13173
break;
@@ -134,24 +76,38 @@ BuildScript IntermediateAttempt(BuildScript s) =>
13476
return attempt;
13577
}
13678

79+
/// <summary>
80+
/// A script that checks that the C# extractor has been executed.
81+
/// </summary>
82+
public BuildScript CheckExtractorRun(bool warnOnFailure) =>
83+
BuildScript.Create(actions =>
84+
{
85+
if (actions.FileExists(Extractor.GetCSharpLogPath()))
86+
return 0;
87+
88+
if (warnOnFailure)
89+
Log(Severity.Error, "No C# code detected during build.");
90+
91+
return 1;
92+
});
93+
13794
protected override void AutobuildFailureDiagnostic()
13895
{
13996
// if `ScriptPath` is not null here, the `BuildCommandAuto` rule was
14097
// run and found at least one script to execute
141-
if (this.buildCommandAutoRule is not null &&
142-
this.buildCommandAutoRule.ScriptPath is not null)
98+
if (this.autoBuildRule.BuildCommandAutoRule.ScriptPath is not null)
14399
{
144100
DiagnosticMessage message;
145101

146102
// if we found multiple build scripts in the project directory, then we can say
147103
// as much to indicate that we may have picked the wrong one;
148104
// otherwise, we just report that the one script we found didn't work
149-
if (this.buildCommandAutoRule.CandidatePaths.Count() > 1)
105+
if (this.autoBuildRule.BuildCommandAutoRule.CandidatePaths.Count() > 1)
150106
{
151107
message = MakeDiagnostic("multiple-build-scripts", "There are multiple potential build scripts");
152108
message.MarkdownMessage =
153109
"CodeQL found multiple potential build scripts for your project and " +
154-
$"attempted to run `{buildCommandAutoRule.ScriptPath}`, which failed. " +
110+
$"attempted to run `{autoBuildRule.BuildCommandAutoRule.ScriptPath}`, which failed. " +
155111
"This may not be the right build script for your project. " +
156112
$"Set up a [manual build command]({buildCommandDocsUrl}).";
157113
}
@@ -160,7 +116,7 @@ protected override void AutobuildFailureDiagnostic()
160116
message = MakeDiagnostic("script-failure", "Unable to build project using build script");
161117
message.MarkdownMessage =
162118
"CodeQL attempted to build your project using a script located at " +
163-
$"`{buildCommandAutoRule.ScriptPath}`, which failed. " +
119+
$"`{autoBuildRule.BuildCommandAutoRule.ScriptPath}`, which failed. " +
164120
$"Set up a [manual build command]({buildCommandDocsUrl}).";
165121
}
166122

@@ -180,37 +136,37 @@ protected override void AutobuildFailureDiagnostic()
180136

181137
AddDiagnostic(message);
182138
}
183-
else if (dotNetRule is not null && dotNetRule.NotDotNetProjects.Any())
139+
else if (autoBuildRule.DotNetRule.NotDotNetProjects.Any())
184140
{
185141
var message = MakeDiagnostic("dotnet-incompatible-projects", "Some projects are incompatible with .NET Core");
186142
message.MarkdownMessage =
187143
"CodeQL found some projects which cannot be built with .NET Core:\n" +
188-
string.Join('\n', dotNetRule.NotDotNetProjects.Select(p => $"- `{p.FullPath}`"));
144+
string.Join('\n', autoBuildRule.DotNetRule.NotDotNetProjects.Select(p => $"- `{p.FullPath}`"));
189145
message.Severity = DiagnosticMessage.TspSeverity.Warning;
190146

191147
AddDiagnostic(message);
192148
}
193149

194150
// report any projects that failed to build with .NET Core
195-
if (dotNetRule is not null && dotNetRule.FailedProjectsOrSolutions.Any())
151+
if (autoBuildRule.DotNetRule.FailedProjectsOrSolutions.Any())
196152
{
197153
var message = MakeDiagnostic("dotnet-build-failure", "Some projects or solutions failed to build using .NET Core");
198154
message.MarkdownMessage =
199155
"CodeQL was unable to build the following projects using .NET Core:\n" +
200-
string.Join('\n', dotNetRule.FailedProjectsOrSolutions.Select(p => $"- `{p.FullPath}`")) +
156+
string.Join('\n', autoBuildRule.DotNetRule.FailedProjectsOrSolutions.Select(p => $"- `{p.FullPath}`")) +
201157
$"\nSet up a [manual build command]({buildCommandDocsUrl}).";
202158
message.Severity = DiagnosticMessage.TspSeverity.Error;
203159

204160
AddDiagnostic(message);
205161
}
206162

207163
// report any projects that failed to build with MSBuild
208-
if (msBuildRule is not null && msBuildRule.FailedProjectsOrSolutions.Any())
164+
if (autoBuildRule.MsBuildRule.FailedProjectsOrSolutions.Any())
209165
{
210166
var message = MakeDiagnostic("msbuild-build-failure", "Some projects or solutions failed to build using MSBuild");
211167
message.MarkdownMessage =
212168
"CodeQL was unable to build the following projects using MSBuild:\n" +
213-
string.Join('\n', msBuildRule.FailedProjectsOrSolutions.Select(p => $"- `{p.FullPath}`")) +
169+
string.Join('\n', autoBuildRule.MsBuildRule.FailedProjectsOrSolutions.Select(p => $"- `{p.FullPath}`")) +
214170
$"\nSet up a [manual build command]({buildCommandDocsUrl}).";
215171
message.Severity = DiagnosticMessage.TspSeverity.Error;
216172

‎csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,11 @@ protected string RequireEnvironmentVariable(string name)
258258
throw new InvalidEnvironmentException($"The environment variable {name} has not been set.");
259259
}
260260

261-
protected string TrapDir { get; }
261+
public string TrapDir { get; }
262262

263-
protected string SourceArchiveDir { get; }
263+
public string SourceArchiveDir { get; }
264264

265-
protected string DiagnosticsDir { get; }
265+
public string DiagnosticsDir { get; }
266266

267267
protected abstract DiagnosticClassifier DiagnosticClassifier { get; }
268268

0 commit comments

Comments
 (0)