Use better AOT defaults in Web SDK #29984
Conversation
halter73
requested review from
DamianEdwards,
JamesNK,
mitchdenny and
eerhardt
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
|
|
||
| var buildProperties = testProject.GetPropertyValues(testAsset.TestRoot, targetFramework); | ||
| buildProperties["PublishTrimmed"].Should().Be("true"); | ||
| buildProperties["TrimMode"].Should().Be(""); |
There was a problem hiding this comment.
This verifies we don't set TrimMode to "partial" like we do in the PublishTrimmed-only case. Does anyone know of a better way to verify we get "full" trimming?
| @@ -87,7 +88,8 @@ Copyright (C) Microsoft Corporation. All rights reserved. | |||
| <EFSQLScriptsFolderName Condition="$(EFSQLScriptsFolderName) == ''">EFSQLScripts</EFSQLScriptsFolderName> | |||
|
|
|||
| <!-- If TrimMode has not already been set, default to partial trimming, as AspNet is not currently fully trim-compatible --> | |||
| <TrimMode Condition="'$(PublishTrimmed)' == 'true' and '$(TrimMode)' == ''">partial</TrimMode> | |||
| <!-- We allow the full trimming default with PublishAot enabled, because the parts that are not trim-compatible are not aot-compatible either --> | |||
| <TrimMode Condition="'$(TrimMode)' == '' and '$(PublishTrimmed)' == 'true' and '$(PublishAot)' != 'true'">partial</TrimMode> | |||
There was a problem hiding this comment.
We should think about defaulting to full with PublishTrimmed true as well
There was a problem hiding this comment.
yeah this came up in the original issue and discussion but it seems a bit premature right now given the state of trimmability across all of ASP.NET Core. Not ruling it out for .NET 8 though if we think we get far enough to make it the better default.
There was a problem hiding this comment.
Can we track it in an issue and punt it to the next release if we decide?
There was a problem hiding this comment.
I reopened #30059 and put it in the Goldilocks project for tracking.
This PR provides better defaults for the TrimMode and PublishIISAssets properties when the PublishAot property is set to true.
Once this is merged, we should be able to follow up by removing these properties from the AOT version of the new "api" project template.
Fixes #30059
Fixes #29896