From 9b46eebfb83d3ca0023b3cc45de46d5ad2aea622 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 16 May 2018 22:59:20 -0500 Subject: [PATCH] [XamlG] builds incrementally Context: https://github.com/xamarin/Xamarin.Forms/pull/2230 The main performance problem with the collection of MSBuild targets in `Xamarin.Forms.targets` is they don't build incrementally. I addressed this with `XamlC` using a "stamp" file; however, it is not quite so easy to setup the same thing with `XamlG`. They way "incremental" builds are setup in MSBuild, is by specifying the `Inputs` and `Outputs` of a ``. MSBuild will partially build a target when some outputs are not up to date, and skip it entirely if they are all up to date. The best docs I can find on MSBuild incremental builds: https://msdn.microsoft.com/en-us/library/ms171483.aspx Unfortunately a few things had to happen to make this work for `XamlG`: - Define a new target `_FindXamlGFiles` that is invoked before `XamlG` - `_FindXamlGFiles` defines the `_XamlGInputs` and `_XamlGOutputs` ``'s - `_FindXamlGFiles` must also define `` and ``, in case the `XamlG` target is skipped - `XamlGTask` now needs to get passed in a list of `OutputFiles`, since we have computed these paths ahead of time - `XamlGTask` should validate the lengths of `XamlFiles` and `OutputFiles` match, used error message from MSBuild proper: https://github.com/Microsoft/msbuild/blob/a691a44f0e515e9a03ede8df0bff22185681c8b9/src/Tasks/Copy.cs#L505 `XamlG` now builds incrementally! Other changes: - I reordered my past `` element in XamlC to match others in this file - `FilesWrite` is actually supposed to be `FileWrites`, see canonical source of how `Clean` works and what `FileWrites` is here: https://github.com/Microsoft/msbuild/issues/2408#issuecomment-321082997 Unfortunately, there is more to do to get this PR "feature complete": - TESTS? This refactoring is complex and has a bit of nuance to it. I can't imagine we can get this improvement right (at all), without some new testing infrastructure in place that invokes MSBuild. - There were some checks for `%(TargetPath)` being blank in the C# code of `XamlGTask`. I presume this is from the designer and/or design-time builds. We probably need to figure out the proper way to do this instead of using `%(TargetPath)` at all. After testing the current implementation I had all kinds of crazy temporary files in my `$(IntermediateOutputPath)`. This probably isn't the right way to handle this issue. - CssG needs the exact same setup, as it was patterned after `XamlG` --- .nuspec/Xamarin.Forms.targets | 29 ++++++++++++---------- Xamarin.Forms.Build.Tasks/XamlGTask.cs | 33 +++++++++++--------------- Xamarin.Forms.Xaml.Xamlg/Xamlg.cs | 2 +- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/.nuspec/Xamarin.Forms.targets b/.nuspec/Xamarin.Forms.targets index 841b415b4f9..01799cf084e 100644 --- a/.nuspec/Xamarin.Forms.targets +++ b/.nuspec/Xamarin.Forms.targets @@ -64,18 +64,21 @@ - - - <_XamlGAlreadyExecuted>true - + + + <_XamlGInputs Include="@(EmbeddedResource)" Condition="'%(Extension)' == '.xaml' AND '$(DefaultLanguageSourceExtension)' == '.cs'" /> + <_XamlGOutputs Include="@(_XamlGInputs->'$(IntermediateOutputPath)%(TargetPath).g.cs')" /> + + + + + + - - - + XamlFiles="@(_XamlGInputs)" + OutputFiles="@(_XamlGOutputs)" + Language="$(Language)" + AssemblyName="$(AssemblyName)" /> @@ -95,7 +98,7 @@ DebugType = "$(DebugType)" KeepXamlResources = "$(XFKeepXamlResources)" /> - + @@ -115,7 +118,7 @@ Language = "$(Language)" AssemblyName = "$(AssemblyName)" OutputPath = "$(IntermediateOutputPath)"> - + diff --git a/Xamarin.Forms.Build.Tasks/XamlGTask.cs b/Xamarin.Forms.Build.Tasks/XamlGTask.cs index 97d5959921d..dd41e4f941e 100644 --- a/Xamarin.Forms.Build.Tasks/XamlGTask.cs +++ b/Xamarin.Forms.Build.Tasks/XamlGTask.cs @@ -10,17 +10,14 @@ namespace Xamarin.Forms.Build.Tasks { public class XamlGTask : Task { - List _generatedCodeFiles = new List(); - [Required] public ITaskItem[] XamlFiles { get; set; } - [Output] - public ITaskItem[] GeneratedCodeFiles => _generatedCodeFiles.ToArray(); + [Required] + public ITaskItem [] OutputFiles { get; set; } public string Language { get; set; } public string AssemblyName { get; set; } - public string OutputPath { get; set; } public override bool Execute() { @@ -32,22 +29,20 @@ public override bool Execute() return true; } - foreach (var xamlFile in XamlFiles) { - //when invoked from `UpdateDesigntimeXaml` target, the `TargetPath` isn't set, use a random one instead - var targetPath = xamlFile.GetMetadata("TargetPath"); - if (string.IsNullOrWhiteSpace(targetPath)) - targetPath = $".{Path.GetRandomFileName()}"; + if (XamlFiles.Length != OutputFiles?.Length) { + Log.LogErrorWithCodeFromResources("General.TwoVectorsMustHaveSameLength", XamlFiles.Length, OutputFiles?.Length, "XamlFiles", "OutputFiles"); + return false; + } - var outputFile = Path.Combine(OutputPath, $"{targetPath}.g.cs"); - if (Path.DirectorySeparatorChar == '/' && outputFile.Contains(@"\")) - outputFile = outputFile.Replace('\\','/'); - else if (Path.DirectorySeparatorChar == '\\' && outputFile.Contains(@"/")) - outputFile = outputFile.Replace('/', '\\'); - - var generator = new XamlGenerator(xamlFile, Language, AssemblyName, outputFile, Log); + for (int i = 0; i < XamlFiles.Length; i++) { + var xamlFile = XamlFiles [i]; + var outputFile = OutputFiles [i]; + var generator = new XamlGenerator(xamlFile, Language, AssemblyName, outputFile.ItemSpec, Log); try { - if (generator.Execute()) - _generatedCodeFiles.Add(new TaskItem(Microsoft.Build.Evaluation.ProjectCollection.Escape(outputFile))); + if (!generator.Execute()) { + //If Execute() fails, the file still needs to exist because it is added to the ItemGroup + File.WriteAllText (outputFile.ItemSpec, string.Empty); + } } catch (XmlException xe) { Log.LogError(null, null, null, xamlFile.ItemSpec, xe.LineNumber, xe.LinePosition, 0, 0, xe.Message, xe.HelpLink, xe.Source); diff --git a/Xamarin.Forms.Xaml.Xamlg/Xamlg.cs b/Xamarin.Forms.Xaml.Xamlg/Xamlg.cs index 312f525707a..b833ad734bb 100644 --- a/Xamarin.Forms.Xaml.Xamlg/Xamlg.cs +++ b/Xamarin.Forms.Xaml.Xamlg/Xamlg.cs @@ -80,7 +80,7 @@ public static void Main(string[] args) AssemblyName = "test", Language = "C#", XamlFiles = new[] { item }, - OutputPath = Path.GetDirectoryName(f), + OutputFiles = new[] { new TaskItem(Path.Combine(Path.GetDirectoryName(f), item.ItemSpec)) } };