[Breaking] RuntimeIdentifier No Longer Implies SelfContained Apps by Default
#30038
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
nagilson
changed the title
RuntimeIdentifier No Longer Implies SelfContained Apps by Default
Update ILLink Tests to use SelfContained explicitly Repair Publish* Tests that will be broken and now need to add SC Update publish trimmed test to expect self contained to be explicitly specified Update Publish* related tests to add SC PublishSingleFile test adds SC now IL Link and Publish* Tests now add SelfContained where a RuntimeIdentifier used to infer it automaticall per .NET 8 breaking change
Make blazor tests not give SC if already defined. Fix additional tests expecting old RID -> SC behavior Blazor WASM Update test assets Blazor WASM Tests Updated to add SelfContained when a RuntimeIdentifier is specified to keep old behavior after .NET 8 breaking change. GlobalPropertyFlowTests now expect RID to not Infer SC * Note that the failure was moved from 1 function to another ... technically the old test failed because it was relying on RID setting SC which would cause a failure as one EXE was SC but the other wasn't. With the new change, that no longer occured Meanwhile, the other test relied on the properties flowing the RID. When the RID flowed, SC would be inferred, so the referenced project would be an EXE. But now it's not inferred, so this test should instead fail.
…er expect SC to give a RID.
…ther blazor test asset to add SC
… for 8 breaking change
nagilson
force-pushed
the
nagilson-rid-no-more-sc-2.0
branch
from
016b5ba
to
0042e6e
Compare
… test projects" This reverts commit 552b1b3.
5 tasks
Remove the old self-contained warning as it would be redundant with the breaking change warning and only occurred with -r. Co-authored-by: richlander <rlander@microsoft.com>
nagilson
force-pushed
the
nagilson-rid-no-more-sc-2.0
branch
from
48d7a7f
to
c0ce586
Compare
…is correct regardless of the value
sbomer
approved these changes
Jan 26, 2023
src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToRunILLink.cs
Outdated
Show resolved
Hide resolved
src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToRunILLink.cs
Outdated
Show resolved
Hide resolved
src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToRunILLink.cs
Outdated
Show resolved
Hide resolved
…stake Co-authored-by: Sven Boemer <sbomer@gmail.com>
Co-authored-by: Sven Boemer <sbomer@gmail.com>
Co-authored-by: Sven Boemer <sbomer@gmail.com>
dsplaisted
reviewed
May 2, 2023
src/Tasks/Microsoft.NET.Build.Tasks/ValidateExecutableReferences.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>
…dentifier is Undesired We do this as SelfContained can imply RuntimeIdentifier and before RID used to imply SelfContained, and we want this beahvior to be coupled in this test Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>
…k to error in older selfcontained exe reference scenarios
dsplaisted
approved these changes
May 5, 2023
src/Tasks/Microsoft.NET.Build.Tasks/ValidateExecutableReferences.cs
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/ValidateExecutableReferences.cs
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/ValidateExecutableReferences.cs
Outdated
Show resolved
Hide resolved
…ldve been deleted Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>
…d version class to simplify code Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>
|
Here's the breaking change doc: dotnet/docs#33726 |
jonathanpeppers
added a commit
to dotnet/android
that referenced
this pull request
May 12, 2023
This was referenced May 12, 2023
jonathanpeppers
added a commit
to dotnet/android
that referenced
this pull request
May 16, 2023
Fixes: dotnet/sdk#32539 Changes: dotnet/installer@0ce8918...8488614 Changes: dotnet/runtime@9a7db55...888bac3 Changes: dotnet/emsdk@31a4a87...ab09b0b Changes: dotnet/cecil@80d3f38...c32f0be Updates: * Microsoft.Dotnet.Sdk.Internal: from 8.0.100-preview.5.23228.7 to 8.0.100-preview.5.23264.2 * Microsoft.NETCore.App.Ref: from 8.0.0-preview.4.23225.14 to 8.0.0-preview.5.23260.3 * Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100.Transport: from 8.0.0-preview.4.23219.1 to 8.0.0-preview.5.23252.1 * Microsoft.NET.ILLink.Tasks: from 8.0.0-preview.4.23225.14 to 8.0.0-preview.5.23260.3 * Microsoft.DotNet.Cecil: from 0.11.4-alpha.23218.2 to 0.11.4-alpha.23252.1 ~~ Other changes ~~ In [dotnet/sdk#30038][0], the default value for `$(SelfContained)` was changed for various types of projects. Unforunately, this broke `Release` builds on Android. Opting into non-self-contained builds triggers: error XALNS7028: System.IO.FileNotFoundException: Could not load assembly 'mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e'. Perhaps it doesn't exist in the Mono for Android profile? But we can solve the issues by: * Pass `$(SelfContained)` as `true` for all "inner builds" that run for each `$(RuntimeIdentifier)`. * Set `$(UseCurrentRuntimeIdentifier)` by default, so we don't opt into the Host's RID. Builds would select `win-x64` or `osx-64` otherwise. [0]: dotnet/sdk#30038 Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Jun 20, 2023
Fixes: dotnet/maui#15696 Context: dotnet/sdk#30038 In .NET 8 Preview 5, there are various changes to default values: * `dotnet publish` is now `Release` by default * Builds that provide a `$(RuntimeIdentifier)` are no longer "self contained" by default. The result of this change is the problem: dotnet new android dotnet publish Microsoft.NET.RuntimeIdentifierInference.targets(212,5): error NETSDK1191: A runtime identifier for the property 'SelfContained' couldn't be inferred. Specify a rid explicitly. While these commands all work: dotnet build dotnet build -c Release dotnet publish -c Debug dotnet publish -r android-arm64 `Debug` configurations work fine, because trimming is not involved. `dotnet build` works fine, because the offending default appears to be: https://github.com/dotnet/sdk/blob/540b197311bc8d1c2a991fed1b935b094a4d7b2d/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets#L64-L76 `Microsoft.NET.RuntimeIdentifierInference.targets` has a lot of MSBuild validation logic, that its main job is to emit nice error messages for incorrect combinations of MSBuild properties/settings. Android is kind of the odd one out when you compare to .NET console apps, NativeAOT, etc.: * We default to `RuntimeIdentifiers=android-arm;android-arm64;android-x86;android-x64`. * We do our own "inner build" for each `RuntimeIdentifier`. * Inside our build we force `$(SelfContained)` to `true` no matter what. As there is no such thing as a system-wide .NET on Android. The fix appears to be to default `PublishSelfContained=false` and let our existing logic force `SelfContained=true`. I added a new test for this scenario. Our existing test didn't work because it was passing a `RuntimeIdentifier`.
jonathanpeppers
added a commit
to dotnet/android
that referenced
this pull request
Jun 20, 2023
Fixes: dotnet/maui#15696 Context: dotnet/sdk#30038 In .NET 8 Preview 5, there are various changes to default values: * `dotnet publish` is now `Release` by default * Builds that provide a `$(RuntimeIdentifier)` are no longer "self contained" by default. The result of this change is the problem: dotnet new android dotnet publish Microsoft.NET.RuntimeIdentifierInference.targets(212,5): error NETSDK1191: A runtime identifier for the property 'SelfContained' couldn't be inferred. Specify a rid explicitly. While these commands all work: dotnet build dotnet build -c Release dotnet publish -c Debug dotnet publish -r android-arm64 `Debug` configurations work fine, because trimming is not involved. `dotnet build` works fine, because the offending default appears to be: https://github.com/dotnet/sdk/blob/540b197311bc8d1c2a991fed1b935b094a4d7b2d/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets#L64-L76 `Microsoft.NET.RuntimeIdentifierInference.targets` has a lot of MSBuild validation logic, that its main job is to emit nice error messages for incorrect combinations of MSBuild properties/settings. Android is kind of the odd one out when you compare to .NET console apps, NativeAOT, etc.: * We default to `RuntimeIdentifiers=android-arm;android-arm64;android-x86;android-x64`. * We do our own "inner build" for each `RuntimeIdentifier`. * Inside our build we force `$(SelfContained)` to `true` no matter what. As there is no such thing as a system-wide .NET on Android. The fix appears to be to default `PublishSelfContained=false` and let our existing logic force `SelfContained=true`. I added a new test for this scenario. Our existing test didn't catch this because it was passing a `RuntimeIdentifier`.
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.
RuntimeIdentifierNo Longer ImpliesSelfContainedApps by DefaultRIDS no longer imply Self-Contained for TFM >= 8.0
Warns for apps that didn't set SC but set RIDs in TFM < 8 that they need to set SC explicitly to get the same behavior with TFM 8.0. Warning will go away after SC is set, either to true or false.
PublishTrimmed, PublishSingleFile, and PublishAot will keep the old behavior of implying SC with 8+ TFMs, though they will only do so for publish which is a breaking change. PublishReadyToRun will not maintain this behavior and you will need to specify SelfContained explicitly to use it with PublishReadyToRun in TFMs above 7.
Remove the old error that occurred when
-rgiven without-self-containedas it did not catch property in project scenarios, and it's redundant with the breaking change warning. We also have a more understandable default now, (setting one property does not add another that is not necessary.)Update IL Link Tests and Publish* tests that expected the old behavior. Talked with that team, we don't want to add an implicit
SelfContainedthere even if it will break customers as it limits the design potential in the future for these properties.Talked to Blazor WASM who would be broken by this, they have created work items to add
SelfContainedso customers aren't broken. I modified their tests as well to pass by addingSelfContained.This is the current VS behavior and we designed this in a NET 8 breaking changes list. It will help us align with VS behavior.
Tested Behavior:
Questions:
TargetFrameworkInferencesometimes not run? I accounted for that, but don't understand the contexts.