Skip to content

Commit

Permalink
[One .NET] fix for incremental CoreCompile (dotnet#5661)
Browse files Browse the repository at this point in the history
Context: https://github.com/xamarin/Xamarin.Forms/tree/main-handler

I noticed that when building Maui, `CoreCompile` seems to be running
on every build no matter what:

    Building target "CoreCompile" completely.
    Input file "obj\Debug\net6.0-android\Core-net6.csproj.CoreCompileInputs.cache" is newer than output file "bin\Debug\net6.0-android\Microsoft.Maui.xml".

I could reproduce this in a test:

* Build `AppA` & `LibraryB`
* Build `AppA` & `LibraryB` again, `LibraryB` will run `CoreCompile`
  *every* time.

There is a `_GenerateCompileDependencyCache` target that basically does:

    <Hash ItemsToHash="@(CoreCompileCache)">
      <Output TaskParameter="HashResult" PropertyName="CoreCompileDependencyHash" />
    </Hash>
    <WriteLinesToFile
        Lines="$(CoreCompileDependencyHash)"
        File="$(IntermediateOutputPath)$(MSBuildProjectFile).CoreCompileInputs.cache"
        Overwrite="True"
        WriteOnlyWhenDifferent="True"
    />

https://github.com/dotnet/msbuild/blob/83cd7d4e36b71d5b2cefd02cb9a5a58d27dd6a75/src/Tasks/Microsoft.Common.CurrentVersion.targets#L3529

This `*.CoreCompileInputs.cache` file triggers `CoreCompile` to run
again when it needs to.

However, this file is actually updating on every build, because:

1. Our "outer" build has all our preprocessor defines listed in
   `@(CoreCompileCache)` like `__MOBILE__`, `__ANDROID__`, etc.
2. The "inner" build for each `$(RuntimeIdentifier)` does *not* have
   these symbols!

And so we get into a situation where `CoreCompile` will always run.
The inner & outer builds write different values in this file.

To solve this problem, I added our `_AddAndroidDefines` to run before
`CoreCompile` in inner builds.

I also needed some changes to our MSBuild test framework:

* Make `IsTargetSkipped()` and `AssertTargetIsSkipped()` supported for
  new project types.
* Make `IsTargetSkipped()` return `false` if a `Building target
  "{target}" completely.` message is found.
  • Loading branch information
jonathanpeppers authored Mar 2, 2021
1 parent 11c30ac commit 905878b
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ _ResolveAssemblies MSBuild target.
<CoreBuildDependsOn>
$([MSBuild]::Unescape($(CoreBuildDependsOn.Replace('IncrementalClean;', ''))))
</CoreBuildDependsOn>
<CompileDependsOn>
_AddAndroidDefines;
$(CompileDependsOn);
</CompileDependsOn>
</PropertyGroup>

<Target Name="_ComputeFilesToPublishForRuntimeIdentifiers"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,24 @@ public static void AssertTargetIsNotSkipped (this BuildOutput output, string tar
Assert.IsFalse (output.IsTargetSkipped (target), $"The target {target} should have *not* been skipped.");
}

[DebuggerHidden]
public static void AssertTargetIsSkipped (this DotNetCLI dotnet, string target, int? occurrence = null)
{
if (occurrence != null)
Assert.IsTrue (dotnet.IsTargetSkipped (target), $"The target {target} should have been skipped. ({occurrence})");
else
Assert.IsTrue (dotnet.IsTargetSkipped (target), $"The target {target} should have been skipped.");
}

[DebuggerHidden]
public static void AssertTargetIsNotSkipped (this DotNetCLI dotnet, string target, int? occurrence = null)
{
if (occurrence != null)
Assert.IsFalse (dotnet.IsTargetSkipped (target), $"The target {target} should have *not* been skipped. ({occurrence})");
else
Assert.IsFalse (dotnet.IsTargetSkipped (target), $"The target {target} should have *not* been skipped.");
}

[DebuggerHidden]
public static void AssertTargetIsPartiallyBuilt (this BuildOutput output, string target, int? occurrence = null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,41 @@ public void XamarinLegacySdk ()
nupkg.AssertContainsEntry (nupkgPath, $"lib/{legacyTargetFramework}/{proj.ProjectName}.dll");
}

[Test]
public void DotNetIncremental ()
{
// Setup dependencies App A -> Lib B
var path = Path.Combine ("temp", TestName);

var libB = new XASdkProject (outputType: "Library") {
ProjectName = "LibraryB"
};
libB.Sources.Clear ();
libB.Sources.Add (new BuildItem.Source ("Foo.cs") {
TextContent = () => "public class Foo { }",
});

// Will save the project, does not need to build it
CreateDotNetBuilder (libB, Path.Combine (path, libB.ProjectName));

var appA = new XASdkProject {
ProjectName = "AppA",
Sources = {
new BuildItem.Source ("Bar.cs") {
TextContent = () => "public class Bar : Foo { }",
}
}
};
appA.AddReference (libB);
var appBuilder = CreateDotNetBuilder (appA, Path.Combine (path, appA.ProjectName));
Assert.IsTrue (appBuilder.Build (), $"{appA.ProjectName} should succeed");
appBuilder.AssertTargetIsNotSkipped ("CoreCompile");

// Build again, no changes
Assert.IsTrue (appBuilder.Build (), $"{appA.ProjectName} should succeed");
appBuilder.AssertTargetIsSkipped ("CoreCompile");
}

DotNetCLI CreateDotNetBuilder (string relativeProjectDir = null)
{
if (string.IsNullOrEmpty (relativeProjectDir)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,14 @@ public List<string> GetAssemblyMapCache ()
return File.ReadLines (path).ToList ();
}

public bool IsTargetSkipped (string target)
public bool IsTargetSkipped (string target) => IsTargetSkipped (Builder.LastBuildOutput, target);

public static bool IsTargetSkipped (IEnumerable<string> output, string target)
{
bool found = false;
foreach (var line in Builder.LastBuildOutput) {
foreach (var line in output) {
if (line.Contains ($"Building target \"{target}\" completely."))
return false;
found = line.Contains ($"Target {target} skipped due to ")
|| line.Contains ($"Skipping target \"{target}\" because it has no ") //NOTE: message can say `inputs` or `outputs`
|| line.Contains ($"Target \"{target}\" skipped, due to")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ public IEnumerable<string> LastBuildOutput {
}
}

public bool IsTargetSkipped (string target) => BuildOutput.IsTargetSkipped (LastBuildOutput, target);

List<string> GetDefaultCommandLineArgs (string verb, string target = null, string [] parameters = null)
{
string testDir = Path.GetDirectoryName (projectOrSolution);
Expand Down

0 comments on commit 905878b

Please sign in to comment.