perf: disable GitVersion MSBuild task globally#5266
Conversation
…terns Disable the GitVersion MSBuild task by default in Directory.Build.props, eliminating 153+ process spawns per build. CI already computes the version once and passes properties explicitly, so the per-project task was redundant. Also: - Skip CleanSourceGeneratedViewer in CI and when directory doesn't exist - Remove redundant -p:DisableGitVersionTask=true from CI workflow - Fix unquoted MSBuild condition expressions (AP-02) Incremental no-op build: ~30s → ~4s Full build: 5m27s → 4m20s
There was a problem hiding this comment.
Code Review
This is a well-targeted, low-risk performance PR. The changes are clearly motivated and the implementation is sound. A few observations:
✅ DisableGitVersionTask in Directory.Build.props
Good move. GenerateGitVersionInformation=false was already set, meaning no class was being generated — but the GitVersion MSBuild task was still spawning a process. Setting DisableGitVersionTask=true globally prevents that process from running entirely.
The 153+ per-TFM invocation problem is real in large solutions that multi-target (net8/net9/net10), and the fix here is correct: CI already computes the version once via gittools/actions/gitversion/execute and injects the properties explicitly via -p:, so the per-project task serves no purpose.
One thing worth documenting (even if just as a comment above the property): local developer builds will no longer have GitVersion-computed version properties unless they pass them explicitly. Since GenerateGitVersionInformation was already false, the generated GitVersionInformation class wasn't available anyway, so this mainly matters for anyone trying to do local NuGet pack testing. It's probably fine for this repo, but a comment clarifying the local-dev trade-off would help future maintainers avoid confusion.
✅ CleanSourceGeneratedViewer target conditions (TestProject.props)
The two new conditions are both correct:
'$(ContinuousIntegrationBuild)' != 'true'— CI doesn't need the SourceGeneratedViewer at all, so skipping the delete-and-recreate cycle there is the right call.Exists('$(ProjectDir)SourceGeneratedViewer')— avoids a no-opItemGroup+Deleteon first build before the directory exists.
The <Message> removal is also a good cleanup — unconditional MSBuild log noise hurts incremental-build usability.
One minor architectural note: the target still lacks Inputs/Outputs for proper MSBuild incremental tracking on local builds, meaning it runs before every local build when the directory exists. For a fast directory-clean target this is probably acceptable, but if the viewer directory ever grows large (many TFMs × many source generators), adding an Inputs/Outputs guard would be the idiomatic MSBuild solution.
✅ MSBuild condition quoting fixes
The $(MSBuildProjectName) == 'TUnit.Pipeline' → '$(MSBuildProjectName)' == 'TUnit.Pipeline' fixes are correct. Unquoted property references in MSBuild Condition expressions can silently evaluate unexpectedly (e.g., when the property is empty, the expression becomes == 'TUnit.Pipeline' which may parse differently across MSBuild versions). The quoted form is the canonical, safe syntax.
✅ CI workflow cleanup
Removing the now-redundant -p:DisableGitVersionTask=true overrides from both the Build and Publish AOT steps is correct housekeeping. The root Directory.Build.props now owns this setting globally.
Summary
The performance claims (~30s → ~4s no-op local, 5m27s → 4m20s CI) are plausible given the described root cause (153+ subprocess invocations). The changes are minimal, focused, and correct. The only suggestion I'd offer is adding a brief developer-facing comment near DisableGitVersionTask to explain the local-dev implication, which would make the intent clear to anyone who encounters a $(Version) that resolves empty in a local build.
Approved.
DisableGitVersionTask=true was added in #5266 but PackTUnitFilesModule was not updated to pass version properties explicitly. This caused dotnet pack to rebuild assemblies with default AssemblyVersion (1.0.0.0), creating strong-name version mismatches at runtime in consumers (FileNotFoundException for TUnit.Core).
DisableGitVersionTask=true was added in #5266 but PackTUnitFilesModule was not updated to pass version properties explicitly. This caused dotnet pack to rebuild assemblies with default AssemblyVersion (1.0.0.0), creating strong-name version mismatches at runtime in consumers (FileNotFoundException for TUnit.Core).
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.21.30 to 1.23.7. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.23.7 <!-- Release notes generated using configuration in .github/release.yml at v1.23.7 --> ## What's Changed ### Other Changes * feat: use results directory provided by Microsoft Testing Platform in HtmlReporter by @DavidZidar in thomhurst/TUnit#5294 * feat: add benchmarks for Imposter and Mockolate mocking frameworks by @vbreuss in thomhurst/TUnit#5295 * feat: add TUnit0080 analyzer for missing polyfill types by @thomhurst in thomhurst/TUnit#5292 * fix: respect user-set TUnitImplicitUsings from Directory.Build.props by @thomhurst in thomhurst/TUnit#5280 * perf: optimize TUnit.Mocks hot paths by @thomhurst in thomhurst/TUnit#5300 ### Dependencies * chore(deps): update tunit to 1.22.19 by @thomhurst in thomhurst/TUnit#5296 ## New Contributors * @DavidZidar made their first contribution in thomhurst/TUnit#5294 **Full Changelog**: thomhurst/TUnit@v1.22.19...v1.23.7 ## 1.22.19 <!-- Release notes generated using configuration in .github/release.yml at v1.22.19 --> ## What's Changed ### Other Changes * Add mock library benchmarks: TUnit.Mocks vs Moq, NSubstitute, FakeItEasy by @Copilot in thomhurst/TUnit#5284 * perf: lazily initialize optional MockEngine collections by @thomhurst in thomhurst/TUnit#5289 * Always emit TUnit.Mocks.Generated namespace from source generator by @Copilot in thomhurst/TUnit#5282 ### Dependencies * chore(deps): update tunit to 1.22.6 by @thomhurst in thomhurst/TUnit#5285 **Full Changelog**: thomhurst/TUnit@v1.22.6...v1.22.19 ## 1.22.6 <!-- Release notes generated using configuration in .github/release.yml at v1.22.6 --> ## What's Changed ### Other Changes * fix: use IComputeResource to filter waitable Aspire resources by @thomhurst in thomhurst/TUnit#5278 * fix: preserve StateBag when creating per-test TestBuilderContext by @thomhurst in thomhurst/TUnit#5279 ### Dependencies * chore(deps): update tunit to 1.22.3 by @thomhurst in thomhurst/TUnit#5275 **Full Changelog**: thomhurst/TUnit@v1.22.3...v1.22.6 ## 1.22.3 <!-- Release notes generated using configuration in .github/release.yml at v1.22.3 --> ## What's Changed ### Other Changes * fix: pass assembly version properties to dotnet pack by @thomhurst in thomhurst/TUnit#5274 ### Dependencies * chore(deps): update tunit to 1.22.0 by @thomhurst in thomhurst/TUnit#5272 **Full Changelog**: thomhurst/TUnit@v1.22.0...v1.22.3 ## 1.22.0 <!-- Release notes generated using configuration in .github/release.yml at v1.22.0 --> ## What's Changed ### Other Changes * perf: run GitVersion once in CI instead of per-project by @slang25 in thomhurst/TUnit#5259 * perf: disable GitVersion MSBuild task globally by @thomhurst in thomhurst/TUnit#5266 * fix: skip IResourceWithoutLifetime resources in Aspire fixture wait logic by @thomhurst in thomhurst/TUnit#5268 * fix: relax docs site Node.js engine constraint to >=24 by @thomhurst in thomhurst/TUnit#5269 * fix: catch unhandled exceptions in ExecuteRequestAsync to prevent IDE RPC crashes by @thomhurst in thomhurst/TUnit#5271 * feat: register HTML report as MTP session artifact by @thomhurst in thomhurst/TUnit#5270 ### Dependencies * chore(deps): update tunit to 1.21.30 by @thomhurst in thomhurst/TUnit#5254 * chore(deps): update opentelemetry to 1.15.1 by @thomhurst in thomhurst/TUnit#5258 * chore(deps): bump node-forge from 1.3.1 to 1.4.0 in /docs by @dependabot[bot] in thomhurst/TUnit#5255 * chore(deps): bump picomatch from 2.3.1 to 2.3.2 in /docs by @dependabot[bot] in thomhurst/TUnit#5256 * chore(deps): update react by @thomhurst in thomhurst/TUnit#5261 * chore(deps): update node.js to >=18.20.8 by @thomhurst in thomhurst/TUnit#5262 * chore(deps): update node.js to v24 by @thomhurst in thomhurst/TUnit#5264 **Full Changelog**: thomhurst/TUnit@v1.21.30...v1.22.0 Commits viewable in [compare view](thomhurst/TUnit@v1.21.30...v1.23.7). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Updated [TUnit.Core](https://github.com/thomhurst/TUnit) from 1.21.6 to 1.23.7. <details> <summary>Release notes</summary> _Sourced from [TUnit.Core's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.23.7 <!-- Release notes generated using configuration in .github/release.yml at v1.23.7 --> ## What's Changed ### Other Changes * feat: use results directory provided by Microsoft Testing Platform in HtmlReporter by @DavidZidar in thomhurst/TUnit#5294 * feat: add benchmarks for Imposter and Mockolate mocking frameworks by @vbreuss in thomhurst/TUnit#5295 * feat: add TUnit0080 analyzer for missing polyfill types by @thomhurst in thomhurst/TUnit#5292 * fix: respect user-set TUnitImplicitUsings from Directory.Build.props by @thomhurst in thomhurst/TUnit#5280 * perf: optimize TUnit.Mocks hot paths by @thomhurst in thomhurst/TUnit#5300 ### Dependencies * chore(deps): update tunit to 1.22.19 by @thomhurst in thomhurst/TUnit#5296 ## New Contributors * @DavidZidar made their first contribution in thomhurst/TUnit#5294 **Full Changelog**: thomhurst/TUnit@v1.22.19...v1.23.7 ## 1.22.19 <!-- Release notes generated using configuration in .github/release.yml at v1.22.19 --> ## What's Changed ### Other Changes * Add mock library benchmarks: TUnit.Mocks vs Moq, NSubstitute, FakeItEasy by @Copilot in thomhurst/TUnit#5284 * perf: lazily initialize optional MockEngine collections by @thomhurst in thomhurst/TUnit#5289 * Always emit TUnit.Mocks.Generated namespace from source generator by @Copilot in thomhurst/TUnit#5282 ### Dependencies * chore(deps): update tunit to 1.22.6 by @thomhurst in thomhurst/TUnit#5285 **Full Changelog**: thomhurst/TUnit@v1.22.6...v1.22.19 ## 1.22.6 <!-- Release notes generated using configuration in .github/release.yml at v1.22.6 --> ## What's Changed ### Other Changes * fix: use IComputeResource to filter waitable Aspire resources by @thomhurst in thomhurst/TUnit#5278 * fix: preserve StateBag when creating per-test TestBuilderContext by @thomhurst in thomhurst/TUnit#5279 ### Dependencies * chore(deps): update tunit to 1.22.3 by @thomhurst in thomhurst/TUnit#5275 **Full Changelog**: thomhurst/TUnit@v1.22.3...v1.22.6 ## 1.22.3 <!-- Release notes generated using configuration in .github/release.yml at v1.22.3 --> ## What's Changed ### Other Changes * fix: pass assembly version properties to dotnet pack by @thomhurst in thomhurst/TUnit#5274 ### Dependencies * chore(deps): update tunit to 1.22.0 by @thomhurst in thomhurst/TUnit#5272 **Full Changelog**: thomhurst/TUnit@v1.22.0...v1.22.3 ## 1.22.0 <!-- Release notes generated using configuration in .github/release.yml at v1.22.0 --> ## What's Changed ### Other Changes * perf: run GitVersion once in CI instead of per-project by @slang25 in thomhurst/TUnit#5259 * perf: disable GitVersion MSBuild task globally by @thomhurst in thomhurst/TUnit#5266 * fix: skip IResourceWithoutLifetime resources in Aspire fixture wait logic by @thomhurst in thomhurst/TUnit#5268 * fix: relax docs site Node.js engine constraint to >=24 by @thomhurst in thomhurst/TUnit#5269 * fix: catch unhandled exceptions in ExecuteRequestAsync to prevent IDE RPC crashes by @thomhurst in thomhurst/TUnit#5271 * feat: register HTML report as MTP session artifact by @thomhurst in thomhurst/TUnit#5270 ### Dependencies * chore(deps): update tunit to 1.21.30 by @thomhurst in thomhurst/TUnit#5254 * chore(deps): update opentelemetry to 1.15.1 by @thomhurst in thomhurst/TUnit#5258 * chore(deps): bump node-forge from 1.3.1 to 1.4.0 in /docs by @dependabot[bot] in thomhurst/TUnit#5255 * chore(deps): bump picomatch from 2.3.1 to 2.3.2 in /docs by @dependabot[bot] in thomhurst/TUnit#5256 * chore(deps): update react by @thomhurst in thomhurst/TUnit#5261 * chore(deps): update node.js to >=18.20.8 by @thomhurst in thomhurst/TUnit#5262 * chore(deps): update node.js to v24 by @thomhurst in thomhurst/TUnit#5264 **Full Changelog**: thomhurst/TUnit@v1.21.30...v1.22.0 ## 1.21.30 <!-- Release notes generated using configuration in .github/release.yml at v1.21.30 --> ## What's Changed ### Other Changes * feat: add test discovery Activity span for tracing by @thomhurst in thomhurst/TUnit#5246 * Fix mock generator not preserving nullable annotations on reference types by @Copilot in thomhurst/TUnit#5251 * Fix ITestSkippedEventReceiver not firing for [Skip]-attributed tests by @thomhurst in thomhurst/TUnit#5253 * Use CallerArgumentExpression for TestDataRow by default. by @m-gasser in thomhurst/TUnit#5135 ### Dependencies * chore(deps): update tunit to 1.21.24 by @thomhurst in thomhurst/TUnit#5247 **Full Changelog**: thomhurst/TUnit@v1.21.24...v1.21.30 ## 1.21.24 <!-- Release notes generated using configuration in .github/release.yml at v1.21.24 --> ## What's Changed ### Other Changes * Fix OpenTelemetry missing root span by reordering session activity lifecycle by @Copilot in thomhurst/TUnit#5245 ### Dependencies * chore(deps): update tunit to 1.21.20 by @thomhurst in thomhurst/TUnit#5241 * chore(deps): update dependency stackexchange.redis to 2.12.8 by @thomhurst in thomhurst/TUnit#5243 **Full Changelog**: thomhurst/TUnit@v1.21.20...v1.21.24 ## 1.21.20 <!-- Release notes generated using configuration in .github/release.yml at v1.21.20 --> ## What's Changed ### Other Changes * fix: respect TUnitImplicitUsings set in Directory.Build.props by @thomhurst in thomhurst/TUnit#5225 * feat: covariant assertions for interfaces and non-sealed classes by @thomhurst in thomhurst/TUnit#5226 * feat: support string-to-parseable type conversions in [Arguments] by @thomhurst in thomhurst/TUnit#5227 * feat: add string length range assertions by @thomhurst in thomhurst/TUnit#4935 * Fix BeforeEvery/AfterEvery hooks for Class and Assembly not being executed by @Copilot in thomhurst/TUnit#5239 ### Dependencies * chore(deps): update tunit to 1.21.6 by @thomhurst in thomhurst/TUnit#5228 * chore(deps): update dependency gitversion.msbuild to 6.7.0 by @thomhurst in thomhurst/TUnit#5229 * chore(deps): update dependency gitversion.tool to v6.7.0 by @thomhurst in thomhurst/TUnit#5230 * chore(deps): update aspire to 13.2.0 - autoclosed by @thomhurst in thomhurst/TUnit#5232 * chore(deps): update dependency typescript to v6 by @thomhurst in thomhurst/TUnit#5233 * chore(deps): update dependency polyfill to 9.23.0 by @thomhurst in thomhurst/TUnit#5235 * chore(deps): update dependency polyfill to 9.23.0 by @thomhurst in thomhurst/TUnit#5236 **Full Changelog**: thomhurst/TUnit@v1.21.6...v1.21.20 Commits viewable in [compare view](thomhurst/TUnit@v1.21.6...v1.23.7). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
Directory.Build.props— eliminates 153+dotnet-gitversionprocess spawns per build. CI already computes the version once viagittools/actions/gitversion/executeand passes properties explicitly, making the per-project MSBuild task redundant.CleanSourceGeneratedViewerin CI — the target was running every build with noInputs/Outputs, deleting and recreating source-gen viewer files unnecessarily. Now conditioned onContinuousIntegrationBuild != trueand directory existence.-p:DisableGitVersionTask=truefrom CI workflow — no longer needed since it's the default.Directory.Build.propsandTUnit.Templates/content/Directory.Build.props.Performance Impact
Test plan
dotnet build TUnit.CI.slnx)