Skip to content

Support SkipNonexistentTargets in project reference target protocol #8330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Feb 7, 2023

Conversation

DmitriyShepelev
Copy link
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

@dfederm
Copy link
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
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.

…pNonexistentTargets='true'` to `GetTargetFrameworksWithPlatformForSingleTargetFramework`
@DmitriyShepelev DmitriyShepelev force-pushed the skip-nonexistent-targets branch from 47fd30c to 11cb710 Compare January 25, 2023 23:48
@rokonec
Copy link
Member

rokonec commented Jan 30, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DmitriyShepelev
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

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

@rokonec
Copy link
Member

rokonec commented Jan 30, 2023

/azp run

@azure-pipelines
Copy link

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
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 <[email protected]>
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