Don't set UseApphost or SelfContained to true for PublishAot

[MichalStrehovsky]

Fixes dotnet/runtime#79575.

UseAppHost doesn't make any sense for PublishAot, and neither do the behaviors around SelfContained - if SelfContained is true, the bin directory gets populated with a ton of garbage files we don't need during publish, and the publish process starts to have opinions about whether UseAppHost=true should be specified.

Cc @dotnet/ilc-contrib @ViktorHofer

[vitek-karas]

Technically we should set PublishSelfContained=true as the app will be self-contained and some things might change behavior based on that. But that setting is also broken :-(
I just wonder if setting it to false will break something somewhere because it will now think we're publishing an FDD app.

[MichalStrehovsky]

The problem is that we're not doing either - I couldn't find a single behavior of SelfContained=true in the SDK that looks desirable for PublishAot. I don't think FDD behaviors are desirable either but I don't see any of them getting in the way the way SelfContained is so FDD just looks like a path of least resistance. The nice side effect of setting SelfContained to false is that publish is becoming faster because we no longer copy 100 files to the bin directory that Defender needs to scan.

[ivanpovazan]

@rolfbjarne @steveisok @kotlarmilos just a heads-up

[ViktorHofer]

Just making sure that you saw @jkotas comment in the tracking issue: dotnet/runtime#79575 (comment)

We do want to keep using app host for dotnet run or F5 in Visual Studio. I do not think we want to change the global default to false when PublishAot is set.

I guess with your change the dotnet run and F5 scenario isn't impacted as they don't use the publishing output?

[MichalStrehovsky]

Just making sure that you saw @jkotas comment in the tracking issue: dotnet/runtime#79575 (comment)

We do want to keep using app host for dotnet run or F5 in Visual Studio. I do not think we want to change the global default to false when PublishAot is set.

That's what _IsPublishing is supposed to guard against.

[steveisok]

I understand the reason for the change, but nativeaot is self contained. Are we doing something out of order in the build?

[agocke]

• @dsplaisted @nagilson

[MichalStrehovsky]

I understand the reason for the change, but nativeaot is self contained. Are we doing something out of order in the build?

The problem is what SelfContained does in the Sdk - and we don't want pretty much any of those behaviors.

I still want to explore an alternative approach (allow SelfContained=true and just suppress the useless behaviors. If so just passing --self-contained to publish with aot doesn't error out. I just didn't have time to come back to this yet.

[nagilson]

_IsPublishing will never activate in VS and only when doing dotnet publish in the SDK, is this the behavior you had expected?

cc @richlander for heads up of this PR to revert SelfContained implication for PublishAot

[MichalStrehovsky]

The problem this is trying to fix is that if someone takes a project and just wants to publish it as native aot, we'll end up downloading apphost and downloading the entire runtime pack, copy all of that into bin (as part of Build) and then do the actual native aot publish that doesn't need any of this because we don't publish the result of Build. So we just copy around a 100 MB of garbage.

It is wasteful but it wouldn't be the first wasteful thing the SDK is doing. Which is also why I didn't get back to this yet because maybe it doesn't move the needle enough.

I am ambivalent to fixing this. The current behavior makes publish slower and less efficient, but a lot of things in the SDK are already slow and inefficient. I'm honestly leaning towards just closing this and won't fixing the issue, especially given #33229 (comment) it's not possible for this optimization to kick in within VS.

[richlander]

Seems like a good direction to me.

AppHost and SelfContained are CoreCLR concepts.

There is an element of similarity with this issue: #34198

[akoeplinger]

I think we're currently relying on the SelfContained publish behavior regarding runtime packs in the NativeAOT iOS scenario. I do agree that a lot of this is wasteful but I'd be more comfortable if we look into this for .NET 9.

[richlander]

The SDK has a different bar. A middle ground is to defer it to 8.0.200.

[akoeplinger]

I'm pretty sure it'd need corresponding workload changes in dotnet/runtime though.

[richlander]

Got it. Then, waiting for next release is likely the best choice.

[ghost]

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

[MichalStrehovsky]

I now learned some new things while investigating dotnet/runtime#95496 and I no longer want to pursue this.