Emulate AssignProjectConfiguration behavior in graph construction #8625
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dfederm
commented
Apr 3, 2023
| projectReferenceItem, | ||
| requesterInstance.GlobalPropertiesDictionary, | ||
| // Only allow reuse in scenarios where we will not mutate the collection. | ||
| // TODO: Should these mutations be moved to globalPropertiesModifiers in the future? |
There was a problem hiding this comment.
Note: I'm not planning on addressing this TODO in this PR. Dynamic platform resolution already deviated from this pattern, so a future PR should fix both of these.
058f67c to
cc4fd19
Compare
rainersigwald
approved these changes
Apr 17, 2023
|
Hello! I noticed that you're targeting one of our servicing branches. Please consider updating the version. |
This was referenced Jun 7, 2023
Add support for ShouldUnsetParentConfigurationAndPlatform in solution-based graph construction
#8855
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The for sln-based builds, the
AssignProjectConfigurationtask ends up using the Configuration and Platform defined in the sln rather than passing through the global properties from the referencing project or attempting to do dynamic platform negotiation. This change adds equivalent functionality to graph construction.A concrete scenario this fixes for graph-based builds using an sln file is that most csproj define the "x86" platform while most vcxproj define "Win32". Previously for a graph build, if the csproj referenced the vcxproj, the platform passed to vcxproj would be x86, not Win32. Even worse, the vcxproj would be an entry point anyway, so it would double-build with both x86 AND Win32, which leads to race conditions.
Customer Impact
Microsoft-internal customer using sln-based builds will be able to opt-into graph builds
Regression?
No
Testing
Manual validation in the customer repo, as well as added unit tests
Risk
Low. Graph builds are a less-used feature, and this adds parity to what non-graph builds and VS-based builds do. It's unlikely that any behavioral change would be impactful due to those other scenarios presumably working for customers who may be using graph builds.