Merge pull request #508 from dotnet/perfImprovements

AArnott · web-flow · commit 4f10374e8c5f · 2020-09-04T17:27:33.000-06:00
Invoke `GetBuildVersion` MSBuild task only once per project (or per repo)
diff --git a/doc/globalproperties_log.png b/doc/globalproperties_log.png
diff --git a/doc/msbuild.md b/doc/msbuild.md
@@ -1,7 +1,6 @@
 # MSBuild
 
-Installing the Nerdbank.GitVersioning package from NuGet into your
-MSBuild-based projects 
+Installing the `Nerdbank.GitVersioning` package from NuGet into your MSBuild-based projects is the recommended way to add version information to your MSBuild project outputs including assemblies, native dll's, and packages.
 
 ## Public releases
 
@@ -12,11 +11,56 @@ in order to avoid the git commit ID being included in the NuGet package version.
 
 From the command line, building a release version might look like this:
 
-    msbuild /p:PublicRelease=true
+```ps1
+msbuild /p:PublicRelease=true
+```
 
 Note you may consider passing this switch to any build that occurs in the
-branch that you publish released NuGet packages from. 
+branch that you publish released NuGet packages from.
 You should only build with this property set from one release branch per
 major.minor version to avoid the risk of producing multiple unique NuGet
 packages with a colliding version spec.
 
