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

Create Microsoft.DotNet.ILCompiler.LLVM.props #2203

Merged
merged 8 commits into from
Mar 22, 2023

Conversation

kant2002
Copy link
Contributor

Missing bit fo #2196

You would still required to have

<KnownILCompilerPack Update="Microsoft.DotNet.ILCompiler" 
       TargetFramework="net7.0"
       ILCompilerPackNamePattern="runtime.**RID**.Microsoft.DotNet.ILCompiler.LLVM"
       ILCompilerPackVersion="7.0.0-preview.5.23113.1"
       ILCompilerRuntimeIdentifiers="browser-wasm;linux-musl-x64;linux-x64;win-x64;linux-arm;linux-arm64;linux-musl-arm;linux-musl-arm64;osx-arm64;osx-x64;win-arm;win-arm64;win-x86"
       />

@jkotas jkotas added the area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly) label Feb 23, 2023
Copy link

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to understand why the EmitLegacyAssetsFileItems workaround did not work.

Do you have a binlog of the failure?

@kant2002
Copy link
Contributor Author

You can add <PublishAot>true</PublishAot> and you will see an issue.
Process of resolving host/runtime ILC packages depends on the KnownILCompilerPack. If you set PublishAot=true, that’s required to have set IlcHostPackagePath and RuntimePackagePath and point them to proper locations.

if you do not specify <PublishAot>true</PublishAot> then EmitLegacyAssetsFileItems indeed helps.

@SingleAccretion
Copy link

SingleAccretion commented Feb 23, 2023

PublishAot breaks things

I see. In the current state of things, this is "intentional", PublishAot is used as a discriminator for "doing SDK-based publish" versus "doing package-based publish".

Once we merge enough of upstream this will, of course, change.

Process of resolving host/runtime ILC packages depends on the KnownILCompilerPack. If you set PublishAot=true, that’s required to have set IlcHostPackagePath and RuntimePackagePath and point them to proper locations.

This is concerning. Am I correct this means this change doesn't fix the publishing as-is, and you need to override KnownILCompilerPack as well (how would you do that)?

@kant2002
Copy link
Contributor Author

I put snippet in the original comment to PR. That’s all what’s needed.

@SingleAccretion
Copy link

Okay, I will need some time to internalize how the SDK-driven publishing works. The goal would of course be that the user doesn't need to override anything manually.

@kant2002
Copy link
Contributor Author

I hope that my suggestion dotnet/runtime#73678 (comment) would be approved. That’s all what we need from what I see. And we are have default experience “as documented”.

Missing bit fo dotnet#2196

You would still required to have 
```
<KnownILCompilerPack Update="Microsoft.DotNet.ILCompiler" 
       TargetFramework="net7.0"
       ILCompilerPackNamePattern="runtime.**RID**.Microsoft.DotNet.ILCompiler.LLVM"
       ILCompilerPackVersion="7.0.0-preview.5.23113.1"
       ILCompilerRuntimeIdentifiers="browser-wasm;linux-musl-x64;linux-x64;win-x64;linux-arm;linux-arm64;linux-musl-arm;linux-musl-arm64;osx-arm64;osx-x64;win-arm;win-arm64;win-x86"
       />
```
@kant2002
Copy link
Contributor Author

@SingleAccretion I think we can safely merge this, since it works in 2 places 😄

Copy link

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Sorry for the wait; the logic flow here was not straightforward to untangle.

@jkotas jkotas merged commit 050c776 into dotnet:feature/NativeAOT-LLVM Mar 22, 2023
@kant2002 kant2002 deleted the patch-7 branch March 23, 2023 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants