-
Notifications
You must be signed in to change notification settings - Fork 446
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
Add win-x86 to known ILCompiler and NativeAOT runtime packs #19166
Conversation
If I'm not mistaken, this change has the potential to break our own repositories that list |
I wasn't sure about that either that's why I ended up not merging yet. In the past we always started producing these first. |
(However, Net90NativeAOTRuntimePackRids does contain the wasi/wasm packs that are only produced in a separate nuget feed from the runtimelab repo and that one doesn't cause problems.) |
Interesting, I thought that we never ship artifacts from runtimelab. Did that change? To which feed do we publish runtimelab artifacts?
Probably because it only affects projects that specify a RuntimeIdentifier. Maybe none of our projects specify the wasi/wasm RID and enable NativeAOT? |
We've been doing that single the runtimelab beginnings - they should go to the dotnet-experimental feed.
Mmm, I did a small experiment with a console hello world app that has This was discussed in #18121 (comment) but looks like this aspect was missed. Cc @akoeplinger |
It’s fine to wait for the official packs to be built first. There’s a whole chain of PRs that is currently blocked on a 3-line change in the JIT but I decided to preemptively open all the PRs to avoid any further delays once that one issue/PR is resolved. This should not really break anything, as stated in some of the previous comments about WASI/WASM. The failure mode for KnownRuntimePacks is already catastrophic as it is. If there’s a RID listed without a corresponding NuGet you will get an error about resolving NuGet package which is somewhat searchable/actionable. It only happens when trying to publish for the newly added RID. Conversely, if a RID is missing in the list, the runtime pack resolution doesn’t produce an error but it also doesn’t populate ItemGroups and properties that are expected by the NativeAOT publishing. The end result is that you get cryptic error that must be traced back through BinLogs. |
Scratch that, it fails with this error even if I drop PublishAot and add |
Right, but I was surprised that we add support for artifacts produced by runtimelab into the SDK (in this case known RID items). cc @jkotas |
The SDK doesn't care where the runtime packs come from, e.g. we also have linux-s390x or linux-loongarch64 in the lists even though we don't build them on the Microsoft side. Having them in the list just makes it much easier for third parties to provide them. We can wait with merging this until we have at least one 9.0 preview version on nuget.org with the win-x86 package to be on the safe side. |
I see the hardcoded RID lists in the SDK as a broken design. Ideally, the SDK would not care about the RID that you are publishing for. It would just take the RID and try to download the package for it.
+1 |
We now have packages but they are non-functional. Fix is in dotnet/runtime#100512. |
Validated we now produce good packages and the update got ingested into installer. Thank you for your work on this Filip! |
Related to dotnet/runtime#86573