Removing sln level turn off of setplatform feature#8885
Merged
rainersigwald merged 1 commit intodotnet:vs17.7from Jul 12, 2023
Merged
Removing sln level turn off of setplatform feature#8885rainersigwald merged 1 commit intodotnet:vs17.7from
rainersigwald merged 1 commit intodotnet:vs17.7from
Conversation
Member
|
Can you please add a test covering this? |
Member
|
Also, what's the impact on graph + platform negotiation? |
Contributor
Author
By this do you mean automation? or a description of the testing we went through in order to validate this change |
Member
|
I'd prefer an automated test in this repo, yeah. If there aren't any end-to-end tests for this feature area maybe only the graph stuff gets tested. |
Member
rainersigwald
left a comment
There was a problem hiding this comment.
Pushed a few nits and have a few more comments here.
3c96ea7 to
6e2ac16
Compare
JanKrivanek
approved these changes
Jul 7, 2023
Member
JanKrivanek
left a comment
There was a problem hiding this comment.
Looks good to me.
Thank you!
Member
There was a problem hiding this comment.
super nit: Sln or even better SolutionFile
298a4db to
e33c544
Compare
Currently we turn off dynamic platform resolution for a whole solution if a single project in the solution is assigned a configuration. This is problematic as some projects are outside of the scope of the solution but still have certain targets that run on them that are architecture specific. These projects will build as the wrong architecture because no configuration is defined and no platform negotiation takes place. I removed the conditional that turns platform negotiation off on a sln level. The logic to turn this off on a project level is already in place through checking is a projectreference has setplatform appended to it. This will make sure no projects with configurations defined will be negotiated for as MSbuild adds setplatform metadata to projectreferences with configurations.
e33c544 to
657005a
Compare
Merged
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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
Allow dynamic platform resolution for projects not mentioned in a solution.
Customer Impact
Enables adoption of dynamic platform resolution for dev-desktop scenarios in a large internal repo.
Regression?
No.
Testing
This was tested in the VS repo on the VC sln which covers lots of scenarios
Risk
Low--feature has low adoption because of sticking points like this, so blast radius of regression is low.
Details
Context
Currently we turn off dynamic platform resolution for a whole solution if a single project in the solution is assigned a configuration. This is problematic as some projects are outside of the scope of the solution but still have certain targets that run on them that are architecture specific. These projects will build as the wrong architecture because no configuration is defined and no platform negotiation takes place.
Changes Made
I removed the conditional that turns platform negotiation off on a sln level. The logic to turn this off on a project level is already in place through checking is a projectreference has setplatform appended to it. This will make sure no projects with configurations defined will be negotiated for as MSbuild ads setplatform metadata to projectreferences with configurations.