+## Build performance
+
+Repos with many projects or many commits between major/minor version bumps can suffer from compromised build performance due to the MSBuild task that computes the version information for each project.
+You can assess the impact that Nerdbank.GitVersioning has on your build time by running a build with the `-clp:PerformanceSummary` switch and look for the `Nerdbank.GitVersioning.Tasks.GetBuildVersion` task.
+
+### Reducing `GetBuildVersion` invocations
+
+If the `GetBuildVersion` task is running many times, yet you have just one (or a few) `version.json` files in your repository, you can reduce this task to being called just once per `version.json` file to *significantly* improve build performance.
+To do this, drop a `Directory.Build.props` file in the same directory as your `version.json` file(s) with this content:
+
+```xml
+<Project>
+  <PropertyGroup>
+    <GitVersionBaseDirectory>$(MSBuildThisFileDirectory)</GitVersionBaseDirectory>
+  </PropertyGroup>
+</Project>
+```
+
+This MSBuild property instructs all projects in or under that directory to share a computed version based on that directory rather than their individual project directories.
+Check the impact of this change by re-running MSBuild with the `-clp:PerformanceSummary` switch as described above.
+
+If you still see many invocations, you may have a build that sets global properties on P2P references.
+Investigate this using the MSBuild `/bl` switch and view the log with the excellent [MSBuild Structured Log Viewer](https://msbuildlog.com) tool.
+Using that tool, search for `$task GetBuildVersion` and look at the global properties passed to the special `Nerdbank.GitVersioning.Inner.targets` project, as shown:
+
+[![MSBuild Structure Logger screenshot](globalproperties_log.png)
+
+Compare the set of global properties for each `Nerdbank.GitVersioning.Inner.targets` project to identify which properties are unique each time.
+Add the names of the unique properties to the `Directory.Build.props` file:
+
+```xml
+<Project>
+  <PropertyGroup>
+    <GitVersionBaseDirectory>$(MSBuildThisFileDirectory)</GitVersionBaseDirectory>
+  </PropertyGroup>
+  <ItemGroup>
+    <NBGV_GlobalPropertiesToRemove Include="ChangingProperty1" />
+    <NBGV_GlobalPropertiesToRemove Include="ChangingProperty2" />
+  </ItemGroup>
+</Project>
+```
+
+That should get you down to one invocation of the `GetBuildVersion` task per `version.json` file in your repo.
diff --git a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj
@@ -12,7 +12,7 @@
     <Compile Include="..\Shared\**\*.cs" LinkBase="Shared" />
   </ItemGroup>
   <ItemGroup>
-    <EmbeddedResource Include="..\Nerdbank.GitVersioning.Tasks\build\Nerdbank.GitVersioning.targets">
+    <EmbeddedResource Include="..\Nerdbank.GitVersioning.Tasks\build\*.targets">
       <Visible>false</Visible>
       <Link>Targets\%(FileName)%(Extension)</Link>
     </EmbeddedResource>
diff --git a/src/Nerdbank.GitVersioning.Tasks/GetBuildVersion.cs b/src/Nerdbank.GitVersioning.Tasks/GetBuildVersion.cs
@@ -23,7 +23,7 @@ public GetBuildVersion()
         /// <summary>
         /// Gets or sets identifiers to append as build metadata.
         /// </summary>
-        public string[] BuildMetadata { get; set; }
+        public string BuildMetadata { get; set; }
 
         /// <summary>
         /// Gets or sets the value of the PublicRelease property in MSBuild at the
@@ -33,15 +33,15 @@ public GetBuildVersion()
         public string DefaultPublicRelease { get; set; }
 
         /// <summary>
-        /// Gets or sets the path to the repo root. If null or empty, behavior defaults to using Environment.CurrentDirectory and searching upwards.
+        /// Gets or sets the path to the repo root. If null or empty, behavior defaults to using <see cref="ProjectDirectory"/> and searching upwards.
         /// </summary>
         public string GitRepoRoot { get; set; }
 
         /// <summary>
         /// Gets or sets the relative path from the <see cref="GitRepoRoot"/> to the directory under it that contains the project being built.
         /// </summary>
         /// <value>
-        /// If not supplied, the directories from <see cref="GitRepoRoot"/> to <see cref="Environment.CurrentDirectory"/>
+        /// If not supplied, the directories from <see cref="GitRepoRoot"/> to <see cref="ProjectDirectory"/>
         /// will be searched for version.json.
         /// If supplied, the value <em>must</em> fall beneath the <see cref="GitRepoRoot"/> (i.e. this value should not contain "..\").
         /// </value>
@@ -51,6 +51,11 @@ public GetBuildVersion()
         /// </remarks>
         public string ProjectPathRelativeToGitRepoRoot { get; set; }
 
+        /// <summary>
+        /// Gets or sets the path to the project directory.
+        /// </summary>
+        public string ProjectDirectory { get; set; }
+
         /// <summary>
         /// Gets or sets the optional override build number offset.
         /// </summary>
@@ -66,6 +71,12 @@ public GetBuildVersion()
         [Required]
         public string TargetsPath { get; set; }
 
+        /// <summary>
+        /// Gets or sets the list of properties to be set in MSBuild.
+        /// </summary>
+        [Output]
+        public ITaskItem[] OutputProperties { get; set; }
+
         /// <summary>
         /// Gets or sets a value indicating whether the project is building
         /// in PublicRelease mode.
@@ -202,15 +213,15 @@ protected override bool ExecuteInner()
 
                 var cloudBuild = CloudBuild.Active;
                 var overrideBuildNumberOffset = (this.OverrideBuildNumberOffset == int.MaxValue) ? (int?)null : this.OverrideBuildNumberOffset;
-                var oracle = VersionOracle.Create(Directory.GetCurrentDirectory(), this.GitRepoRoot, cloudBuild, overrideBuildNumberOffset, this.ProjectPathRelativeToGitRepoRoot);
+                var oracle = VersionOracle.Create(this.ProjectDirectory, this.GitRepoRoot, cloudBuild, overrideBuildNumberOffset, this.ProjectPathRelativeToGitRepoRoot);
                 if (!string.IsNullOrEmpty(this.DefaultPublicRelease))
                 {
                     oracle.PublicRelease = string.Equals(this.DefaultPublicRelease, "true", StringComparison.OrdinalIgnoreCase);
                 }
 
                 if (this.BuildMetadata != null)
                 {
-                    oracle.BuildMetadata.AddRange(this.BuildMetadata);
+                    oracle.BuildMetadata.AddRange(this.BuildMetadata.Split(';'));
                 }
 
                 if (IsMisconfiguredPrereleaseAndSemVer1(oracle))
@@ -266,6 +277,41 @@ protected override bool ExecuteInner()
                     this.CloudBuildVersionVars = cloudBuildVersionVars.ToArray();
                 }
 
+                var outputProperties = new Dictionary<string, PropertySet>(StringComparer.OrdinalIgnoreCase)
+                {
+                    { "BuildVersion", this.Version },
+                    { "AssemblyInformationalVersion", this.AssemblyInformationalVersion },
+                    { "AssemblyFileVersion", this.AssemblyFileVersion },
+                    { "FileVersion", this.AssemblyFileVersion },
+                    { "BuildVersionSimple", this.SimpleVersion },
+                    { "PrereleaseVersion", this.PrereleaseVersion },
+                    { "MajorMinorVersion", this.MajorMinorVersion },
+                    { "AssemblyVersion", this.AssemblyVersion },
+                    { "GitCommitId", this.GitCommitId },
+                    { "GitCommitIdShort", this.GitCommitIdShort },
+                    { "GitCommitDateTicks", this.GitCommitDateTicks },
+                    { "GitVersionHeight", this.GitVersionHeight.ToString(CultureInfo.InvariantCulture) },
+                    { "BuildNumber", this.BuildNumber.ToString(CultureInfo.InvariantCulture) },
+                    { "BuildVersionNumberComponent", this.BuildNumber.ToString(CultureInfo.InvariantCulture) },
+                    { "PublicRelease", this.PublicRelease.ToString(CultureInfo.InvariantCulture) },
+                    { "BuildingRef", this.BuildingRef },
+                    { "CloudBuildNumber", new PropertySet(this.CloudBuildNumber) { HonorPresetValue = true } },
+                    { "SemVerBuildSuffix", this.BuildMetadataFragment },
+                    { "NuGetPackageVersion", this.NuGetPackageVersion },
+                    { "ChocolateyPackageVersion", this.ChocolateyPackageVersion },
+                    { "Version", this.NuGetPackageVersion },
+                    { "PackageVersion", this.NuGetPackageVersion },
+                    { "NPMPackageVersion", this.NpmPackageVersion.ToString(CultureInfo.InvariantCulture) },
+                    { "BuildVersion3Components", $"{this.MajorMinorVersion}.{this.BuildNumber}" },
+                };
+                this.OutputProperties = outputProperties.Select(kv =>
+                {
+                    var item = new TaskItem(kv.Key);
+                    item.SetMetadata("Value", kv.Value.Value);
+                    item.SetMetadata("HonorPresetValue", kv.Value.HonorPresetValue ? "true" : "false");
+                    return item;
+                }).ToArray();
+
                 return !this.Log.HasLoggedErrors;
             }
             catch (ArgumentOutOfRangeException ex)
