Skip to content

C#: Autobuilder fixes #609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adityasharad merged 2 commits into from
Dec 10, 2018
Merged

C#: Autobuilder fixes #609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad77afe
C#: Autobuilder fixes
hvitved Dec 4, 2018
1e9fe00
C#: Address review comments
hvitved Dec 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 72 additions & 48 deletions csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild/AspBuildRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Semmle.Autobuild
/// </summary>
class AspBuildRule : IBuildRule
{
public BuildScript Analyse(Autobuilder builder)
public BuildScript Analyse(Autobuilder builder, bool auto)
{
var command = new CommandBuilder(builder.Actions).
RunCommand(builder.Actions.PathCombine(builder.SemmleJavaHome, "bin", "java")).
Expand Down
28 changes: 23 additions & 5 deletions csharp/autobuilder/Semmle.Autobuild/AutobuildOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Linq;
using System.Text.RegularExpressions;

namespace Semmle.Autobuild
{
Expand Down Expand Up @@ -37,14 +38,14 @@ public void ReadEnvironment(IBuildActions actions)
{
RootDirectory = actions.GetCurrentDirectory();
VsToolsVersion = actions.GetEnvironmentVariable(prefix + "VSTOOLS_VERSION");
MsBuildArguments = actions.GetEnvironmentVariable(prefix + "MSBUILD_ARGUMENTS");
MsBuildArguments = actions.GetEnvironmentVariable(prefix + "MSBUILD_ARGUMENTS").AsStringWithExpandedEnvVars(actions);
MsBuildPlatform = actions.GetEnvironmentVariable(prefix + "MSBUILD_PLATFORM");
MsBuildConfiguration = actions.GetEnvironmentVariable(prefix + "MSBUILD_CONFIGURATION");
MsBuildTarget = actions.GetEnvironmentVariable(prefix + "MSBUILD_TARGET");
DotNetArguments = actions.GetEnvironmentVariable(prefix + "DOTNET_ARGUMENTS");
DotNetArguments = actions.GetEnvironmentVariable(prefix + "DOTNET_ARGUMENTS").AsStringWithExpandedEnvVars(actions);
DotNetVersion = actions.GetEnvironmentVariable(prefix + "DOTNET_VERSION");
BuildCommand = actions.GetEnvironmentVariable(prefix + "BUILD_COMMAND");
Solution = actions.GetEnvironmentVariable(prefix + "SOLUTION").AsList(new string[0]);
Solution = actions.GetEnvironmentVariable(prefix + "SOLUTION").AsListWithExpandedEnvVars(actions, new string[0]);

IgnoreErrors = actions.GetEnvironmentVariable(prefix + "IGNORE_ERRORS").AsBool("ignore_errors", false);
Buildless = actions.GetEnvironmentVariable(prefix + "BUILDLESS").AsBool("buildless", false);
Expand Down Expand Up @@ -92,12 +93,29 @@ public static Language AsLanguage(this string key)
}
}

public static string[] AsList(this string value, string[] defaultValue)
public static string[] AsListWithExpandedEnvVars(this string value, IBuildActions actions, string[] defaultValue)
{
if (value == null)
return defaultValue;

return value.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).ToArray();
return value.
Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).
Select(s => AsStringWithExpandedEnvVars(s, actions)).ToArray();
}

static readonly Regex linuxEnvRegEx = new Regex(@"\$([a-zA-Z_][a-zA-Z_0-9]*)", RegexOptions.Compiled);

