Pack: reuse existing evaluations instead of forcing BuildProjectReferences=false

[baronfel]

Pack: stop forcing BuildProjectReferences=false in inner MSBuild calls to reuse existing evaluations

Fixes: NuGet/Home#11530
Fixes: NuGet/Home#14998

Summary

The NuGet Pack targets (NuGet.Build.Tasks.Pack.targets) orchestrate packing by calling back
into the project (and its project references) with the MSBuild task to gather versions, source
files, framework references, and suppressed dependencies. Several of those inner calls passed
BuildProjectReferences=false as a global property.

Because MSBuild keys project evaluations (and project instances) by *project path + global
properties*, adding BuildProjectReferences=false produced a distinct global-property set from
the instances that were already evaluated and built during the Build that precedes Pack. The
result was a second, redundant evaluation (and instance) for each affected target framework and
project reference — effectively doubling evaluations for multi-targeting graphs.

This change removes BuildProjectReferences=false from those inner calls so they match the
already-evaluated/already-built instances and reuse them instead of forcing new evaluations.

It additionally eliminates a second source of redundant evaluations that is specific to
single-targeting projects (see Single-targeting evaluation reuse below).

Why this is safe: BuildProjectReferences is only meaningful to the P2P protocol

The key observation is that BuildProjectReferences only influences the MSBuild
project-to-project reference (P2P) protocol — specifically ResolveProjectReferences /
_MSBuildProjectReferenceExistent, where it decides whether a referenced project is built (target
outputs produced) or merely has its outputs predicted. It is not consulted by any of the targets the
Pack targets actually invoke on these calls (_GetProjectVersion, SourceFilesProjectOutputGroup,
_GetFrameworkAssemblyReferences, _GetFrameworksWithSuppressedDependencies, and the
_WalkEachTargetPerFramework data-collection targets). None of those targets participate in the P2P
protocol.

So passing BuildProjectReferences=false gave no direct benefit to what Pack was collecting — it
only changed the global-property set, which forced MSBuild to spin up a separate evaluation and
re-compute target results, duplicating work that the preceding Build had already done. Removing it
lets Pack reuse those existing evaluations and target results with no change to the data it gathers.

Change detail

src/NuGet.Core/NuGet.Build.Tasks.Pack/NuGet.Build.Tasks.Pack.targets:

1. _GetProjectReferenceVersions — removed Properties="BuildProjectReferences=false;" from the
   MSBuild task that collects versions from project references. The child now inherits the parent's
   global properties and reuses the reference's existing (outer) evaluation.
2. _WalkEachTargetPerFramework — deleted the _ProjectsWithTFMNoBuild item entirely. It was
   identical to _ProjectsWithTFM except for the extra BuildProjectReferences=false metadata.
3. The three consumers of _ProjectsWithTFMNoBuild now use _ProjectsWithTFM
   (TargetFramework=<tfm> only):
   • SourceFilesProjectOutputGroup (only runs when IncludeSource=true)
   • _GetFrameworkAssemblyReferences
   • _GetFrameworksWithSuppressedDependencies
4. _WalkEachTargetPerFramework — _ProjectsWithTFM is now defined conditionally on whether the
   project is cross-targeting:
   • Cross-targeting ('$(TargetFrameworks)' != ''): unchanged — each inner-framework instance is
     addressed by its TargetFramework=<tfm> global property.
   • Single-targeting ('$(TargetFrameworks)' == ''): the TargetFramework global property is
     omitted so the inner MSBuild calls reuse the current (already-built) instance instead of forking
     a redundant single-TFM evaluation. This mirrors the existing IsInnerBuild detection already used
     in the same file.

Net diff: 6 insertions / 7 deletions in the targets file.

Data-equivalence

The data produced by the changed targets is identical to the old targets for the common case:

• Generated .nuspec (main and symbols) — identical.
• Resolved project-reference dependency versions — identical.
• Framework assembly references and suppressed dependencies — identical.
• Default (non-source) .nupkg — byte-identical.

Validated by A/B packing the same projects with the unmodified vs. modified targets and diffing the
nuspec/package outputs.

Side effect: source packages can contain strictly-more Compile items

There is one intentional behavioral change, and it only affects pack --include-source (i.e.
producing a .symbols.nupkg with a src/ folder). It does not affect normal packages.

