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

Commit

Permalink
[XamlG] builds incrementally, add MSBuild integration tests
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jonathanpeppers committed May 24, 2018
1 parent 8f3d7af commit 760c358
Show file tree
Hide file tree
Showing 11 changed files with 571 additions and 53 deletions.
38 changes: 22 additions & 16 deletions .nuspec/Xamarin.Forms.targets
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,23 @@
</CoreCompileDependsOn>
</PropertyGroup>

<Target Name="XamlG" BeforeTargets="BeforeCompile" DependsOnTargets="PrepareResourceNames" Condition="'$(_XamlGAlreadyExecuted)'!='true'">
<PropertyGroup>
<_XamlGAlreadyExecuted>true</_XamlGAlreadyExecuted>
</PropertyGroup>
<Target Name="_FindXamlGFiles" DependsOnTargets="PrepareResourceNames">
<ItemGroup>
<_XamlGInputs Include="@(EmbeddedResource)" Condition="'%(Extension)' == '.xaml' AND '$(DefaultLanguageSourceExtension)' == '.cs' AND '%(TargetPath)' != ''" />
<_XamlGOutputs Include="@(_XamlGInputs->'$(IntermediateOutputPath)%(TargetPath).g.cs')" />
</ItemGroup>
</Target>

<Target Name="XamlG" BeforeTargets="BeforeCompile" DependsOnTargets="_FindXamlGFiles" Inputs="@(_XamlGInputs)" Outputs="@(_XamlGOutputs)">
<XamlGTask
XamlFiles="@(EmbeddedResource)" Condition="'%(Extension)' == '.xaml' AND '$(DefaultLanguageSourceExtension)' == '.cs'"
Language = "$(Language)"
AssemblyName = "$(AssemblyName)"
OutputPath = "$(IntermediateOutputPath)">
<Output ItemName="FilesWrite" TaskParameter="GeneratedCodeFiles" />
<Output ItemName="Compile" TaskParameter="GeneratedCodeFiles" />
</XamlGTask>
XamlFiles="@(_XamlGInputs)"
OutputFiles="@(_XamlGOutputs)"
Language="$(Language)"
AssemblyName="$(AssemblyName)" />
<ItemGroup>
<FileWrites Include="@(_XamlGOutputs)" />
<Compile Include="@(_XamlGOutputs)" />
</ItemGroup>
</Target>

<!-- XamlC -->
Expand All @@ -86,17 +91,18 @@
</CompileDependsOn>
</PropertyGroup>

<Target Name="XamlC" AfterTargets="AfterCompile" Condition="'$(_XamlCAlreadyExecuted)'!='true'">
<PropertyGroup>
<_XamlCAlreadyExecuted>true</_XamlCAlreadyExecuted>
</PropertyGroup>
<Target Name="XamlC" AfterTargets="AfterCompile" Inputs="$(IntermediateOutputPath)$(TargetFileName)" Outputs="$(IntermediateOutputPath)XamlC.stamp">
<XamlCTask
Assembly = "$(IntermediateOutputPath)$(TargetFileName)"
ReferencePath = "@(ReferencePath)"
OptimizeIL = "true"
DebugSymbols = "$(DebugSymbols)"
DebugType = "$(DebugType)"
KeepXamlResources = "$(XFKeepXamlResources)" />
<Touch Files="$(IntermediateOutputPath)XamlC.stamp" AlwaysCreate="True" />
<ItemGroup>
<FileWrites Include="$(IntermediateOutputPath)XamlC.stamp" />
</ItemGroup>
</Target>

<!-- CssG -->
Expand All @@ -115,7 +121,7 @@
Language = "$(Language)"
AssemblyName = "$(AssemblyName)"
OutputPath = "$(IntermediateOutputPath)">
<Output ItemName="FilesWrite" TaskParameter="GeneratedCodeFiles" />
<Output ItemName="FileWrites" TaskParameter="GeneratedCodeFiles" />
<Output ItemName="Compile" TaskParameter="GeneratedCodeFiles" />
</CssGTask>
</Target>
Expand Down
32 changes: 16 additions & 16 deletions Xamarin.Forms.Build.Tasks/XamlGTask.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Xml;

Expand All @@ -10,44 +9,45 @@ namespace Xamarin.Forms.Build.Tasks
{
public class XamlGTask : Task
{
List<ITaskItem> _generatedCodeFiles = new List<ITaskItem>();

[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()
{
bool success = true;
Log.LogMessage(MessageImportance.Normal, "Generating code behind for XAML files");

if (XamlFiles == null) {
//NOTE: should not happen due to [Required], but there appears to be a place this is class is called directly
if (XamlFiles == null || OutputFiles == null) {
Log.LogMessage("Skipping XamlG");
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.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");
return false;
}

var outputFile = Path.Combine(OutputPath, $"{targetPath}.g.cs");
for (int i = 0; i < XamlFiles.Length; i++) {
var xamlFile = XamlFiles[i];
var outputFile = OutputFiles[i].ItemSpec;
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);
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 <Compile/> ItemGroup
File.WriteAllText (outputFile, string.Empty);
}
}
catch (XmlException xe) {
Log.LogError(null, null, null, xamlFile.ItemSpec, xe.LineNumber, xe.LinePosition, 0, 0, xe.Message, xe.HelpLink, xe.Source);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
using System;
using System.Collections;
using System.Collections.Generic;
using Microsoft.Build.Framework;

namespace Xamarin.Forms.Xaml.UnitTests
{
public class DummyBuildEngine : IBuildEngine
{
public List<BuildErrorEventArgs> Errors { get; } = new List<BuildErrorEventArgs> ();

public List<BuildWarningEventArgs> Warnings { get; } = new List<BuildWarningEventArgs> ();

public List<BuildMessageEventArgs> Messages { get; } = new List<BuildMessageEventArgs> ();

public void LogErrorEvent (BuildErrorEventArgs e)
{
Errors.Add (e);
}

public void LogWarningEvent (BuildWarningEventArgs e)
{
Warnings.Add (e);
}

public void LogMessageEvent (BuildMessageEventArgs e)
{
Messages.Add (e);
}

public void LogCustomEvent (CustomBuildEventArgs e)
Expand Down
Loading

0 comments on commit 760c358

Please sign in to comment.