[msbuild] setup inputs and outputs for XamlC target #2230

jonathanpeppers · 2018-04-03T03:52:40Z

Description of Change

Context: https://github.com/jonathanpeppers/XamarinAndroidBuildTimes

After doing a profiling/review of Xamarin.Android project build times, I
noticed that the XamlC target was running on every build no matter
what. The project I used was the Forms Master/Detail template from VS
2017 15.6.4.

In my test repo, I was timing the following situations:

Completely clean/fresh build
Build again, no changes
Change C#, build again
Change AndroidResource XML, build again

In all cases XamlC was running, due to the task not having setup
proper inputs and outputs for the MSBuild target. Thinking about it, it
seemed like we could skip XamlC as long as the input assembly did not
change, such as case no. 2 or no. 4.

Changes to the XamlC target:

Setup $(IntermediateOutputPath)$(TargetFileName) (the assembly) as input
Setup a XamlC.stamp file as an output
<Touch /> the XamlC.stamp file after running the XamlCTask
Add to the FileWrites MSBuild item, so that the IncrementalClean
target doesn't delete the stamp file
Removed the _XamlCAlreadyExecuted property as it should not be
needed after adding inputs and outputs

On my Windows machine, this improved the following build times for cases:

same (XamlC should run)
3.685s -> 2.887s
same (XamlC should run) (would also be same as XAML changing)
12.126s -> 11.214s

Since this was basically an empty project, I suspect the improvements
would be more drastic for apps with lots of XAML and using XamlC.

Tests?

I didn't see any project where MSBuild-related tests live. Might be something needed? not sure how many MSBuild-related bugs are happening for XF.

Instead I tested the change by modifying ~\.nuget\packages\xamarin.forms\2.5.0.280555\build\netstandard1.0\Xamarin.Forms.targets and saving MSBuild binary logs. Download here.

You can find the before logs here.

Bugs Fixed

n/a

API Changes

n/a

Behavioral Changes

XamlC should only run during a build if the input assembly has changed.

PR Checklist

Has tests (if omitted, state reason in description)
Rebased on top of master at time of PR
Changes adhere to coding standard
Consolidate commits as makes sense

jonathanpeppers · 2018-04-05T16:40:15Z

.nuspec/Xamarin.Forms.targets

-		<PropertyGroup>
-			<_XamlCAlreadyExecuted>true</_XamlCAlreadyExecuted>
-		</PropertyGroup>
+	<Target Name="XamlC" Inputs="$(IntermediateOutputPath)$(TargetFileName)" Outputs="$(IntermediateOutputPath)XamlC.stamp">


I see a mistake, AfterTargets got removed. I'll put that back

StephaneDelcroix · 2018-04-11T07:19:30Z

if XamlC is ran on an already processed assembly, it mainly no-op (as the xaml files are removed from the resources), but every ms is good to shave.

@jassmith @bholmes @davidortinau: shouldn't this be targeted to 3.0.0 ?

StephaneDelcroix · 2018-04-11T07:19:37Z

build

Context: https://github.com/jonathanpeppers/XamarinAndroidBuildTimes After doing a profiling/review of Xamarin.Android project build times, I noticed that the `XamlC` target was running on every build no matter what. The project I used was the Forms Master/Detail template from VS 2017 15.6.4. In my test repo, I was timing the following situations: 1. Completely clean/fresh build 2. Build again, no changes 3. Change C#, build again 4. Change `AndroidResource` XML, build again In all cases `XamlC` was running, due to the task not having setup proper inputs and outputs for the MSBuild target. Thinking about it, it seemed like we could skip `XamlC` as long as the input assembly did not change, such as case no. 2 or no. 4. Changes to the `XamlC` target: - Setup `$(IntermediateOutputPath)$(TargetFileName)` (the assembly) as input - Setup a `XamlC.stamp` file as an output - `<Touch />` the `XamlC.stamp` file after running the `XamlCTask` - Add to the `FileWrites` MSBuild item, so that the `IncrementalClean` target doesn't delete the stamp file On my Windows machine, this improved the following build times for cases: 1. same (XamlC should run) 2. 3.685s -> 2.887s 3. same (XamlC should run) (would also be same as XAML changing) 4. 12.126s -> 11.214s Since this was basically an empty project, I suspect the improvements would be more drastic for apps with lots of XAML and using `XamlC`.

Context: xamarin#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! Other changes: - I reordered my past `<Output />` 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: dotnet/msbuild#2408 (comment) 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`

Context: xamarin#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, such as: dotnet/android#1693 Other changes: - I reordered my past `<Output />` 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: dotnet/msbuild#2408 (comment) 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`

Context: xamarin#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 Other changes: - I reordered my past `<Output />` 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: dotnet/msbuild#2408 (comment) 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`

Context: xamarin#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.

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.

* [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`.

rmarinho requested a review from StephaneDelcroix April 3, 2018 09:23

dellis1972 self-requested a review April 3, 2018 13:03

dellis1972 approved these changes Apr 3, 2018

View reviewed changes

samhouts added the t/housekeeping ♻︎ Internal only changes, won't be included in release notes label Apr 4, 2018

jonathanpeppers commented Apr 5, 2018

View reviewed changes

jonathanpeppers force-pushed the xamlc-target branch from d576545 to b5072d1 Compare April 5, 2018 16:42

StephaneDelcroix approved these changes Apr 11, 2018

View reviewed changes

jonathanpeppers force-pushed the xamlc-target branch from b5072d1 to 65fbdf3 Compare April 11, 2018 15:33

jassmith approved these changes Apr 11, 2018

View reviewed changes

StephaneDelcroix changed the base branch from master to 3.0.0 April 11, 2018 19:37

StephaneDelcroix changed the base branch from 3.0.0 to master April 11, 2018 19:37

rmarinho merged commit f1f7ef5 into xamarin:master Apr 12, 2018

jonathanpeppers deleted the xamlc-target branch April 12, 2018 12:14

samhouts added this to the 3.1.0 milestone May 5, 2018

jonathanpeppers mentioned this pull request May 17, 2018

[XamlG] builds incrementally, add MSBuild integration tests #2755

Closed

4 tasks

jassmith mentioned this pull request May 24, 2018

[XamlG] builds incrementally, add MSBuild integration tests #2825

Merged

4 tasks

samhouts modified the milestone: 3.1.0 Jun 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[msbuild] setup inputs and outputs for XamlC target #2230

[msbuild] setup inputs and outputs for XamlC target #2230

jonathanpeppers commented Apr 3, 2018 •

edited by samhouts

Loading

jonathanpeppers Apr 5, 2018

StephaneDelcroix commented Apr 11, 2018

StephaneDelcroix commented Apr 11, 2018

[msbuild] setup inputs and outputs for XamlC target #2230

[msbuild] setup inputs and outputs for XamlC target #2230

Conversation

jonathanpeppers commented Apr 3, 2018 • edited by samhouts Loading

Description of Change

Tests?

Bugs Fixed

API Changes

Behavioral Changes

PR Checklist

jonathanpeppers Apr 5, 2018

Choose a reason for hiding this comment

StephaneDelcroix commented Apr 11, 2018

StephaneDelcroix commented Apr 11, 2018

jonathanpeppers commented Apr 3, 2018 •

edited by samhouts

Loading