public static string AsStringWithExpandedEnvVars(this string value, IBuildActions actions)
{
if (string.IsNullOrEmpty(value))
return value;

// `Environment.ExpandEnvironmentVariables` only works with Windows-style
// environment variables
var windowsStyle = actions.IsWindows()
Copy link
Contributor

@calumgrant calumgrant Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably support both expansion styles on all platforms. In particular, the documentation doesn't even mention the Windows-style at all, and users may expect it to work using $ even on Windows.

Copy link
Contributor Author

@hvitved hvitved Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM_SRC is introduced as an environment variable here, so I think we should use the syntax for environment variables on the given platform. In the same way, if LGTM_SRC is used in a build_command, it is interpreted as a command script on Windows, so will have to use %LGTM_SRC% instead of $LGTM_SRC.

Copy link
Contributor

@calumgrant calumgrant Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good argument.

? value
: linuxEnvRegEx.Replace(value, m => $"%{m.Groups[1].Value}%");
return actions.EnvironmentExpandEnvironmentVariables(windowsStyle);
}
}
}
33 changes: 17 additions & 16 deletions csharp/autobuilder/Semmle.Autobuild/Autobuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ interface IBuildRule
/// Analyse the files and produce a build script.
/// </summary>
/// <param name="builder">The files and options relating to the build.</param>
BuildScript Analyse(Autobuilder builder);
/// <param name="auto">Whether this build rule is being automatically applied.</param>
BuildScript Analyse(Autobuilder builder, bool auto);
}

/// <summary>
Expand Down Expand Up @@ -135,9 +136,9 @@ public Autobuilder(IBuildActions actions, AutobuildOptions options)
foreach (var solution in options.Solution)
{
if (actions.FileExists(solution))
ret.Add(new Solution(this, solution));
ret.Add(new Solution(this, solution, true));
else
Log(Severity.Error, $"The specified solution file {solution} was not found");
Log(Severity.Error, $"The specified project or solution file {solution} was not found");
}
return ret;
}
Expand Down Expand Up @@ -172,7 +173,7 @@ IEnumerable<IProjectOrSolution> FindFiles(string extension, Func<string, Project
return ret;

// Then look for `.sln` files
ret = FindFiles(".sln", f => new Solution(this, f))?.ToList();
ret = FindFiles(".sln", f => new Solution(this, f, false))?.ToList();
if (ret != null)
return ret;

Expand Down Expand Up @@ -250,17 +251,17 @@ BuildScript CheckExtractorRun(bool warnOnFailure) =>
switch (GetCSharpBuildStrategy())
{
case CSharpBuildStrategy.CustomBuildCommand:
attempt = new BuildCommandRule().Analyse(this) & CheckExtractorRun(true);
attempt = new BuildCommandRule().Analyse(this, false) & CheckExtractorRun(true);
break;
case CSharpBuildStrategy.Buildless:
// No need to check that the extractor has been executed in buildless mode
attempt = new StandaloneBuildRule().Analyse(this);
attempt = new StandaloneBuildRule().Analyse(this, false);
break;
case CSharpBuildStrategy.MSBuild:
attempt = new MsBuildRule().Analyse(this) & CheckExtractorRun(true);
attempt = new MsBuildRule().Analyse(this, false) & CheckExtractorRun(true);
break;
case CSharpBuildStrategy.DotNet:
attempt = new DotNetRule().Analyse(this) & CheckExtractorRun(true);
attempt = new DotNetRule().Analyse(this, false) & CheckExtractorRun(true);
break;
case CSharpBuildStrategy.Auto:
var cleanTrapFolder =
Expand All @@ -285,20 +286,20 @@ BuildScript IntermediateAttempt(BuildScript s) =>

attempt =
// First try .NET Core
IntermediateAttempt(new DotNetRule().Analyse(this)) |
IntermediateAttempt(new DotNetRule().Analyse(this, true)) |
// Then MSBuild
(() => IntermediateAttempt(new MsBuildRule().Analyse(this))) |
(() => IntermediateAttempt(new MsBuildRule().Analyse(this, true))) |
// And finally look for a script that might be a build script
(() => new BuildCommandAutoRule().Analyse(this) & CheckExtractorRun(true)) |
(() => new BuildCommandAutoRule().Analyse(this, true) & CheckExtractorRun(true)) |
// All attempts failed: print message
AutobuildFailure();
break;
}

return
attempt &
(() => new AspBuildRule().Analyse(this)) &
(() => new XmlBuildRule().Analyse(this));
(() => new AspBuildRule().Analyse(this, false)) &
(() => new XmlBuildRule().Analyse(this, false));
}