@@ -292,5 +338,20 @@ private static bool IsMisconfiguredPrereleaseAndSemVer1(VersionOracle oracle)
             Requires.NotNull(oracle, nameof(oracle));
             return oracle.VersionOptions?.NuGetPackageVersion?.SemVer == 1 && oracle.PrereleaseVersion != SemanticVersionExtensions.MakePrereleaseSemVer1Compliant(oracle.PrereleaseVersion, 0);
         }
+
+        private struct PropertySet
+        {
+            public PropertySet(string value)
+            {
+                this.Value = value;
+                this.HonorPresetValue = false;
+            }
+
+            public string Value { get; set; }
+
+            public bool HonorPresetValue { get; set; }
+
+            public static implicit operator PropertySet(string value) => new PropertySet(value);
+        }
     }
 }
diff --git a/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.nuspec b/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.nuspec
@@ -43,6 +43,8 @@ IMPORTANT: The 3.x release may produce a different version height than prior maj
     <file src="$BaseOutputPath$netcoreapp2.0\Validation.dll" target="build\MSBuildCore\Validation.dll" />
 
     <file src="build\Nerdbank.GitVersioning.targets" target="build\Nerdbank.GitVersioning.targets" />
+    <file src="build\Nerdbank.GitVersioning.Common.targets" target="build\Nerdbank.GitVersioning.Common.targets" />
+    <file src="build\Nerdbank.GitVersioning.Inner.targets" target="build\Nerdbank.GitVersioning.Inner.targets" />
     <file src="buildCrossTargeting\Nerdbank.GitVersioning.targets" target="buildCrossTargeting\Nerdbank.GitVersioning.targets" />
     <file src="tools\Create-VersionFile.ps1" target="tools\Create-VersionFile.ps1" />
     <file src="tools\Get-CommitId.ps1" target="tools\Get-CommitId.ps1" />
