Skip to content

only call package restore targets on transitive projects - #15717

Open
brettfo wants to merge 1 commit into
mainfrom
dev/brettfo/nuget-restore-build
Open

only call package restore targets on transitive projects#15717
brettfo wants to merge 1 commit into
mainfrom
dev/brettfo/nuget-restore-build

Conversation

@brettfo

@brettfo brettfo commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

When discovering project dependencies, we directly invoke the targets Restore, ResolveProjectReferences, and GenerateBuildDependencyFile. When MSBuild traverses through <ProjectReference> elements, it will call Build which indirectly calls the targets we care about. This extra call to Build can be slow and fill the log with unnecessary errors, although it doesn't prevent dependency discovery.

The fix is to explicitly set what targets can be called on project references to the ones we want.

Copilot AI review requested due to automatic review settings July 29, 2026 20:00
@github-actions github-actions Bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Jul 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Limits MSBuild project-reference traversal to dependency-discovery targets, avoiding unnecessary builds.

Changes:

  • Configures ProjectReferenceBuildTargets.
  • Adds sentinel-based regression coverage.
Show a summary per file
File Description
SdkProjectDiscovery.cs Restricts targets invoked on referenced projects.
DiscoveryWorkerTests.Project.cs Verifies referenced projects are not built.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 29, 2026 20:48
@brettfo
brettfo force-pushed the dev/brettfo/nuget-restore-build branch from a7903e1 to 80a9230 Compare July 29, 2026 20:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (1)

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs:182

  • requiredTargets is chosen from the starting project's style, but this global property is inherited by every referenced project. In a mixed graph, an SDK-style root therefore asks a legacy reference to run Restore and GenerateBuildDependencyFile, even though this file's existing legacy branch documents that those targets are absent; the reverse direction restricts an SDK reference to only ResolveProjectReferences. This can produce missing-target errors or incomplete dependency data. Please select the target set in the context of each referenced project—for example, invoke a discovery target from the injected targets file that conditionally depends on the appropriate SDK/legacy targets.
                args.Add($"/p:ProjectReferenceBuildTargets=\"{string.Join(";", requiredTargets)}\"");
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@brettfo
brettfo marked this pull request as ready for review July 29, 2026 21:20
@brettfo
brettfo requested a review from a team as a code owner July 29, 2026 21:20

@mohamedelhalosy2023-stack mohamedelhalosy2023-stack left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: dotnet:nuget NuGet packages via nuget or dotnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants