Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Commit b84b35b

Browse files
Jason Smithrmarinho
Jason Smith
authored and
rmarinho
committed
[XamlG] builds incrementally, add MSBuild integration tests (#2825)
* [XamlG] builds incrementally, add MSBuild integration tests Context: #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 `<Target />`. 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` `<ItemGroup />`'s - `_FindXamlGFiles` must also define `<Compile />` and `<FileWrites />`, 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! To give some context on how much improvement we can see with build times, consider the following command: msbuild Xamarin.Forms.ControlGallery.Android/Xamarin.Forms.ControlGallery.Android.csproj If you run it once, it will take a while--this change will not improve the first build. On the second build with the exact same command, it *should* be much faster. Before this commit, the second build on my machine takes: 40.563s After the change: 23.692s `XamlG` has cascading impact on build times when it isn't built incrementally: - The C# assembly always changes - Hence, `XamlC` will always run - Hence, `GenerateJavaStubs` will always run - Hence, `javac.exe` and `dx.jar` will always run I am making other improvements like this in Xamarin.Android itself, that will further improve these times, such as: dotnet/android#1693 ~~ New MSBuild Integration Tests ~~ Added some basic MSBuild testing infrastructure: - Tests write project files to `bin/Debug/temp/TestName` - Each test has an `sdkStyle` flag for testing the new project system versus the old one - `[TearDown]` deletes the entire directory, with a retry for `IOException` on Windows - Used the `Microsoft.Build.Locator` NuGet package for locating `MSBuild.exe` on Windows - These tests take 2-5 seconds each So for example, the simplest test, `BuildAProject` writes to `Xamarin.Forms.Xaml.UnitTests\bin\Debug\temp\BuildAProject(False)\test.csproj`: <?xml version="1.0" encoding="utf-8"?> <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> <PropertyGroup> <Configuration>Debug</Configuration> <Platform>AnyCPU</Platform> <OutputType>Library</OutputType> <OutputPath>bin\Debug</OutputPath> <TargetFrameworkVersion>v4.7</TargetFrameworkVersion> </PropertyGroup> <ItemGroup> <Reference Include="mscorlib" /> <Reference Include="System" /> <Reference Include="Xamarin.Forms.Core.dll"> <HintPath>..\..\Xamarin.Forms.Core.dll</HintPath> </Reference> <Reference Include="Xamarin.Forms.Xaml.dll"> <HintPath>..\..\Xamarin.Forms.Xaml.dll</HintPath> </Reference> </ItemGroup> <ItemGroup> <Compile Include="AssemblyInfo.cs" /> </ItemGroup> <Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" /> <Import Project="..\..\..\..\..\.nuspec\Xamarin.Forms.targets" /> <ItemGroup> <EmbeddedResource Include="MainPage.xaml" /> </ItemGroup> </Project> Invokes `msbuild`, and checks the intermediate output for files being generated. Tested scenarios: - Build a simple project - Build, then build again, and make sure targets were skipped - Build, then clean, make sure files are gone - Build, with linked files - Design-time build - Call `UpdateDesignTimeXaml` directly - Build, add a new file, build again - Build, update timestamp on a file, build again - XAML file with random XML content - XAML file with invalid XML content - A general `EmbeddedResource` that shouldn't go through XamlG Adding these tests found a bug! `IncrementalClean` was deleting `XamlC.stamp`. I fixed this by using `<ItemGroup />`, which will be propery evaluated even if the target is skipped. ~~ Other Changes ~~ - `FilesWrite` is actually supposed to be `FileWrites`, see canonical source of how `Clean` works and what `FileWrites` is here: dotnet/msbuild#2408 (comment) - Moved `DummyBuildEngine` into `MSBuild` directory--makes sense? maybe don't need to? - Added a `XamlGDifferentInputOutputLengths` test to check the error message - Expanded `DummyBuildEngine` so you can assert against log messages - Changed a setting in `.Xamarin.Forms.Android.sln` so the unit test project is built - My VS IDE monkeyed with a few files, and I kept any *good* (or relevant) changes: `Xamarin.Forms.UnitTests.csproj`, `Xamarin.Forms.Xaml.UnitTests\app.config`, etc. There were some checks for `%(TargetPath)` being blank in the C# code of `XamlGTask`. In that case it was using `Path.GetRandomFileName`, but we can't do this if we are setting up inputs and outputs for `XamlG`. I presume this is from the designer and/or design-time builds before `DependsOnTargets="PrepareResourceNames"` was added. I tested design-time builds in VS on Windows, and `$(TargetPath)` was set. To be sure we don't break anything here, I exclude inputs to `XamlG` if `%(TargetPath)` is somehow blank. See relevant MSBuild code for `%(TargetPath)` here: https://github.com/Microsoft/msbuild/blob/05151780901c38b4613b2f236ab8b091349dbe94/src/Tasks/Microsoft.Common.CurrentVersion.targets#L2822 ~~ Future changes ~~ CssG needs the exact same setup, as it was patterned after `XamlG`. This should probably be done in a future PR. * [msbuild] improved lookup of Xamarin.Forms.targets in integration tests Context: https://devdiv.visualstudio.com/DevDiv/_build?buildId=1717939 Context: https://devdiv.visualstudio.com/DevDiv/_build?buildId=1718306 It looks like the VSTS builds for release branches are running tests in a staging directory. This means we can't reliably import `Xamarin.Forms.targets` as what was working locally in the Xamarin.Forms source tree. So to fix this: - Look for `.nuspec/Xamarin.Forms.targets`, at the default location and then using the `BUILD_SOURCESDIRECTORY` environment variable as a fallback - Copy all `*.targets` files to the test directory - Our `*.csproj` files under test can import the file from there. We have to copy the targets files here to be sure that MSBuild can load `Xamarin.Forms.Build.Tasks.dll`, which is also referenced by the unit tests. I also made the tests abort earlier if they can't find `Xamarin.Forms.targets`.
1 parent 8f3d7af commit b84b35b

File tree

11 files changed

+595
-53
lines changed

11 files changed

+595
-53
lines changed

‎.nuspec/Xamarin.Forms.targets‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,23 @@
6464
</CoreCompileDependsOn>
6565
</PropertyGroup>
6666

67-
<Target Name="XamlG" BeforeTargets="BeforeCompile" DependsOnTargets="PrepareResourceNames" Condition="'$(_XamlGAlreadyExecuted)'!='true'">
68-
<PropertyGroup>
69-
<_XamlGAlreadyExecuted>true</_XamlGAlreadyExecuted>
70-
</PropertyGroup>
67+
<Target Name="_FindXamlGFiles" DependsOnTargets="PrepareResourceNames">
68+
<ItemGroup>
69+
<_XamlGInputs Include="@(EmbeddedResource)" Condition="'%(Extension)' == '.xaml' AND '$(DefaultLanguageSourceExtension)' == '.cs' AND '%(TargetPath)' != ''" />
70+
<_XamlGOutputs Include="@(_XamlGInputs->'$(IntermediateOutputPath)%(TargetPath).g.cs')" />
71+
</ItemGroup>
72+
</Target>
73+
74+
<Target Name="XamlG" BeforeTargets="BeforeCompile" DependsOnTargets="_FindXamlGFiles" Inputs="@(_XamlGInputs)" Outputs="@(_XamlGOutputs)">
7175
<XamlGTask
72-
XamlFiles="@(EmbeddedResource)" Condition="'%(Extension)' == '.xaml' AND '$(DefaultLanguageSourceExtension)' == '.cs'"
73-
Language = "$(Language)"
74-
AssemblyName = "$(AssemblyName)"
75-
OutputPath = "$(IntermediateOutputPath)">
76-
<Output ItemName="FilesWrite" TaskParameter="GeneratedCodeFiles" />
77-
<Output ItemName="Compile" TaskParameter="GeneratedCodeFiles" />
78-
</XamlGTask>
76+
XamlFiles="@(_XamlGInputs)"
77+
OutputFiles="@(_XamlGOutputs)"
78+
Language="$(Language)"
79+
AssemblyName="$(AssemblyName)" />
80+
<ItemGroup>
81+
<FileWrites Include="@(_XamlGOutputs)" />
82+
<Compile Include="@(_XamlGOutputs)" />
83+
</ItemGroup>
7984
</Target>
8085

8186
<!-- XamlC -->
@@ -86,17 +91,18 @@
8691
</CompileDependsOn>
8792
</PropertyGroup>
8893

89-
<Target Name="XamlC" AfterTargets="AfterCompile" Condition="'$(_XamlCAlreadyExecuted)'!='true'">
90-
<PropertyGroup>
91-
<_XamlCAlreadyExecuted>true</_XamlCAlreadyExecuted>
92-
</PropertyGroup>
94+
<Target Name="XamlC" AfterTargets="AfterCompile" Inputs="$(IntermediateOutputPath)$(TargetFileName)" Outputs="$(IntermediateOutputPath)XamlC.stamp">
9395
<XamlCTask
9496
Assembly = "$(IntermediateOutputPath)$(TargetFileName)"
9597
ReferencePath = "@(ReferencePath)"
9698
OptimizeIL = "true"
9799
DebugSymbols = "$(DebugSymbols)"
98100
DebugType = "$(DebugType)"
99101
KeepXamlResources = "$(XFKeepXamlResources)" />
102+
<Touch Files="$(IntermediateOutputPath)XamlC.stamp" AlwaysCreate="True" />
103+
<ItemGroup>
104+
<FileWrites Include="$(IntermediateOutputPath)XamlC.stamp" />
105+
</ItemGroup>
100106
</Target>
101107

102108
<!-- CssG -->
@@ -115,7 +121,7 @@
115121
Language = "$(Language)"
116122
AssemblyName = "$(AssemblyName)"
117123
OutputPath = "$(IntermediateOutputPath)">
118-
<Output ItemName="FilesWrite" TaskParameter="GeneratedCodeFiles" />
124+
<Output ItemName="FileWrites" TaskParameter="GeneratedCodeFiles" />
119125
<Output ItemName="Compile" TaskParameter="GeneratedCodeFiles" />
120126
</CssGTask>
121127
</Target>

‎Xamarin.Forms.Build.Tasks/XamlGTask.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Collections.Generic;
32
using System.IO;
43
using System.Xml;
54

@@ -10,44 +9,45 @@ namespace Xamarin.Forms.Build.Tasks
109
{
1110
public class XamlGTask : Task
1211
{
13-
List<ITaskItem> _generatedCodeFiles = new List<ITaskItem>();
14-
1512
[Required]
1613
public ITaskItem[] XamlFiles { get; set; }
1714

18-
[Output]
19-
public ITaskItem[] GeneratedCodeFiles => _generatedCodeFiles.ToArray();
15+
[Required]
16+
public ITaskItem[] OutputFiles { get; set; }
2017

2118
public string Language { get; set; }
2219
public string AssemblyName { get; set; }
23-
public string OutputPath { get; set; }
2420

2521
public override bool Execute()
2622
{
2723
bool success = true;
2824
Log.LogMessage(MessageImportance.Normal, "Generating code behind for XAML files");
2925

30-
if (XamlFiles == null) {
26+
//NOTE: should not happen due to [Required], but there appears to be a place this is class is called directly
27+
if (XamlFiles == null || OutputFiles == null) {
3128
Log.LogMessage("Skipping XamlG");
3229
return true;
3330
}
3431

35-
foreach (var xamlFile in XamlFiles) {
36-
//when invoked from `UpdateDesigntimeXaml` target, the `TargetPath` isn't set, use a random one instead
37-
var targetPath = xamlFile.GetMetadata("TargetPath");
38-
if (string.IsNullOrWhiteSpace(targetPath))
39-
targetPath = $".{Path.GetRandomFileName()}";
32+
if (XamlFiles.Length != OutputFiles.Length) {
33+
Log.LogError("\"{2}\" refers to {0} item(s), and \"{3}\" refers to {1} item(s). They must have the same number of items.", XamlFiles.Length, OutputFiles.Length, "XamlFiles", "OutputFiles");
34+
return false;
35+
}
4036

41-
var outputFile = Path.Combine(OutputPath, $"{targetPath}.g.cs");
37+
for (int i = 0; i < XamlFiles.Length; i++) {
38+
var xamlFile = XamlFiles[i];
39+
var outputFile = OutputFiles[i].ItemSpec;
4240
if (Path.DirectorySeparatorChar == '/' && outputFile.Contains(@"\"))
4341
outputFile = outputFile.Replace('\\','/');
4442
else if (Path.DirectorySeparatorChar == '\\' && outputFile.Contains(@"/"))
4543
outputFile = outputFile.Replace('/', '\\');
46-
44+
4745
var generator = new XamlGenerator(xamlFile, Language, AssemblyName, outputFile, Log);
4846
try {
49-
if (generator.Execute())
50-
_generatedCodeFiles.Add(new TaskItem(Microsoft.Build.Evaluation.ProjectCollection.Escape(outputFile)));
47+
if (!generator.Execute()) {
48+
//If Execute() fails, the file still needs to exist because it is added to the <Compile/> ItemGroup
49+
File.WriteAllText (outputFile, string.Empty);
50+
}
5151
}
5252
catch (XmlException xe) {
5353
Log.LogError(null, null, null, xamlFile.ItemSpec, xe.LineNumber, xe.LinePosition, 0, 0, xe.Message, xe.HelpLink, xe.Source);

‎Xamarin.Forms.Xaml.UnitTests/DummyBuildEngine.cs‎ renamed to ‎Xamarin.Forms.Xaml.UnitTests/MSBuild/DummyBuildEngine.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,31 @@
11
using System;
22
using System.Collections;
3+
using System.Collections.Generic;
34
using Microsoft.Build.Framework;
45

56
namespace Xamarin.Forms.Xaml.UnitTests
67
{
78
public class DummyBuildEngine : IBuildEngine
89
{
10+
public List<BuildErrorEventArgs> Errors { get; } = new List<BuildErrorEventArgs> ();
11+
12+
public List<BuildWarningEventArgs> Warnings { get; } = new List<BuildWarningEventArgs> ();
13+
14+
public List<BuildMessageEventArgs> Messages { get; } = new List<BuildMessageEventArgs> ();
15+
916
public void LogErrorEvent (BuildErrorEventArgs e)
1017
{
18+
Errors.Add (e);
1119
}
1220

1321
public void LogWarningEvent (BuildWarningEventArgs e)
1422
{
23+
Warnings.Add (e);
1524
}
1625

1726
public void LogMessageEvent (BuildMessageEventArgs e)
1827
{
28+
Messages.Add (e);
1929
}
2030

2131
public void LogCustomEvent (CustomBuildEventArgs e)

0 commit comments

Comments
 (0)