From 3dd029a8b54a3dac3542f11ba479ad40a29246f3 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Fri, 22 Sep 2017 12:19:39 -0700 Subject: [PATCH] Simplify the MSBuild targets in dotnet-watch Use CustomAfterMicrosoftCommonTargets instead of MSBuildProjectExtensionsPath. - No more need to write to obj/$(Project).g.dotnetwatch.targets - Works on project that have changed default file locations via BaseIntermediateOutputPath Simplify DotNetWatch targets - Condense to one targets file - Simplify dependency chain of targets - Build project references in a parallel --- Directory.Build.props | 2 + .../CommandLineOptions.cs | 8 -- .../Internal/MsBuildFileSetFactory.cs | 80 ++++++++----------- .../Microsoft.DotNet.Watcher.Tools.csproj | 5 +- src/Microsoft.DotNet.Watcher.Tools/Program.cs | 27 +------ .../dotnetwatch.targets | 18 ----- .../toolassets/DotNetWatch.targets | 68 ++++++++++++++++ .../toolassets/DotNetWatchCommon.targets | 28 ------- .../toolassets/DotNetWatchInner.targets | 70 ---------------- .../toolassets/DotNetWatchOuter.targets | 69 ---------------- .../DotNetWatcherTests.cs | 5 -- ...otNet.Watcher.Tools.FunctionalTests.csproj | 4 +- .../Scenario/WatchableApp.cs | 7 +- .../KitchenSink/KitchenSink.csproj | 1 + .../AssertEx.cs | 6 +- ...icrosoft.DotNet.Watcher.Tools.Tests.csproj | 4 +- .../MsBuildFileSetFactoryTest.cs | 15 +--- 17 files changed, 119 insertions(+), 298 deletions(-) delete mode 100644 src/Microsoft.DotNet.Watcher.Tools/dotnetwatch.targets create mode 100644 src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatch.targets delete mode 100644 src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchCommon.targets delete mode 100644 src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchInner.targets delete mode 100644 src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchOuter.targets diff --git a/Directory.Build.props b/Directory.Build.props index bff80fce71..90338e5345 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -13,6 +13,8 @@ false + $(MSBuildThisFileDirectory)obj\$(MSBuildProjectName)\ + $(MSBuildThisFileDirectory)bin\$(MSBuildProjectName)\ diff --git a/src/Microsoft.DotNet.Watcher.Tools/CommandLineOptions.cs b/src/Microsoft.DotNet.Watcher.Tools/CommandLineOptions.cs index 22705c9265..e6e23890d5 100644 --- a/src/Microsoft.DotNet.Watcher.Tools/CommandLineOptions.cs +++ b/src/Microsoft.DotNet.Watcher.Tools/CommandLineOptions.cs @@ -13,7 +13,6 @@ namespace Microsoft.DotNet.Watcher internal class CommandLineOptions { public string Project { get; private set; } - public string MSBuildProjectExtensionsPath { get; private set; } public bool IsHelp { get; private set; } public bool IsQuiet { get; private set; } public bool IsVerbose { get; private set; } @@ -78,12 +77,6 @@ Examples: var optProjects = app.Option("-p|--project ", "The project to watch", CommandOptionType.SingleValue); - var optMSBuildProjectExtensionsPath = app.Option("--msbuildprojectextensionspath ", - "The MSBuild project extensions path. Defaults to \"obj\".", - CommandOptionType.SingleValue); - // Hide from help text because this is an internal option that will hopefully go away when/if #244 is resolved. - optMSBuildProjectExtensionsPath.ShowInHelpText = false; - var optQuiet = app.Option("-q|--quiet", "Suppresses all output except warnings and errors", CommandOptionType.NoValue); var optVerbose = app.VerboseOption(); @@ -113,7 +106,6 @@ Examples: return new CommandLineOptions { Project = optProjects.Value(), - MSBuildProjectExtensionsPath = optMSBuildProjectExtensionsPath.Value(), IsQuiet = optQuiet.HasValue(), IsVerbose = optVerbose.HasValue(), RemainingArguments = app.RemainingArguments, diff --git a/src/Microsoft.DotNet.Watcher.Tools/Internal/MsBuildFileSetFactory.cs b/src/Microsoft.DotNet.Watcher.Tools/Internal/MsBuildFileSetFactory.cs index 0190b089ca..837881c907 100644 --- a/src/Microsoft.DotNet.Watcher.Tools/Internal/MsBuildFileSetFactory.cs +++ b/src/Microsoft.DotNet.Watcher.Tools/Internal/MsBuildFileSetFactory.cs @@ -17,21 +17,19 @@ namespace Microsoft.DotNet.Watcher.Internal public class MsBuildFileSetFactory : IFileSetFactory { private const string TargetName = "GenerateWatchList"; - private const string ProjectExtensionFileExtension = ".dotnetwatch.g.targets"; - private const string WatchTargetsFileName = "DotNetWatchCommon.targets"; + private const string WatchTargetsFileName = "DotNetWatch.targets"; private readonly IReporter _reporter; private readonly string _projectFile; - private readonly string _projectExtensionsPath; - private readonly string _watchTargetsDir; private readonly OutputSink _outputSink; private readonly ProcessRunner _processRunner; private readonly bool _waitOnError; + private readonly IReadOnlyList _buildFlags; public MsBuildFileSetFactory(IReporter reporter, string projectFile, - string msBuildProjectExtensionsPath, - bool waitOnError) - : this(reporter, projectFile, msBuildProjectExtensionsPath, new OutputSink()) + bool waitOnError, + bool trace) + : this(reporter, projectFile, new OutputSink(), trace) { _waitOnError = waitOnError; } @@ -39,8 +37,8 @@ namespace Microsoft.DotNet.Watcher.Internal // output sink is for testing internal MsBuildFileSetFactory(IReporter reporter, string projectFile, - string msBuildProjectExtensionsPath, - OutputSink outputSink) + OutputSink outputSink, + bool trace) { Ensure.NotNull(reporter, nameof(reporter)); Ensure.NotNullOrEmpty(projectFile, nameof(projectFile)); @@ -48,29 +46,13 @@ namespace Microsoft.DotNet.Watcher.Internal _reporter = reporter; _projectFile = projectFile; - _watchTargetsDir = FindWatchTargetsDir(); _outputSink = outputSink; _processRunner = new ProcessRunner(reporter); - - // default value for MSBuildProjectExtensionsPath is $(BaseIntermediateOutputPath), which defaults to 'obj/'. - _projectExtensionsPath = string.IsNullOrEmpty(msBuildProjectExtensionsPath) - ? Path.Combine(Path.GetDirectoryName(_projectFile), "obj") - : msBuildProjectExtensionsPath; + _buildFlags = InitializeArgs(FindTargetsFile(), trace); } - internal List BuildFlags { get; } = new List - { - "/nologo", - "/v:n", - "/t:" + TargetName, - "/p:DotNetWatchBuild=true", // extensibility point for users - "/p:DesignTimeBuild=true", // don't do expensive things - }; - public async Task CreateAsync(CancellationToken cancellationToken) { - EnsureInitialized(); - var watchList = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); try { @@ -91,9 +73,8 @@ namespace Microsoft.DotNet.Watcher.Internal { "msbuild", _projectFile, - $"/p:_DotNetWatchTargetsLocation={_watchTargetsDir}", // add our dotnet-watch targets $"/p:_DotNetWatchListFile={watchList}" - }.Concat(BuildFlags), + }.Concat(_buildFlags), OutputCapture = capture }; @@ -162,29 +143,31 @@ namespace Microsoft.DotNet.Watcher.Internal } } - // Ensures file exists in $(MSBuildProjectExtensionsPath)/$(MSBuildProjectFile).dotnetwatch.targets - private void EnsureInitialized() + private IReadOnlyList InitializeArgs(string watchTargetsFile, bool trace) { - // see https://github.com/Microsoft/msbuild/blob/bf9b21cc7869b96ea2289ff31f6aaa5e1d525a26/src/XMakeTasks/Microsoft.Common.targets#L127 - var projectExtensionFile = Path.Combine(_projectExtensionsPath, - Path.GetFileName(_projectFile) + ProjectExtensionFileExtension); - - if (!File.Exists(projectExtensionFile)) + var args = new List { - // ensure obj folder is available - Directory.CreateDirectory(Path.GetDirectoryName(projectExtensionFile)); + "/nologo", + "/v:n", + "/t:" + TargetName, + "/p:DotNetWatchBuild=true", // extensibility point for users + "/p:DesignTimeBuild=true", // don't do expensive things + "/p:CustomAfterMicrosoftCommonTargets=" + watchTargetsFile, + "/p:CustomAfterMicrosoftCommonCrossTargetingTargets=" + watchTargetsFile, + }; - using (var fileStream = new FileStream(projectExtensionFile, FileMode.Create)) - using (var assemblyStream = GetType().GetTypeInfo().Assembly.GetManifestResourceStream("dotnetwatch.targets")) - { - assemblyStream.CopyTo(fileStream); - } + if (trace) + { + // enables capturing markers to know which projects have been visited + args.Add("/p:_DotNetWatchTraceOutput=true"); } + + return args; } - private string FindWatchTargetsDir() + private string FindTargetsFile() { - var assemblyDir = Path.GetDirectoryName(GetType().GetTypeInfo().Assembly.Location); + var assemblyDir = Path.GetDirectoryName(typeof(MsBuildFileSetFactory).Assembly.Location); var searchPaths = new[] { AppContext.BaseDirectory, @@ -194,8 +177,13 @@ namespace Microsoft.DotNet.Watcher.Internal Path.Combine(AppContext.BaseDirectory, "../../toolassets"), // relative to packaged deps.json }; - var targetPath = searchPaths.Select(p => Path.Combine(p, WatchTargetsFileName)).First(File.Exists); - return Path.GetDirectoryName(targetPath); + var targetPath = searchPaths.Select(p => Path.Combine(p, WatchTargetsFileName)).FirstOrDefault(File.Exists); + if (targetPath == null) + { + _reporter.Error("Fatal error: could not find DotNetWatch.targets"); + return null; + } + return targetPath; } } } diff --git a/src/Microsoft.DotNet.Watcher.Tools/Microsoft.DotNet.Watcher.Tools.csproj b/src/Microsoft.DotNet.Watcher.Tools/Microsoft.DotNet.Watcher.Tools.csproj index 6b11bd5bcf..443587cf07 100644 --- a/src/Microsoft.DotNet.Watcher.Tools/Microsoft.DotNet.Watcher.Tools.csproj +++ b/src/Microsoft.DotNet.Watcher.Tools/Microsoft.DotNet.Watcher.Tools.csproj @@ -12,9 +12,8 @@ - - - + + diff --git a/src/Microsoft.DotNet.Watcher.Tools/Program.cs b/src/Microsoft.DotNet.Watcher.Tools/Program.cs index 019c95a329..7e8200b102 100644 --- a/src/Microsoft.DotNet.Watcher.Tools/Program.cs +++ b/src/Microsoft.DotNet.Watcher.Tools/Program.cs @@ -87,14 +87,12 @@ namespace Microsoft.DotNet.Watcher { return await ListFilesAsync(_reporter, options.Project, - options.MSBuildProjectExtensionsPath, _cts.Token); } else { return await MainInternalAsync(_reporter, options.Project, - options.MSBuildProjectExtensionsPath, options.RemainingArguments, _cts.Token); } @@ -129,7 +127,6 @@ namespace Microsoft.DotNet.Watcher private async Task MainInternalAsync( IReporter reporter, string project, - string msbuildProjectExtensionsPath, ICollection args, CancellationToken cancellationToken) { @@ -147,8 +144,8 @@ namespace Microsoft.DotNet.Watcher var fileSetFactory = new MsBuildFileSetFactory(reporter, projectFile, - NormalizePath(msbuildProjectExtensionsPath), - waitOnError: true); + waitOnError: true, + trace: false); var processInfo = new ProcessSpec { Executable = DotNetMuxer.MuxerPathOrDefault(), @@ -174,7 +171,6 @@ namespace Microsoft.DotNet.Watcher private async Task ListFilesAsync( IReporter reporter, string project, - string msbuildProjectExtensionsPath, CancellationToken cancellationToken) { // TODO multiple projects should be easy enough to add here @@ -191,8 +187,8 @@ namespace Microsoft.DotNet.Watcher var fileSetFactory = new MsBuildFileSetFactory(reporter, projectFile, - NormalizePath(msbuildProjectExtensionsPath), - waitOnError: false); + waitOnError: false, + trace: false); var files = await fileSetFactory.CreateAsync(cancellationToken); if (files == null) @@ -211,21 +207,6 @@ namespace Microsoft.DotNet.Watcher private static IReporter CreateReporter(bool verbose, bool quiet, IConsole console) => new PrefixConsoleReporter(console, verbose || CliContext.IsGlobalVerbose(), quiet); - private string NormalizePath(string path) - { - if (path == null || Path.IsPathRooted(path)) - { - return path; - } - - if (string.IsNullOrWhiteSpace(path)) - { - return _workingDir; - } - - return Path.Combine(_workingDir, path); - } - public void Dispose() { _console.CancelKeyPress -= OnCancelKeyPress; diff --git a/src/Microsoft.DotNet.Watcher.Tools/dotnetwatch.targets b/src/Microsoft.DotNet.Watcher.Tools/dotnetwatch.targets deleted file mode 100644 index d3e73557be..0000000000 --- a/src/Microsoft.DotNet.Watcher.Tools/dotnetwatch.targets +++ /dev/null @@ -1,18 +0,0 @@ - - - - <_DotNetWatchTargetsFile Condition="'$(_DotNetWatchTargetsFile)' == ''">$(MSBuildThisFileFullPath) - - - - - - - - - - - \ No newline at end of file diff --git a/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatch.targets b/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatch.targets new file mode 100644 index 0000000000..5ce4f08672 --- /dev/null +++ b/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatch.targets @@ -0,0 +1,68 @@ + + + + + + + + + <_CollectWatchItemsDependsOn Condition=" '$(TargetFrameworks)' != '' AND '$(TargetFramework)' == '' "> + _CollectWatchItemsPerFramework; + + <_CollectWatchItemsDependsOn Condition=" '$(TargetFramework)' != '' "> + _CoreCollectWatchItems; + + + + + + + + <_TargetFramework Include="$(TargetFrameworks)" /> + + + + + + + + + + + + + + + + + + <_WatchProjects Include="%(ProjectReference.Identity)" Condition="'%(ProjectReference.Watch)' != 'false'" /> + + + + + + + + diff --git a/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchCommon.targets b/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchCommon.targets deleted file mode 100644 index c3477393fc..0000000000 --- a/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchCommon.targets +++ /dev/null @@ -1,28 +0,0 @@ - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchInner.targets b/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchInner.targets deleted file mode 100644 index 9bdd83dd6d..0000000000 --- a/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchInner.targets +++ /dev/null @@ -1,70 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - <_DotNetWatchProjects Include="@(ProjectReference->'%(FullPath)')" Condition="'%(ProjectReference.Watch)' != 'false'" /> - <_DotNetWatchImportsTargets Include="@(_DotNetWatchProjects->'%(RelativeDir)obj\%(FileName)%(Extension).dotnetwatch.targets')"> - $(_DotNetWatchTargetsFile) - - - - - - - - - - - <_DotNetWatchProjects Include="$(MSBuildProjectFullPath)"/> - - - \ No newline at end of file diff --git a/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchOuter.targets b/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchOuter.targets deleted file mode 100644 index f2dfb9e104..0000000000 --- a/src/Microsoft.DotNet.Watcher.Tools/toolassets/DotNetWatchOuter.targets +++ /dev/null @@ -1,69 +0,0 @@ - - - - - - - - - <_TargetFramework Include="$(TargetFrameworks)" /> - - - - - - - - - - <_TargetFramework Include="$(TargetFrameworks)" /> - - - - - - diff --git a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/DotNetWatcherTests.cs b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/DotNetWatcherTests.cs index ed89240fb3..368c55a202 100644 --- a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/DotNetWatcherTests.cs +++ b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/DotNetWatcherTests.cs @@ -41,11 +41,6 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests : base("KitchenSink", logger) { } - - protected override IEnumerable GetDefaultArgs() - { - return new[] { "--msbuildprojectextensionspath", ".net/obj", "run", "--" }; - } } } } diff --git a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Microsoft.DotNet.Watcher.Tools.FunctionalTests.csproj b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Microsoft.DotNet.Watcher.Tools.FunctionalTests.csproj index 07cb79b4c3..d0bac72314 100644 --- a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Microsoft.DotNet.Watcher.Tools.FunctionalTests.csproj +++ b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Microsoft.DotNet.Watcher.Tools.FunctionalTests.csproj @@ -8,9 +8,7 @@ - - - + diff --git a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Scenario/WatchableApp.cs b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Scenario/WatchableApp.cs index 070cede829..d204d5159a 100644 --- a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Scenario/WatchableApp.cs +++ b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Scenario/WatchableApp.cs @@ -99,7 +99,7 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests await PrepareAsync(); } - var args = GetDefaultArgs().Concat(arguments); + var args = new[] { "run", "--" }.Concat(arguments); Start(args, name); // Make this timeout long because it depends much on the MSBuild compilation speed. @@ -107,11 +107,6 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests await Process.GetOutputLineAsync(StartedMessage).TimeoutAfter(TimeSpan.FromMinutes(2)); } - protected virtual IEnumerable GetDefaultArgs() - { - return new[] { "run", "--" }; - } - public virtual void Dispose() { _logger?.WriteLine("Disposing WatchableApp"); diff --git a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/TestProjects/KitchenSink/KitchenSink.csproj b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/TestProjects/KitchenSink/KitchenSink.csproj index 64ce763e70..aeb186ac39 100755 --- a/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/TestProjects/KitchenSink/KitchenSink.csproj +++ b/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/TestProjects/KitchenSink/KitchenSink.csproj @@ -2,6 +2,7 @@ .net/obj + .net/bin diff --git a/test/Microsoft.DotNet.Watcher.Tools.Tests/AssertEx.cs b/test/Microsoft.DotNet.Watcher.Tools.Tests/AssertEx.cs index 0ffba97152..4d897058fd 100644 --- a/test/Microsoft.DotNet.Watcher.Tools.Tests/AssertEx.cs +++ b/test/Microsoft.DotNet.Watcher.Tools.Tests/AssertEx.cs @@ -20,14 +20,14 @@ namespace Microsoft.DotNet.Watcher.Tools.Tests public static void EqualFileList(IEnumerable expectedFiles, IEnumerable actualFiles) { - Func normalize = p => p.Replace('\\', '/'); + string normalize(string p) => p.Replace('\\', '/'); var expected = new HashSet(expectedFiles.Select(normalize)); var actual = new HashSet(actualFiles.Where(p => !string.IsNullOrEmpty(p)).Select(normalize)); if (!expected.SetEquals(actual)) { throw new AssertActualExpectedException( - expected: string.Join("\n", expected), - actual: string.Join("\n", actual), + expected: "\n" + string.Join("\n", expected), + actual: "\n" + string.Join("\n", actual), userMessage: "File sets should be equal"); } } diff --git a/test/Microsoft.DotNet.Watcher.Tools.Tests/Microsoft.DotNet.Watcher.Tools.Tests.csproj b/test/Microsoft.DotNet.Watcher.Tools.Tests/Microsoft.DotNet.Watcher.Tools.Tests.csproj index 1bc4dc44ae..c005f40d88 100644 --- a/test/Microsoft.DotNet.Watcher.Tools.Tests/Microsoft.DotNet.Watcher.Tools.Tests.csproj +++ b/test/Microsoft.DotNet.Watcher.Tools.Tests/Microsoft.DotNet.Watcher.Tools.Tests.csproj @@ -6,9 +6,7 @@ - - - + diff --git a/test/Microsoft.DotNet.Watcher.Tools.Tests/MsBuildFileSetFactoryTest.cs b/test/Microsoft.DotNet.Watcher.Tools.Tests/MsBuildFileSetFactoryTest.cs index 064dc79902..dde03d31ab 100644 --- a/test/Microsoft.DotNet.Watcher.Tools.Tests/MsBuildFileSetFactoryTest.cs +++ b/test/Microsoft.DotNet.Watcher.Tools.Tests/MsBuildFileSetFactoryTest.cs @@ -238,11 +238,7 @@ namespace Microsoft.DotNet.Watcher.Tools.Tests graph.Find("A").WithProjectReference(graph.Find("W"), watch: false); var output = new OutputSink(); - var filesetFactory = new MsBuildFileSetFactory(_reporter, graph.GetOrCreate("A").Path, null, output) - { - // enables capturing markers to know which projects have been visited - BuildFlags = { "/p:_DotNetWatchTraceOutput=true" } - }; + var filesetFactory = new MsBuildFileSetFactory(_reporter, graph.GetOrCreate("A").Path, output, trace: true); var fileset = await GetFileSet(filesetFactory); @@ -270,17 +266,10 @@ namespace Microsoft.DotNet.Watcher.Tools.Tests Assert.Single(output.Current.Lines, line => line.Contains($"Collecting watch items from '{projectName}'")) ); - - // ensure each project is only visited once to collect project references - Assert.All(includedProjects, - projectName => - Assert.Single(output.Current.Lines, - line => line.Contains($"Collecting referenced projects from '{projectName}'")) - ); } private Task GetFileSet(TemporaryCSharpProject target) - => GetFileSet(new MsBuildFileSetFactory(_reporter, target.Path, null, waitOnError: false)); + => GetFileSet(new MsBuildFileSetFactory(_reporter, target.Path, waitOnError: false, trace: false)); private async Task GetFileSet(MsBuildFileSetFactory filesetFactory) {