From 17b7347d75c418e2171cd4d73d39ef449873d1d4 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 7 Nov 2016 22:12:49 -0800 Subject: [PATCH] Verification for our props and targets files --- build/Targets/Imports.targets | 22 ++++ build/Targets/Microsoft.Net.Compilers.props | 17 +-- build/Targets/README.md | 14 +++ build/Targets/Settings.props | 22 ---- cibuild.cmd | 2 +- src/Tools/BuildBoss/BuildBoss.csproj | 1 + src/Tools/BuildBoss/Program.cs | 32 +++++- src/Tools/BuildBoss/ProjectUtil.cs | 17 +++ src/Tools/BuildBoss/README.md | 1 + src/Tools/BuildBoss/SharedUtil.cs | 6 + src/Tools/BuildBoss/TargetsCheckerUtil.cs | 119 ++++++++++++++++++++ 11 files changed, 210 insertions(+), 43 deletions(-) create mode 100644 src/Tools/BuildBoss/TargetsCheckerUtil.cs diff --git a/build/Targets/Imports.targets b/build/Targets/Imports.targets index 639a10ea7a6..f51a2148e30 100644 --- a/build/Targets/Imports.targets +++ b/build/Targets/Imports.targets @@ -367,6 +367,28 @@ $([System.IO.Path]::GetFullPath('$(OptimizationDataFolderPath)\$(TargetName).pgo')) + + + + + + + + + + + + + - - - - - + $(NuGetPackageRoot)\Microsoft.Net.Compilers\$(ToolsetCompilerPackageVersion)\tools diff --git a/build/Targets/README.md b/build/Targets/README.md index d7a3bf242c8..019e4a9587b 100644 --- a/build/Targets/README.md +++ b/build/Targets/README.md @@ -8,6 +8,20 @@ To accomplish this all of the build logic is contained in our central targets fi The individual project files contain declaritive information only. They inherit their build logic by importing [Settings.props](Settings.props) at the start and [Imports.targets](Imports.targets) at the conclusion. +## General rules + +There are a set of general rules to follow for props and targets files: + +- props files + - Do use Import for props files. + - Do not use Import for targets files. + - Do use UsingTask elements. + - Do not use Target elements +- targets files + - Do use Import for targets files. + - Do not use Import for props files. + - Do use Task elements. + ## Files This section describes the purpose and layout of the important files here. diff --git a/build/Targets/Settings.props b/build/Targets/Settings.props index c6c3737e7a7..c90f855cd4d 100644 --- a/build/Targets/Settings.props +++ b/build/Targets/Settings.props @@ -252,26 +252,4 @@ - - - - - - - - - - - - - diff --git a/cibuild.cmd b/cibuild.cmd index 81ff26d7808..4fd0ae5164d 100644 --- a/cibuild.cmd +++ b/cibuild.cmd @@ -120,7 +120,7 @@ echo Running RepoUtil REM Verify the state of our project.jsons echo Running BuildBoss -.\Binaries\%BuildConfiguration%\Exes\BuildBoss\BuildBoss.exe Roslyn.sln Compilers.sln src\Samples\Samples.sln CrossPlatform.sln || goto :BuildFailed +.\Binaries\%BuildConfiguration%\Exes\BuildBoss\BuildBoss.exe Roslyn.sln Compilers.sln src\Samples\Samples.sln CrossPlatform.sln build\Targets || goto :BuildFailed REM Ensure caller sees successful exit. exit /b 0 diff --git a/src/Tools/BuildBoss/BuildBoss.csproj b/src/Tools/BuildBoss/BuildBoss.csproj index 46926cc2b41..6906e59fafb 100644 --- a/src/Tools/BuildBoss/BuildBoss.csproj +++ b/src/Tools/BuildBoss/BuildBoss.csproj @@ -32,6 +32,7 @@ + diff --git a/src/Tools/BuildBoss/Program.cs b/src/Tools/BuildBoss/Program.cs index f498b82abfe..dfad0899c1e 100644 --- a/src/Tools/BuildBoss/Program.cs +++ b/src/Tools/BuildBoss/Program.cs @@ -11,18 +11,25 @@ namespace BuildBoss { internal static class Program { - internal static int Main(string[] solutionFilePaths) + internal static int Main(string[] args) { - if (solutionFilePaths.Length == 0) + if (args.Length == 0) { Usage(); return 1; } var allGood = true; - foreach (var solutionFilePath in solutionFilePaths) + foreach (var arg in args) { - allGood &= ProcessSolution(solutionFilePath); + if (SharedUtil.IsSolutionFile(arg)) + { + allGood &= ProcessSolution(arg); + } + else + { + allGood &= ProcessTargets(arg); + } } return allGood ? 0 : 1; @@ -78,6 +85,23 @@ private static bool ProcessProject(string solutionPath, ProjectData projectData, return true; } + private static bool ProcessTargets(string targets) + { + var checker = new TargetsCheckerUtil(targets); + var textWriter = new StringWriter(); + if (checker.CheckAll(textWriter)) + { + Console.WriteLine($"Processing {Path.GetFileName(targets)} passed"); + return true; + } + else + { + Console.WriteLine($"Processing {Path.GetFileName(targets)} FAILED"); + Console.WriteLine(textWriter.ToString()); + return false; + } + } + private static void Usage() { Console.WriteLine($"BuildBoss "); diff --git a/src/Tools/BuildBoss/ProjectUtil.cs b/src/Tools/BuildBoss/ProjectUtil.cs index 7428ffb583c..9f374d71917 100644 --- a/src/Tools/BuildBoss/ProjectUtil.cs +++ b/src/Tools/BuildBoss/ProjectUtil.cs @@ -16,6 +16,11 @@ internal class ProjectUtil private readonly XDocument _document; private readonly XmlNamespaceManager _manager; + internal ProjectUtil(string filePath) :this(new ProjectKey(filePath), XDocument.Load(filePath)) + { + + } + internal ProjectUtil(ProjectKey key, XDocument document) { _key = key; @@ -89,11 +94,23 @@ internal IEnumerable GetAllPropertyGroupElements() } } + internal IEnumerable GetTargets() + { + return _document.XPathSelectElements("//mb:Target", _manager); + } + internal IEnumerable GetImports() { return _document.XPathSelectElements("//mb:Import", _manager); } + internal IEnumerable GetImportProjects() + { + return GetImports() + .Select(x => x.Attribute("Project")?.Value) + .Where(x => !string.IsNullOrEmpty(x)); + } + internal IEnumerable GetItemGroup() { return _document.XPathSelectElements("//mb:ItemGroup", _manager); diff --git a/src/Tools/BuildBoss/README.md b/src/Tools/BuildBoss/README.md index 4d5d824b23a..f337131ae4a 100644 --- a/src/Tools/BuildBoss/README.md +++ b/src/Tools/BuildBoss/README.md @@ -31,3 +31,4 @@ Our build process depends on being able to correctly classify our projects. Thi This could be done using MSBuild targets but the logic is hard to follow and complicates the build. It's easier to verify here. + diff --git a/src/Tools/BuildBoss/SharedUtil.cs b/src/Tools/BuildBoss/SharedUtil.cs index 3471e80cdcd..4fda101b249 100644 --- a/src/Tools/BuildBoss/SharedUtil.cs +++ b/src/Tools/BuildBoss/SharedUtil.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Text; using System.Threading.Tasks; @@ -13,5 +14,10 @@ internal static class SharedUtil internal static Uri MSBuildNamespaceUri { get; } = new Uri(MSBuildNamespaceUriRaw); internal static XNamespace MSBuildNamespace { get; } = XNamespace.Get(MSBuildNamespaceUriRaw); internal static Encoding Encoding { get; } = Encoding.UTF8; + + internal static bool IsSolutionFile(string path) => Path.GetExtension(path) == ".sln"; + internal static bool IsPropsFile(string path) => Path.GetExtension(path) == ".props"; + internal static bool IsTargetsFile(string path) => Path.GetExtension(path) == ".targets"; + internal static bool IsXslt(string path) => Path.GetExtension(path) == ".xslt"; } } diff --git a/src/Tools/BuildBoss/TargetsCheckerUtil.cs b/src/Tools/BuildBoss/TargetsCheckerUtil.cs new file mode 100644 index 00000000000..7024fa96e1a --- /dev/null +++ b/src/Tools/BuildBoss/TargetsCheckerUtil.cs @@ -0,0 +1,119 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace BuildBoss +{ + internal sealed class TargetsCheckerUtil + { + private readonly string _targetDir; + + internal TargetsCheckerUtil(string targetDir) + { + _targetDir = targetDir; + } + + internal bool CheckAll(TextWriter textWriter) + { + var allGood = true; + + foreach (var filePath in Directory.GetFiles(_targetDir)) + { + var fileName = Path.GetFileName(filePath); + if (fileName == "README.md") + { + continue; + } + + textWriter.WriteLine($"Checking {fileName}"); + if (SharedUtil.IsPropsFile(filePath)) + { + allGood &= CheckProps(new ProjectUtil(filePath), textWriter); + } + else if (SharedUtil.IsTargetsFile(filePath)) + { + allGood &= CheckTargets(new ProjectUtil(filePath), textWriter); + } + else if (SharedUtil.IsXslt(filePath)) + { + // Nothing to verify + } + else + { + textWriter.WriteLine("Unrecognized file type"); + allGood = false; + } + } + + return allGood; + } + + private bool CheckProps(ProjectUtil util, TextWriter textWriter) + { + var allGood = true; + foreach (var project in GetImportProjects(util)) + { + if (!SharedUtil.IsPropsFile(project)) + { + textWriter.WriteLine($"Props files should only Import other props files"); + allGood = false; + } + } + + if (util.GetTargets().Any()) + { + textWriter.WriteLine($"Props files should not contain elements"); + allGood = false; + } + + return allGood; + } + + private bool CheckTargets(ProjectUtil util, TextWriter textWriter) + { + var allGood = true; + foreach (var project in GetImportProjects(util)) + { + if (SharedUtil.IsPropsFile(project)) + { + textWriter.WriteLine($"Targets files should not Import props files"); + allGood = false; + } + } + + return allGood; + } + + private static IEnumerable GetImportProjects(ProjectUtil util) + { + foreach (var project in util.GetImportProjects()) + { + if (IsReplacedValue(project)) + { + continue; + } + + yield return project; + } + } + + /// + /// Is this a whole replaced MSBuild value like $(Trick) + /// + private static bool IsReplacedValue(string value) + { + if (value.Length <= 3) + { + return false; + } + + return + value[0] == '$' && + value[1] == '(' && + value[value.Length - 1] == ')'; + } + } +} -- GitLab