runtime icon indicating copy to clipboard operation
runtime copied to clipboard

[NativeAOT] Clean up AOT Publishing

Open LakshanF opened this issue 3 years ago • 3 comments

Given that building a native AOT application is supported only via the SDK (with the option to get the latest ILCompiler* packs via an explicit reference), the publish path with properties, items, tasks, and targets etc., should be cleaned up. The history of this code has been with specific goals that have changed over time,

  • The original code was added to only support explicit package references
  • SDK support was added for PublishAot with backward compatibility support (for a short period) for the explicit package reference without the SDK path
  • The current support allows SDK only as well as to add explicit package reference with SDK PublishAot. Explicit package reference takes precedence when one exists (for example, to pick up bug fixes in the package earlier)

It should be easy to follow the publishing logic which currently is a little convoluted.

Specifically,

Regarding removing IlcCalledViaPackage, see this comment and https://github.com/dotnet/corert/pull/5123

LakshanF avatar Jul 18 '22 23:07 LakshanF

@LakshanF could you milestone the issue and remove untriaged? It shows up in queries.

MichalStrehovsky avatar Aug 03 '22 09:08 MichalStrehovsky

Incorporating @sbomer suggestions below

The native AOT application publishing experience is outlined below for supported scenarios for PublishAot property and when an explicit package reference to Micsoroft.dotnet.ilcompiler is present

Publishing a native AOT application requires 2 external packages, Micsoroft.dotnet.ilcompiler (hereafter referred as build package) and platform dependent one, runtime.<RID>.microsoft.dotnet.ilcompiler (hereafter referred as runtime package).

The options below take different paths to acquire the packages.

  • The PublishAot property is set to true in the project or elsewhere AND no explicit reference to the build package in the application.
    • The build package that was bundled with the SDK will be used
    • Target ProcessFrameworkReferences will use version information - $(MicrosoftNETCoreAppRuntimePackageVersion), set in GenerateBundledVersions.targets - to download the runtime package
    • AOT Source analyzer will be used during build
    • A telemetry signal will be triggered to indicate an AOT publish action has happened
  • The PublishAot property is set to true in the project or elsewhere AND an explicit reference to the build package is in the application.
    • This scenario will generate a warning but is supported to let developers get a specific version of the ILCompiler package. For example, to get a bug fix that is not yet present in the SDK ILCompiler package versions.
    • The build package that was restored by NuGet will be used
    • Target ProcessFrameworkReferences will use the same version as the build package version information to download the runtime package
    • AOT Source analyzer will be used during build
    • A telemetry signal will be triggered to indicate an AOT publish action has happened
  • PublishAot property is not set AND an explicit reference to the build package is in the application
    • This scenario is not supported to publish a native AOT application
    • No warnings will be generated and a 'normal' publish will happen
  • Cross target publishing support as described here
    • PublishAOT property setting as described above
    • Target ProcessFrameworkReferences will use the same version as the build package version information to also download a runtime package that matches the target
  • PublishAot property is set to false
    • No AOT related publishing will be done
  • Build process
    • Build should be successful, but no post IL processing will be done
    • AOT Source analyzer will be used during build process

These scenarios impose the following requirements

  • Distinguish build package loaded location. Specifically, the build package loaded location to be either via SDK or via NuGet (via an explicit reference) needs to be known.
  • SDK to know the version of the build package that is getting used. This information is needed so as to download the same version runtime packages.
  • Build and runtime packages to know the final root path information of the runtime package that is used. The root path is used to set the full path to the native AOT tools.

These requirements are currently implemented in the following way

  • Set a sentinel to the directory where the package is by the first load. Use that package location for loading all related package assets.
  • Build package assets to detect if the build package is from NuGet and if so, to pass the version information back to SDK prior to the task where SDK downloads runtime packages.
  • SDK will pass to the build package the root path of the runtime package that will be used

Runtime repo impact The above plan requires taking into account that the package targets will be imported in dotnet build. Specifically, no publish related AOT targets will be invoked during the build process. In addition, Building and running native AOT tests in the runtime repo will continue to work as before. The following repo assets are impacted,

  • tests.singlefile.targets: This will trigger importing Microsoft.NETCore.Native.targets.
  • BuildFrameworkNativeObjects.proj: Seems to control the import order of the AOT targets based on a property, IlcCalledViaPackage, where the property determines to use the runtime repo assets as opposed to the SDK assets. We would like to remove (see above comment details) this property in a future change
  • crossgen2.csproj: Imports the AOT targets based on some property status.
  • Assets that do not use PublishAOT and rely on the SDK: They should not be impacted.
  • #73416 and #74046 tracks the runtime changes.

SDK repo impact

  • The above plan also needs to work with running existing AOT tests in the SDK repo. Specifically, the SDK needs to support downloading cross-target runtime packages and passing that information to runtime targets.
  • Currently, the PublishAOT package assets are not hardened to work if any non-NativeAOT assets access them (unlike the trimming assets).
  • https://github.com/dotnet/sdk/pull/27159 tracks these changes.

LakshanF avatar Aug 04 '22 17:08 LakshanF

I think we should simplify the third scenario to behave either identically to the second (as if PublishAot were set to true), or to not do an AOT publish at all. Since the OOB package scenario is already in use, I would probably make it behave like PublishAot is true.

Setting PublishAot property to false is not supported

I think that setting PublishAot to false should prevent AOT publish, turn off the AOT analyzer, etc, even if there is a package reference.

sbomer avatar Aug 04 '22 17:08 sbomer

The PR is getting reverted.

MichalStrehovsky avatar Aug 17 '22 01:08 MichalStrehovsky

@LakshanF you still had some concerns around this in the meeting. Should we keep open or it's good to close?

MichalStrehovsky avatar Aug 17 '22 23:08 MichalStrehovsky

I would like to keep this open for a little longer. I need to clean up the tests in SDK and that requires the runtime changes to flow there. Will do that soon.

LakshanF avatar Aug 18 '22 12:08 LakshanF

Validated the test in the rc1 branch, including cross-target scenario without the explicit packages in the project. However, the test will only be updated on the main branch and not on the RC1 branch due to the high bar for changes in the RC1

LakshanF avatar Aug 22 '22 14:08 LakshanF