-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Download ILCompiler target pack when cross-compiling #30818
base: main
Are you sure you want to change the base?
Conversation
Two things:
|
if (EnableRuntimePackDownload) | ||
{ | ||
// We need to download the target runtime pack | ||
TaskItem runtimePackToDownload = new TaskItem(targetIlcPackName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know why this condition is not needed for other platforms? -r osx-x64
on osx-arm64 or -r linux-arm64
on linux-x64 etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure it's needed for those as well. It's just that the original reporter happened to run things in order that populated the NuGet cache.
Weirdly I can confirm that works... will report back when I analyze the binlog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually quite a mystery. It's not in _PackageToDownload
but it's restored by the Restore
task right after that. The only input that could be relevant is Microsoft.DotNet.ILCompiler
dependency without a RID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is actually the dependency of the ILCompiler package.
This is the working one:
"net8.0/osx-x64": {
"Microsoft.DotNet.ILCompiler/8.0.0-preview.1.23110.8": {
"type": "package",
"dependencies": {
"runtime.osx-x64.Microsoft.DotNet.ILCompiler": "8.0.0-preview.1.23110.8"
},
"build": {
"build/Microsoft.DotNet.ILCompiler.props": {},
"build/Microsoft.DotNet.ILCompiler.targets": {}
}
},
This is the non-working one:
"net8.0/osx-arm64": {
"Microsoft.DotNet.ILCompiler/8.0.0-preview.1.23110.8": {
"type": "package",
"build": {
"build/Microsoft.DotNet.ILCompiler.props": {},
"build/Microsoft.DotNet.ILCompiler.targets": {}
}
},
The runtime.json file in https://nuget.info/packages/Microsoft.DotNet.ILCompiler/8.0.0-preview.1.23110.8 doesn't have the osx-arm64 entry at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weirdly I can confirm that works... will report back when I analyze the binlog.
Ah, same here, amd64 -> arm64 is restoring. I started testing it in a VM and made a Dockerfile 😅
# on linux host, make sure the environment is ready for cross-arch execution:
# docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
FROM amd64/ubuntu:22.04
RUN apt update && apt install -y curl build-essential clang libicu70 file qemu-user-static binutils-aarch64-linux-gnu debootstrap qemu binfmt-support
RUN curl -SLO https://raw.githubusercontent.com/dotnet/arcade/main/eng/common/cross/build-rootfs.sh
RUN curl --create-dirs --output-dir arm64 -SLO https://raw.githubusercontent.com/dotnet/arcade/main/eng/common/cross/arm64/sources.list.jammy
RUN bash build-rootfs.sh arm64 jammy lldb14 --rootfsdir /crossrootfs/arm64
RUN mkdir ~/.dotnet && curl -sSL https://aka.ms/dotnet/8.0.1xx/daily/dotnet-sdk-linux-x64.tar.gz | tar xzf - -C ~/.dotnet
ENV PATH=${PATH}:/root/.dotnet
RUN dotnet new console -n app1
RUN echo '<configuration><packageSources><add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" /></packageSources></configuration>' > nuget.config
RUN dotnet publish app1 -p:PublishAot=true -o dist -r linux-arm64 -p:SysRoot=/crossrootfs/arm64
ENTRYPOINT ["file", "dist/app1"]
docker build . -t test-arm64
followed by docker run test-arm64
prints:
dist/app1: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=19ec64d2622bbc890e741938f651d1473c0e9e50, for GNU/Linux 3.7.0, with debug_info, not stripped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad to have secondary confirmation for what is happening. Microsoft.DotNet.ILCompiler
has dependency for all the linux*
RIDs on the RID-specific packages so it makes the restore working. osx-arm64
was the only outlier (but we are likely to hit this again with the ios-like platforms soon).
Turns out, it is osx-x64>osx-arm64 specific. See discussion under comment above. |
I agree a test would be nice. Unfortunately I am not very familiar with the dotnet/sdk repository and its test infrastructure. I'm not sure how to ensure such a test would not inadvertently get packages from the NuGet cache. It would be possible to test the output of the MSBuild task but testing the whole end-to-end scenario seems a bit tricky. |
I just tried this locally on a Linux-x64 machine for linux-musl-arm64. The result is:
So my question is: why is that error message not appearing? Because it seems like that would force the presence of that runtime pack. |
Would you happen to have a .binlog? I can compare it to the one I got on my Mac. I don't hit the condition because |
Thanks! That's incredibly weird to me, The IlcHostPackageName construction doesn't sound right. It mixes target OS and host arch. That would explain why it behaves differently when cross-compiling for different OS vs different arch. |
dotnet/runtime#82558 is an alternative fix for the original problem. I suppose this change is not strictly necessary if that PR is merged. However, the reason why the other fix works is a mere side-effect of the Microsoft.DotNet.ILCompiler reference. 🤷🏻♂️ |
Tbh, this code is getting kinda complex and I'm not sure I'm understanding all the nuances. I think we need to come up with a doc or something similar that explains exactly what UX should be when a RID is specified that:
|
The alternative fix to bring osx-arm64 to the same plan as (the existing) {freebsd,linux,win}-arm64 looks great.
Non-portable RIDs are not supported yet. I hope that we come up with a better strategy so non-portable RIDs are not needed for NativeAOT for source-build at all, otherwise it will complicate a lot of things and we will be in the same situation as we are with corehost. |
I am on the same boat. :-)
Agreed, this should be clearly specified.
|
Fixes dotnet/runtime#82542