-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use SingleFileHost #11797
Use SingleFileHost #11797
Conversation
71ffa71
to
37e9d2f
Compare
This commit implements the following changes for single-file apps: * When publishing self-contained single-file apps: * Use `SingleFileHost` instead of `apphost` * Trim the native components of the runtime published for the app. * Fixes dotnet#11567. * Implements the [optional additional settings](https://github.com/dotnet/designs/blob/master/accepted/2020/single-file/design.md#optional-settings) for .net 5 `PublishSingleFile`. * dotnet/runtime#36590
@@ -2,18 +2,18 @@ | |||
|
|||
<PropertyGroup> | |||
<OutputType>Exe</OutputType> | |||
<TargetFramework>netcoreapp3.0</TargetFramework> | |||
<TargetFramework>netcoreapp5.0</TargetFramework> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preferrably this should use net5.0
for the TargetFramework.
IsSelfContained ? | ||
IsSingleFile ? RuntimePackAssets.Where(item => !item.GetMetadata(MetadataKeys.DropFromSingleFile).Equals("true")) | ||
.Select(item => RuntimePackAssetInfo.FromItem(item)) : | ||
RuntimePackAssets.Select(item => RuntimePackAssetInfo.FromItem(item)) : | ||
Enumerable.Empty<RuntimePackAssetInfo>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the following to avoid repeating the same Select
clause:
IsSelfContained ? | |
IsSingleFile ? RuntimePackAssets.Where(item => !item.GetMetadata(MetadataKeys.DropFromSingleFile).Equals("true")) | |
.Select(item => RuntimePackAssetInfo.FromItem(item)) : | |
RuntimePackAssets.Select(item => RuntimePackAssetInfo.FromItem(item)) : | |
Enumerable.Empty<RuntimePackAssetInfo>(); | |
IsSelfContained ? | |
RuntimePackAssets.Where(item => IsSingleFile ? !item.GetMetadata(MetadataKeys.DropFromSingleFile).Equals("true") : true) | |
.Select(item => RuntimePackAssetInfo.FromItem(item)) : | |
Enumerable.Empty<RuntimePackAssetInfo>(); |
<_IsSingleFilePublish Condition="'$(PublishSingleFile)' == ''">false</_IsSingleFilePublish> | ||
<_IsSingleFilePublish Condition="'$(PublishSingleFile)' != ''">$(PublishSingleFile)</_IsSingleFilePublish> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for having a separate _IsSingleFilePublish
property rather than using PublishSingleFile
directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PublishSingleFile
property is captured by telemetry, which reports the three states: true
, false
, and unset
. So, I wanted to leave the user input in its pure state and use a different variable.
Thanks for the review, @dsplaisted. I have committed the changes you've suggested. |
The Linux/Osx failures are because |
So once that is fixed then you should be able to update stage 0 for this PR and merge it? |
Yes that is my plan -- would like to have it in Preview 6.
|
(To include update from runtime)
Updating stage0 caused crossgen2 tests to fail because of missing dependencies in the corssgen2 package. dotnet/runtime#37196 Therefore, try disabling the tests temporarily.
@dsplaisted Updating stage0 triggered crossgen2 failure: dotnet/runtime#37196. I think it is a good idea to disable these tests temporarily, rather than holding the global.json update (and this PR) until dotnet/runtime#37196 is fixed. CC: @trylek. |
Sounds good. We don't have a great process to track re-enabling tests like this. Will you remember to re-enable these? We could also file an issue in the sdk repo to track re-enabling the tests when the bug is fixed. |
OK @dsplaisted, I filed #11845 to re-enable the tests once the underlying issue is fixed. |
This commit implements the following changes for single-file apps:
SingleFileHost
instead ofapphost
PublishSingleFile
.