diff --git a/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.Common.targets b/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.Common.targets
@@ -0,0 +1,8 @@
+﻿<?xml version="1.0" encoding="utf-8"?>
+<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>
+    <_NBGV_PlatformSuffix Condition=" '$(_NBGV_PlatformSuffix)' == '' and '$(MSBuildRuntimeType)' == 'Core' ">MSBuildCore/</_NBGV_PlatformSuffix>
+    <_NBGV_PlatformSuffix Condition=" '$(_NBGV_PlatformSuffix)' == '' ">MSBuildFull/</_NBGV_PlatformSuffix>
+    <NerdbankGitVersioningTasksPath Condition=" '$(NerdbankGitVersioningTasksPath)' == '' ">$(MSBuildThisFileDirectory)$(_NBGV_PlatformSuffix)</NerdbankGitVersioningTasksPath>
+  </PropertyGroup>
+</Project>
diff --git a/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.Inner.targets b/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.Inner.targets
@@ -0,0 +1,45 @@
+﻿<?xml version="1.0" encoding="utf-8"?>
+<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+
+  <Import Project="Nerdbank.GitVersioning.Common.targets"/>
+
+  <UsingTask AssemblyFile="$(NerdbankGitVersioningTasksPath)Nerdbank.GitVersioning.Tasks.dll" TaskName="Nerdbank.GitVersioning.Tasks.GetBuildVersion"/>
+  
+  <Target Name="GetBuildVersion_Properties" 
+          DependsOnTargets="GetBuildVersionCore"
+          Returns="@(NBGV_PropertyItems)" />
+  
+  <Target Name="GetBuildVersion_CloudBuildVersionVars" 
+          DependsOnTargets="GetBuildVersionCore"
+          Returns="@(CloudBuildVersionVars)" />
+  
+  <Target Name="GetBuildVersionCore">
+    <Nerdbank.GitVersioning.Tasks.GetBuildVersion
+      BuildingRef="$(_NBGV_BuildingRef)"
+      BuildMetadata="$(BuildMetadata.Replace(',',';'))"
+      DefaultPublicRelease="$(PublicRelease)"
+      ProjectDirectory="$(ProjectDirectory)"
+      GitRepoRoot="$(GitRepoRoot)"
+      ProjectPathRelativeToGitRepoRoot="$(ProjectPathRelativeToGitRepoRoot)"
+      OverrideBuildNumberOffset="$(OverrideBuildNumberOffset)"
+      TargetsPath="$(MSBuildThisFileDirectory)">
+      
+      <!-- All properties and items are to be exported to the calling project through items. -->
+      <Output TaskParameter="OutputProperties" ItemName="NBGV_PropertyItems"/>
+      <Output TaskParameter="CloudBuildVersionVars" ItemName="CloudBuildVersionVars" />
+
+      <!-- Export a couple of properties directly to support our tasks below. -->
+      <Output TaskParameter="AssemblyInformationalVersion" PropertyName="AssemblyInformationalVersion" />
+      <Output TaskParameter="NuGetPackageVersion" PropertyName="NuGetPackageVersion" />
+      <Output TaskParameter="Version" PropertyName="BuildVersion" />
+      <Output TaskParameter="GitCommitId" PropertyName="GitCommitId" />
+    </Nerdbank.GitVersioning.Tasks.GetBuildVersion>
+
+    <Warning Condition=" '$(AssemblyInformationalVersion)' == '' " Text="Unable to determine the git HEAD commit ID to use for informational version number." />
+    <Message Condition=" '$(AssemblyInformationalVersion)' != '' " Text="Building version $(BuildVersion) from commit $(GitCommitId)"/>
+    <Message Condition=" '$(AssemblyInformationalVersion)' == '' " Text="Building version $(BuildVersion)"/>
+    <Message Importance="low" Text="AssemblyInformationalVersion: $(AssemblyInformationalVersion)" />
+    <Message Importance="low" Text="NuGetPackageVersion: $(NuGetPackageVersion)" />
+  </Target>
+
+</Project>
diff --git a/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets b/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets
