[release/6.0.4xx] [dotnet] Accept invalid runtime identifiers for Restore. #15491
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
Validating that projects are only restored using valid runtime identifiers have turned out to be an interesting rabbit hole. Nobody seems to dispute the fact that it's a correct validation, the problem is that it apparently turns out quite complicated to not do the wrong thing for projects with multiple target frameworks. In some scenarios apparently the Restore target might want to restore all target frameworks, which breaks down if whatever the user wants to do requires setting a runtime identifier, because then that runtime identifier is set for all target frameworks. So for the sake of simplicity, we're going to try removing this validation for the Restore target (we're keeping it for the Build target). There are thus two potential scenarios: 1. The Restore succeeds using invalid runtime identifiers. This shouldn't affect valid builds (since those should be completely separate due to using different runtime identifiers). 2. Something else breaks. At worst the user will be given a less comprehensible error message. Time will tell if this is better or worse than the current experience. Ref: NuGet/Home#11653 Ref: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1558247
vs-mobiletools-engineering-service2
added
backported
not-notes-worthy
📋 [PR Build] API Diff 📋API Current PR diff✅ API Diff (from PR only) (no change) View dotnet API diffView dotnet legacy API diffAPI diff✅ API Diff from stable View dotnet API diffView dotnet legacy API diffGenerator diff✅ Generator Diff (no change) Pipeline on Agent XAMBOT-1097.Monterey' |
💻 [PR Build] Tests on macOS Mac Catalina (10.15) passed 💻✅ All tests on macOS Mac Catalina (10.15) passed. Pipeline on Agent |
❌ [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌Failed tests are:
Pipeline on Agent |
rolfbjarne
approved these changes
Jul 14, 2022
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.
Validating that projects are only restored using valid runtime identifiers
have turned out to be an interesting rabbit hole.
Nobody seems to dispute the fact that it's a correct validation, the problem
is that it apparently turns out quite complicated to not do the wrong thing
for projects with multiple target frameworks.
In some scenarios apparently the Restore target might want to restore all
target frameworks, which breaks down if whatever the user wants to do requires
setting a runtime identifier, because then that runtime identifier is set for
all target frameworks.
So for the sake of simplicity, we're going to try removing this validation for
the Restore target (we're keeping it for the Build target).
There are thus two potential scenarios:
affect valid builds (since those should be completely separate due to using
different runtime identifiers).
comprehensible error message. Time will tell if this is better or worse
than the current experience.
Ref: NuGet/Home#11653
Ref: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1558247
Backport of #15357