Skip to content

Commit

Permalink
[XamlG] builds incrementally
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
jonathanpeppers committed May 17, 2018
1 parent 882bf07 commit 9b46eeb
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 33 deletions.
29 changes: 16 additions & 13 deletions .nuspec/Xamarin.Forms.targets
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,21 @@
</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'" />
<_XamlGOutputs Include="@(_XamlGInputs->'$(IntermediateOutputPath)%(TargetPath).g.cs')" />
<FileWrites Include="@(_XamlGOutputs)" />
<Compile Include="@(_XamlGOutputs)" />
</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)" />
</Target>

<!-- XamlC -->
Expand All @@ -95,7 +98,7 @@
DebugType = "$(DebugType)"
KeepXamlResources = "$(XFKeepXamlResources)" />
<Touch Files="$(IntermediateOutputPath)XamlC.stamp" AlwaysCreate="True">
<Output TaskParameter="TouchedFiles" ItemName="FileWrites"/>
<Output ItemName="FileWrites" TaskParameter="TouchedFiles" />
</Touch>
</Target>

Expand All @@ -115,7 +118,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
33 changes: 14 additions & 19 deletions Xamarin.Forms.Build.Tasks/XamlGTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,14 @@ 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()
{
Expand All @@ -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 <Compile/> 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);
Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Xaml.Xamlg/Xamlg.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) }
};


Expand Down

0 comments on commit 9b46eeb

Please sign in to comment.