Skip to content

Support SkipNonexistentTargets in project reference target protocol#8330

Merged
JaynieBai merged 14 commits intodotnet:mainfrom
DmitriyShepelev:skip-nonexistent-targets
Feb 7, 2023
Merged

Support SkipNonexistentTargets in project reference target protocol#8330
JaynieBai merged 14 commits intodotnet:mainfrom
DmitriyShepelev:skip-nonexistent-targets

Conversation

@DmitriyShepelev
Copy link
Copy Markdown
Contributor

@DmitriyShepelev DmitriyShepelev commented Jan 20, 2023

Fixes #4252

Context

This PR extracts the relevant logic from the closed #5297, which adds support for the SkipNonexistentTargets metadatum on the ProjectReferenceTargets item:

<ProjectReferenceTargets Include='<some-target>' Targets='<some-target1>;<some-target2>' SkipNonexistentTargets='<boolean>'>

If SkipNonexistentTargets is true, then any targets in Targets are skipped if they're nonexistent. SkipNonexistentTargets cannot be added to ProjectReferenceTargets items whose Targets contain .default or .projectReferenceTargetsOrDefaultTargets, which represent the default targets and targets specified on the ProjectReference item (with fallback to default targets if none are specified), respectively.

Changes Made

  • GetTargetLists filters out skippable nonexistent targets on referenced projects.
  • Determining whether a target is skippable required storing the defined project targets in BuildResults to ensure that corresponding BuildRequestConfigurations on the build manager node have set project targets if the build manager node created a configuration based on a request from an external node but hadn't received a result (since the project may not have been loaded locally and thus the project targets would be unknown).
  • Added SkipNonexistentTargets='true' to GetTargetFrameworks since in the non-graph case it is added to the relevant MSBuild task.

Testing

UTs and manual testing with /graph and /graph /isolate (the latter run without restore being called due to #6856) on the erroring repos I saw when testing #8249.

Notes

Addressing this since it came up when testing #8249.

cc @dfederm

Comment thread src/Build/BackEnd/Shared/BuildRequestConfiguration.cs Outdated
@dfederm
Copy link
Copy Markdown
Contributor

dfederm commented Jan 23, 2023

Is there a specific use-case for this? I see a theoretical one, but I'm not aware of anything in the typical p2p protocol which uses SkipNonexistentTargets

@Forgind Forgind added the Area: Static Graph Issues with -graph, -isolate, and the related APIs. label Jan 23, 2023
@DmitriyShepelev DmitriyShepelev force-pushed the skip-nonexistent-targets branch from b4ea87a to ac66585 Compare January 25, 2023 15:14
@DmitriyShepelev
Copy link
Copy Markdown
Contributor Author

Is there a specific use-case for this? I see a theoretical one, but I'm not aware of anything in the typical p2p protocol which uses SkipNonexistentTargets

.wixproj projects don't define GetTargetFrameworks nor GetTargetFrameworksWithPlatformForSingleTargetFramework so, if they happen to be referenced, MSBuild will emit errors about nonexistent targets being called.

Comment thread src/Tasks/Microsoft.Managed.After.targets Outdated
Comment thread src/Build/BackEnd/Shared/BuildResult.cs Outdated
Comment thread src/Build/Graph/ProjectInterpretation.cs Outdated
…pNonexistentTargets='true'` to `GetTargetFrameworksWithPlatformForSingleTargetFramework`
@DmitriyShepelev DmitriyShepelev force-pushed the skip-nonexistent-targets branch from 47fd30c to 11cb710 Compare January 25, 2023 23:48
Comment thread src/Tasks/Microsoft.Managed.After.targets
Comment thread src/Tasks/Microsoft.Managed.After.targets Outdated
Comment thread src/Build/BackEnd/Shared/BuildRequestConfiguration.cs Outdated
Comment thread src/Build/Graph/ProjectInterpretation.cs
Comment thread src/Build/BackEnd/Components/Scheduler/Scheduler.cs
@rokonec
Copy link
Copy Markdown
Member

rokonec commented Jan 30, 2023

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@DmitriyShepelev
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Commenter does not have sufficient privileges for PR 8330 in repo dotnet/msbuild

@rokonec
Copy link
Copy Markdown
Member

rokonec commented Jan 30, 2023

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec rokonec added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jan 30, 2023
@JaynieBai
Copy link
Copy Markdown
Member

@DmitriyShepelev Please help resolve the conflicts

@JaynieBai JaynieBai merged commit 446f42e into dotnet:main Feb 7, 2023
@DmitriyShepelev DmitriyShepelev deleted the skip-nonexistent-targets branch February 7, 2023 13:59
JaynieBai pushed a commit that referenced this pull request Apr 19, 2023
Update documentation given the merge of #8249, #8257, and #8330.

---------

Co-authored-by: Rainer Sigwald <raines@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Static Graph Issues with -graph, -isolate, and the related APIs. merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should static graph respect MSBuild.SkipNonExistentProjects and MSBuild.SkipNonExistentTargets?

5 participants