/// <summary>
Expand Down Expand Up @@ -337,13 +338,13 @@ enum CSharpBuildStrategy
BuildScript GetCppBuildScript()
{
if (Options.BuildCommand != null)
return new BuildCommandRule().Analyse(this);
return new BuildCommandRule().Analyse(this, false);

return
// First try MSBuild
new MsBuildRule().Analyse(this) |
new MsBuildRule().Analyse(this, true) |
// Then look for a script that might be a build script
(() => new BuildCommandAutoRule().Analyse(this)) |
(() => new BuildCommandAutoRule().Analyse(this, true)) |
// All attempts failed: print message
AutobuildFailure();
}
Expand Down
8 changes: 8 additions & 0 deletions csharp/autobuilder/Semmle.Autobuild/BuildActions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ public interface IBuildActions
/// Loads the XML document from <paramref name="filename"/>.
/// </summary>
XmlDocument LoadXml(string filename);

/// <summary>
/// Expand all Windows-style environment variables in <paramref name="s"/>,
/// Environment.ExpandEnvironmentVariables()
/// </summary>
string EnvironmentExpandEnvironmentVariables(string s);
}

/// <summary>
Expand Down Expand Up @@ -187,6 +193,8 @@ XmlDocument IBuildActions.LoadXml(string filename)

string IBuildActions.GetFullPath(string path) => Path.GetFullPath(path);

public string EnvironmentExpandEnvironmentVariables(string s) => Environment.ExpandEnvironmentVariables(s);

public static readonly IBuildActions Instance = new SystemBuildActions();
}
}
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild/BuildCommandAutoRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class BuildCommandAutoRule : IBuildRule
"build"
};

public BuildScript Analyse(Autobuilder builder)
public BuildScript Analyse(Autobuilder builder, bool auto)
{
builder.Log(Severity.Info, "Attempting to locate build script");

Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild/BuildCommandRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/// </summary>
class BuildCommandRule : IBuildRule
{
public BuildScript Analyse(Autobuilder builder)
public BuildScript Analyse(Autobuilder builder, bool auto)
{
if (builder.Options.BuildCommand == null)
return BuildScript.Failure;
Expand Down
23 changes: 13 additions & 10 deletions csharp/autobuilder/Semmle.Autobuild/DotNetRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,25 @@ namespace Semmle.Autobuild
/// </summary>
class DotNetRule : IBuildRule
{
public BuildScript Analyse(Autobuilder builder)
public BuildScript Analyse(Autobuilder builder, bool auto)
{
if (!builder.ProjectsOrSolutionsToBuild.Any())
return BuildScript.Failure;

var notDotNetProject = builder.ProjectsOrSolutionsToBuild.
SelectMany(p => Enumerators.Singleton(p).Concat(p.IncludedProjects)).
OfType<Project>().
FirstOrDefault(p => !p.DotNetProject);
if (notDotNetProject != null)
if (auto)
{
builder.Log(Severity.Info, "Not using .NET Core because of incompatible project {0}", notDotNetProject);
return BuildScript.Failure;
}
var notDotNetProject = builder.ProjectsOrSolutionsToBuild.
SelectMany(p => Enumerators.Singleton(p).Concat(p.IncludedProjects)).
OfType<Project>().
FirstOrDefault(p => !p.DotNetProject);
if (notDotNetProject != null)
{
builder.Log(Severity.Info, "Not using .NET Core because of incompatible project {0}", notDotNetProject);
return BuildScript.Failure;
}

builder.Log(Severity.Info, "Attempting to build using .NET Core");
builder.Log(Severity.Info, "Attempting to build using .NET Core");
}

return WithDotNet(builder, dotNet =>
{
Expand Down
5 changes: 3 additions & 2 deletions csharp/autobuilder/Semmle.Autobuild/MsBuildRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ class MsBuildRule : IBuildRule
/// </summary>
const string MsBuild = "msbuild";

public BuildScript Analyse(Autobuilder builder)
public BuildScript Analyse(Autobuilder builder, bool auto)
{
if (!builder.ProjectsOrSolutionsToBuild.Any())
return BuildScript.Failure;

builder.Log(Severity.Info, "Attempting to build using MSBuild");
if (auto)
builder.Log(Severity.Info, "Attempting to build using MSBuild");

var vsTools = GetVcVarsBatFile(builder);

Expand Down
6 changes: 3 additions & 3 deletions csharp/autobuilder/Semmle.Autobuild/Project.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ public Project(Autobuilder builder, string path) : base(builder, path)
{
projFile = builder.Actions.LoadXml(FullPath);
}
catch (XmlException)
catch (Exception e) when (e is XmlException || e is FileNotFoundException)
{
builder.Log(Severity.Info, $"Skipping project file {path} as it is not a valid XML document.");
builder.Log(Severity.Info, $"Unable to read project file {path}.");
return;
}

Expand Down Expand Up @@ -80,7 +80,7 @@ public Project(Autobuilder builder, string path) : base(builder, path)
var projectFilesIncludes = root.SelectNodes("//msbuild:Project/msbuild:ItemGroup/msbuild:ProjectFiles/@Include", mgr).OfType<XmlNode>();
foreach (var include in projectFileIncludes.Concat(projectFilesIncludes))
{
var includePath = builder.Actions.IsWindows() ? include.Value : include.Value.Replace("\\", "/");
var includePath = builder.Actions.PathCombine(include.Value.Split('\\', StringSplitOptions.RemoveEmptyEntries));
ret.Add(new Project(builder, builder.Actions.PathCombine(Path.GetDirectoryName(this.FullPath), includePath)));
}
return ret;
Expand Down
30 changes: 20 additions & 10 deletions csharp/autobuilder/Semmle.Autobuild/Solution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System.Collections.Generic;
using System.Linq;
using Semmle.Util;
using System.IO;
using Semmle.Util.Logging;

namespace Semmle.Autobuild
{
Expand Down Expand Up @@ -55,25 +57,33 @@ class Solution : ProjectOrSolution, ISolution
public string DefaultPlatformName =>
solution == null ? "" : solution.GetDefaultPlatformName();

public Solution(Autobuilder builder, string path) : base(builder, path)
public Solution(Autobuilder builder, string path, bool allowProject) : base(builder, path)
{
try
{
solution = SolutionFile.Parse(FullPath);

includedProjects =
solution.ProjectsInOrder.
Where(p => p.ProjectType == SolutionProjectType.KnownToBeMSBuildFormat).
Select(p => builder.Actions.GetFullPath(FileUtils.ConvertToNative(p.AbsolutePath))).
Select(p => new Project(builder, p)).
ToArray();
}
catch (InvalidProjectFileException)
catch (Exception e) when (e is InvalidProjectFileException || e is FileNotFoundException)
{
// We allow specifying projects as solutions in lgtm.yml, so model
// that scenario as a solution with just that one project
includedProjects = new[] { new Project(builder, path) };
if (allowProject)
{
includedProjects = new[] { new Project(builder, path) };
return;
}

builder.Log(Severity.Info, $"Unable to read solution file {path}.");
includedProjects = new Project[0];
return;
}

includedProjects =
solution.ProjectsInOrder.
Where(p => p.ProjectType == SolutionProjectType.KnownToBeMSBuildFormat).
Select(p => builder.Actions.PathCombine(Path.GetDirectoryName(path), builder.Actions.PathCombine(p.RelativePath.Split('\\', StringSplitOptions.RemoveEmptyEntries)))).
Select(p => new Project(builder, p)).
ToArray();
}

IEnumerable<Version> ToolsVersions => includedProjects.Where(p => p.ValidToolsVersion).Select(p => p.ToolsVersion);
Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild/StandaloneBuildRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Semmle.Autobuild
/// </summary>
class StandaloneBuildRule : IBuildRule
{
public BuildScript Analyse(Autobuilder builder)
public BuildScript Analyse(Autobuilder builder, bool auto)
{
BuildScript GetCommand(string solution)
{
Expand Down
2 changes: 1 addition & 1 deletion csharp/autobuilder/Semmle.Autobuild/XmlBuildRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Semmle.Autobuild
/// </summary>
class XmlBuildRule : IBuildRule
{
public BuildScript Analyse(Autobuilder builder)
public BuildScript Analyse(Autobuilder builder, bool auto)
{
var command = new CommandBuilder(builder.Actions).
RunCommand(builder.Odasa).
Expand Down