Skip to content
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

Publishing as NativeAOT should not depend on AppHost #79575

Closed
MichalStrehovsky opened this issue Dec 13, 2022 · 5 comments · Fixed by #101270
Closed

Publishing as NativeAOT should not depend on AppHost #79575

MichalStrehovsky opened this issue Dec 13, 2022 · 5 comments · Fixed by #101270
Labels
area-NativeAOT-coreclr in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@MichalStrehovsky
Copy link
Member

We currently need to build apphost because for some reason PublishAot ends up needing it in some of our test legs:

buildArgs: -s clr.runtime+clr.aot+host.native+libs -rc $(_BuildConfig) -lc Release -hc Release

Without this line, the CI fails with:

Could not copy the file "/__w/1/s/artifacts/bin/trimmingTests/projects/System.Diagnostics.DiagnosticSource.NativeAotTests/DiagnosticSourceEventSourceTests/linux-x64/obj/Release/net8.0/linux-x64/apphost" because it was not found.

This is probably not just a test issue, but also a product issue. Historically the NativeAOT targets were just an addon to the SDK targets that hacked into the SDK (we e.g. delete outputs the SDK produces in publish and replace them with NativeAOTd stuff).

The SDK probably needs to become more aware of what NativeAOT does and suppress some behaviors.

@ghost
Copy link

ghost commented Dec 13, 2022

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

We currently need to build apphost because for some reason PublishAot ends up needing it in some of our test legs:

buildArgs: -s clr.runtime+clr.aot+host.native+libs -rc $(_BuildConfig) -lc Release -hc Release

Without this line, the CI fails with:

Could not copy the file "/__w/1/s/artifacts/bin/trimmingTests/projects/System.Diagnostics.DiagnosticSource.NativeAotTests/DiagnosticSourceEventSourceTests/linux-x64/obj/Release/net8.0/linux-x64/apphost" because it was not found.

This is probably not just a test issue, but also a product issue. Historically the NativeAOT targets were just an addon to the SDK targets that hacked into the SDK (we e.g. delete outputs the SDK produces in publish and replace them with NativeAOTd stuff).

The SDK probably needs to become more aware of what NativeAOT does and suppress some behaviors.

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: 8.0.0

@ViktorHofer
Copy link
Member

That error can be avoided by setting <UseAppHost>false</UseAppHost> but that will then raise the next error:

C:\Program Files\dotnet\sdk\7.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(186
,5): error NETSDK1067: Self-contained applications are required to use the application host. Either set SelfContained t
o false or set UseAppHost to true. [C:\temp\useaphost\useaphost.csproj]

@MichalStrehovsky
Copy link
Member Author

Yeah, the SDK needs to be taught that PublishAot is sort of like self contained, but it doesn't need an apphost. I probably should have opened this in the SDK repo because the main fix will likely be in the SDK, but SDK repo is where issues typically go die of neglect and loneliness.

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 13, 2022

I would imagine that with the following modifications in the SDK, we should be able to publish without the apphost requirement:

@jkotas
Copy link
Member

jkotas commented Dec 13, 2022

Make UseAppHost default to false when '$(PublishAot)' == 'true'

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.

@agocke agocke added this to AppModel Mar 6, 2023
MichalStrehovsky added a commit to dotnet/sdk that referenced this issue Jun 14, 2023
Fixes dotnet/runtime#79575.

AppHost 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 files we don't need during publish.
@MichalStrehovsky MichalStrehovsky modified the milestones: 8.0.0, 9.0.0 Jul 31, 2023
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Apr 18, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Apr 18, 2024
matouskozak pushed a commit to matouskozak/runtime that referenced this issue Apr 30, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 20, 2024
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this issue May 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr in-pr There is an active PR which will close this issue when it is merged
Projects
Archived in project
3 participants