SourceFilesProjectOutputGroup returns @(Compile) and depends only on
PrepareForBuild;AssignTargetPaths. Previously the BuildProjectReferences=false call resolved to a
fresh, unbuilt instance, so @(Compile) contained only the authored source files. Now that the
call reuses the already-built inner-framework instance, @(Compile) also contains the compile
items that build targets injected during Build (e.g. CoreGenerateAssemblyInfo adding
<Assembly>.AssemblyInfo.cs, and the per-framework <FrameworkMoniker>.AssemblyAttributes.cs).

Concretely, this is additive — every source file that used to be present is still present; the
source package now also includes the build-generated compile items. With the single-targeting
evaluation reuse (below), this now applies consistently to both single- and multi-targeting
projects — in both cases the src/ set gains the generated compile items, e.g.:

 src/<Project>/obj/<Config>/<tfm>/<Project>.AssemblyInfo.cs
src/<Project>/obj/<Config>/<tfm>/<FrameworkMoniker>.AssemblyAttributes.cs

Why this is acceptable

• Symbols/source packages exist to aid debugging; including the exact compiled sources (which is what
  the debugger would step into, generated files included) is defensible and arguably more correct.
• It is opt-in — only --include-source packs are affected.
• The alternative (retaining BuildProjectReferences=false on just the SourceFiles call) would keep
  a redundant evaluation for that path and was rejected in favor of eliminating the property entirely.

The existing functional test was updated to reflect this (see below).

Performance characteristics

Measured on a representative multi-targeting graph: App → Lib1..Lib6 → Core (8 projects), each
targeting net8.0;net48;netstandard2.0; default pack (no --include-source). Evaluation time is
the sum of all ProjectEvaluation durations captured in the pack binary log (with
/profileevaluation enabled). Averages of 3 iterations per cell:

Config	Thermal	Evaluations	Eval time (ms)	BuildProjectReferences evals	Wall time (ms)
baseline	cold	96	7,696	31	8,601
modified	cold	65	5,795	0	7,828
baseline	warm	96	2,817	31	3,461
modified	warm	65	2,301	0	3,372

Deltas (baseline → modified):

• Evaluation count: 96 → 65, −31 (−32%) — deterministic across every run; exactly the eliminated
  BuildProjectReferences=false evaluations.
• Cold evaluation time: −1,900 ms (−24.7%)
• Warm evaluation time: −516 ms (−18.3%)
• Cold wall time: −773 ms (−9.0%)
• Warm wall time: −89 ms (−2.6%)
• BuildProjectReferences evaluations: 31 → 0.

Notes:

• Wall-time savings are smaller than evaluation-time savings because evaluations run in parallel
  across build nodes and overlap other work; eliminated evaluations do not translate 1:1 into
  end-to-end time, but they meaningfully reduce evaluation CPU/allocation pressure.
• Absolute totals include the implicit restore, whose evaluations are identical for both configs
  (they cancel in the delta but inflate the totals). The 31-evaluation delta is purely pack-side.
• Savings scale with TFM count × project-reference count, so deeper/wider real-world graphs (the
  regression concern raised on the issue) benefit proportionally more.

Single-targeting evaluation reuse

The multi-targeting numbers above come from removing BuildProjectReferences=false. A second,
independent redundancy affects single-targeting projects: the pack targets always injected
TargetFramework=<tfm> as a global property into the inner MSBuild data-collection calls. On a
single-targeting project the main build/pack instance has no TargetFramework global property, so
adding one forks a distinct, pack-specific evaluation that duplicates the instance already built by the
preceding Build.

Detecting single-targeting ('$(TargetFrameworks)' == '', matching the existing IsInnerBuild logic)
and omitting the TargetFramework global lets those calls reuse the current instance. Measured on a
single-TFM (net8.0) project, dotnet pack:

Config	Evaluations
baseline	5
modified (BPR removal only)	4
modified + single-TFM reuse	3

The default .nuspec and .nupkg (lib/) are identical to the pre-change output (only the random
psmdcp GUID differs). The --include-source side effect described above becomes consistent between
single- and multi-targeting projects.

Test changes

test/NuGet.Core.FuncTests/Dotnet.Integration.Test/PackCommandTests.cs ::
PackCommand_IncludeSource_AddsSourceFiles

