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

[NativeAOT] Extend the runtime pack approach to all supported platforms #87060

Open
Tracked by #80905
ivanpovazan opened this issue Jun 2, 2023 · 12 comments
Open
Tracked by #80905

Comments

@ivanpovazan
Copy link
Member

Description

Currently when targeting iOS-like platforms with NativeAOT, the build process will also download the host's runtime package (osx), even though it is unused.
Apart from that, workloads that want to integrate with NativeAOT need to take into account the opt-in property: PublishAotUsingRuntimePack introduced here: dotnet/sdk#32100

To improve the cross-compilation and integration experience further, all host/target platforms should be supported in the same fashion producing two nuget packages:

  • [host] Microsoft.DotNet.ILCompiler - carrying only the tools and build integration targets
  • [target] Microsoft.NETCore.App.Runtime.NativeAOT.<os>-<arch> - carrying target runtime and framework libraries
    The need for PublishAotUsingRuntimePack would become redundant.

This should be an extension to: #85047 where NativeAOT runtime packs are initially supported only for iOS/tvOS/Catalyst target platforms.

Note

This should also simplify and improve the way packages are built.
For example, in order to build all the required packages to build an ios application with NativeAOT:

  1. The following subsets are required to build Microsoft.DotNet.ILCompiler package for the host - osx
./build.sh clr+clr.aot+libs+packs
  1. The following subsets are required Microsoft.NETCore.App.Runtime.NativeAOT.ios-arm64 package for the target - ios:
./build.sh clr.nativeaotlibs+clr.nativeaotruntime+libs+packs /p:BuildNativeAOTRuntimePack=true

which involves long build times and building unnecessary components.

@ivanpovazan ivanpovazan added this to the 8.0.0 milestone Jun 2, 2023
@ghost
Copy link

ghost commented Jun 2, 2023

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Currently when targeting iOS-like platforms with NativeAOT, the build process will also download the host's runtime package (osx), even though it is unused.
Apart from that, workloads that want to integrate with NativeAOT need to take into account the opt-in property: PublishAotUsingRuntimePack introduced here: dotnet/sdk#32100

To improve the cross-compilation and integration experience further, all host/target platforms should be supported in the same fashion producing two nuget packages:

  • [host] Microsoft.DotNet.ILCompiler - carrying only the tools and build integration targets
  • [target] Microsoft.NETCore.App.Runtime.NativeAOT.<os>-<arch> - carrying target runtime and framework libraries
    The need for PublishAotUsingRuntimePack would become redundant.

This should be an extension to: #85047 where NativeAOT runtime packs are initially supported only for iOS/tvOS/Catalyst target platforms.

Note

This should also simplify and improve the way packages are built.
For example, in order to build all the required packages to build an ios application with NativeAOT:

  1. The following subsets are required to build Microsoft.DotNet.ILCompiler package for the host - osx
./build.sh clr+clr.aot+libs+packs
  1. The following subsets are required Microsoft.NETCore.App.Runtime.NativeAOT.ios-arm64 package for the target - ios:
./build.sh clr.nativeaotlibs+clr.nativeaotruntime+libs+packs /p:BuildNativeAOTRuntimePack=true

which involves long build times and building unnecessary components.

Author: ivanpovazan
Assignees: -
Labels:

os-ios, area-NativeAOT-coreclr

Milestone: 8.0.0

@MichalStrehovsky
Copy link
Member

@akoeplinger do you think we can close #67080 in favor of this issue? It looks like they're both about the same thing, but this one has a more clear implementation approach.

@filipnavara
Copy link
Member

filipnavara commented Jan 8, 2024

The last thing that's left is unconditionally enabling PublishAotUsingRuntimePack in for net9.0 TFMs in the SDK. We can remove the SDK/frameworks from the ILCompiler packages then.

@MichalStrehovsky
Copy link
Member

The last thing that's left is unconditionally enabling PublishAotUsingRuntimePack in for net9.0 TFMs in the SDK. We can remove the SDK/frameworks from the ILCompiler packages then.

One thing I would like to understand is how does it affect debugging. In the existing scheme, the PDBs (both for managed code and for the native parts on Windows) are part of the shipping package. In the new scheme, I see that they're extracted to a separate .Symbols package. Can ILC/link.exe see those?

@ivanpovazan
Copy link
Member Author

The last thing that's left is unconditionally enabling PublishAotUsingRuntimePack in for net9.0 TFMs in the SDK. We can remove the SDK/frameworks from the ILCompiler packages then.

Just for the clarification, did you mean that we would remove the said property?

@filipnavara
Copy link
Member

did you mean that we would remove the said property?

I am not sure how the SDK versioning works, so we may need to keep it to allow building net8.0-XXX TFM with net9 SDK.

@ivanpovazan
Copy link
Member Author

I might be wrong, but I think we are using the currently installed SDK's version of Microsoft.NET.Build.Tasks.dll even when targeting older framework versions. So I think removing the references PublishAotUsingRuntimePack introduced in dotnet/sdk#32100 would be good enough. /cc: @akoeplinger

@akoeplinger
Copy link
Member

One thing I would like to understand is how does it affect debugging. In the existing scheme, the PDBs (both for managed code and for the native parts on Windows) are part of the shipping package. In the new scheme, I see that they're extracted to a separate .Symbols package. Can ILC/link.exe see those?

@MichalStrehovsky it should work the same way as when publishing a self-contained CoreCLR app. The symbols are ingested into the Microsoft Symbol Server during the official build / release. When debugging Visual Studio downloads them from there or you can use the dotnet-symbol tool to manually download them. Or do you need the symbols during ILC time?

So I think removing the references PublishAotUsingRuntimePack introduced in dotnet/sdk#32100 would be good enough

Kind of, we'd need to keep the property to opt in a .NET 8 app when building from a 9 SDK. I think it is easier if we just default it to true for TF >= 9

@MichalStrehovsky
Copy link
Member

Or do you need the symbols during ILC time?

Yes, that. ILC picks up managed debug info and produces native debug info out of that. This gets passed to the platform linker that produces the final debug info. On Windows, we also need the pdb for obj/lib files (they are separate from object files on Windows).

@akoeplinger
Copy link
Member

Ok, looks like there's an option called IncludeSymbolsInPackage which Microsoft.DotNet.ILCompiler.pkgproj already sets so we just need to set that on the runtime pack project.

@akoeplinger
Copy link
Member

Turned out that IncludeSymbolsInPackage only works for .pkgproj, but the runtime packs use .sfxproj so I found another solution: #96675

akoeplinger added a commit that referenced this issue Jan 9, 2024
* Include symbols in main package for NativeAOT runtime packs

See #87060 (comment)

* PR feedback

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

---------

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
tmds pushed a commit to tmds/runtime that referenced this issue Jan 23, 2024
…6675)

* Include symbols in main package for NativeAOT runtime packs

See dotnet#87060 (comment)

* PR feedback

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

---------

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
@MichalStrehovsky
Copy link
Member

Can't really do much about this due to reasons discussed in dotnet/sdk#37872 (comment).

We can't make progress on this until running the Publish target without also setting _IsPublishing to true doesn't become an error. I'm moving this to .NET 10. VS should soon start setting _IsPublishing when doing a publish from the UI so maybe it becomes more feasible then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

5 participants