• Previously asserted an exact count of 4 src/ files. With the source-package side effect above,
  multi-targeting projects legitimately include additional generated compile items.
• Updated to assert that at least the four authored source files are present
  (ClassLibrary1.csproj, Class1.cs, Extensions/ExtensionMethods.cs, Utils/Utility.cs) via
  Assert.Contains, without pinning an exact count — tolerating the additional generated files while
  still verifying authored sources are packaged.

No other tests assert on src/ source-package contents or on BuildProjectReferences.

Test harness used to derive these conclusions

The measurements above were produced with a self-contained harness (outside the repo) rather than a
full repo build, using the SDK's shipped copy of the Pack targets as the substrate and applying the
identical edits made to the repo file. Reproduction steps:

1. Substrate / override. Pin SDK 9.0.315 via global.json. Inject a modified Pack targets file
   into dotnet pack by passing:

   • -p:CustomBeforeMicrosoftCommonProps=<override.props> where override.props sets
     ImportNuGetBuildTasksPackTargetsFromSdk=true and points NuGetPackTaskAssemblyFile at the SDK's
     real NuGet.Build.Tasks.Pack.dll (absolute path), and
   • -p:NuGetBuildTasksPackTargets=<Pack.baseline.targets | Pack.modified.targets>.

   Pack.baseline.targets is a verbatim copy of the SDK targets; Pack.modified.targets is that copy
   with the same three edits described above. (Passing both properties is required — the SDK only sets
   ImportNuGetBuildTasksPackTargetsFromSdk inside a block that is skipped when
   NuGetBuildTasksPackTargets is pre-set.)

2. Graph. App → Lib1..Lib6 → Core; every project multi-targets net8.0;net48;netstandard2.0.

3. Binlog + profiling. Each dotnet pack run emits -bl:<run>.binlog with /profileevaluation
   so evaluation timing is captured in the log.

4. Evaluation extraction. A small .NET console tool (MSBuild.StructuredLogger) loads each binlog,
   enumerates every ProjectEvaluation, and reports:

   • total evaluation count,
   • summed evaluation duration (ms),
   • count/duration of evaluations whose properties include BuildProjectReferences=false.

5. Thermal protocols.

   • Cold: dotnet build-server shutdown, then delete all project bin/obj directories, then
     pack. (Absolute evaluation times are higher — JIT/disk/first-run costs.)
   • Warm: delete only the emitted .nupkg files between runs, leaving bin/obj intact.

6. Iterations. 3 runs per (config × thermal) cell; results averaged. The evaluation count
   (96 → 65) was identical on every run; timing varied within a few percent.

7. Data-equivalence checks. Separately, the baseline vs. modified nuspec, dependency versions, and
   default .nupkg were diffed to confirm byte/data identity; the --include-source case was diffed
   to characterize the additive source-package change.

[Copilot]

Pull request overview

This PR updates NuGet’s MSBuild pack targets to avoid forcing a distinct global property set (BuildProjectReferences=false) in inner MSBuild calls, enabling MSBuild to reuse already-evaluated/already-built project instances from the preceding build and reducing redundant evaluations during pack.

Changes:

• Removed BuildProjectReferences=false from the inner MSBuild call used to collect project-reference versions (_GetProjectReferenceVersions).
• Refactored _WalkEachTargetPerFramework to reuse the existing instance for single-targeting projects (omit TargetFramework as a global property) while keeping per-TFM addressing for cross-targeting projects; retained BuildProjectReferences=false only for the NoBuild=true path where _GetFrameworkAssemblyReferences runs ResolveReferences.
• Updated the --include-source functional test to assert presence of authored sources without pinning an exact src/ file count (to allow additional build-generated Compile items when reusing built instances).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/NuGet.Core/NuGet.Build.Tasks/NuGet.Build.Tasks.Pack.targets	Stops forcing BuildProjectReferences=false in most inner pack MSBuild calls and adds single-targeting evaluation reuse; preserves NoBuild safety for _GetFrameworkAssemblyReferences.
test/NuGet.Core.FuncTests/Dotnet.Integration.Test/PackCommandTests.cs	Adjusts --include-source assertions to validate authored sources are included without requiring a fixed count.

[nkolev92]

cc @zivkan whenever you got some time, ptal.