-
Notifications
You must be signed in to change notification settings - Fork 10.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
Support building for arm64 on arm64 (*nix) #15354
Conversation
Thanks for the PR @omajid. Have you looked at the build failures introduced by your change? |
No, I hadn't. Looking at them now. |
25226b1
to
c5fe5c7
Compare
@pranavkm I fixed the errors that I could spot. I am not sure what this new Helix ARM64 failure is.
That isn't anything my patch touches... is this a known issue? |
This comment was made automatically. If there is a problem contact aspnetcore-build@microsoft.com. I've triaged the above build. I've created/commented on the following issue(s) |
@pranavkm I don't have access to aspnet/AspNetCore-Internal. Please let me know if anything in that issues requires me to do something here. |
Ping. Anyone willing to review this? |
@dleeapho Hey, do you know someone who can help shepherd this change through? |
Ping. Anyone willing to review this? |
Hey @leecow, any suggestions on who I can poke to get this PR reviewed? It's part of the patches we are trying to add to source-build for arm64 support: dotnet/source-build#1300 |
Hi @JunTaoLuo, could you or someone else take a look at this? Omair has this patched into source-build right now so I think it should be pretty close. |
This approach is different from what we have been doing so far. We specify --arch explicitly in most cases: https://github.com/aspnet/AspNetCore/blob/master/.azure/pipelines/ci.yml#L345. The reason for this is that we would be able to build ARM on a non-ARM machine for example. Is it not possible to do so in source build? The changes in Microsoft.AspNetCore.App.Runtime.csproj look good but I'm unsure we need the other changes. |
Hi @JunTaoLuo ! Thanks for the review!
I think the approach here isn't so different as you might think at first. Before this change, a developer can build AspNetCore for their local architecture (eg, build for x64 for x64) by using just This change simply extends this concept. It now makes the the build system put arm64 in the same category as x64: a developer can simply run As far as I can tell, we are not changing what happens on cross compilation: a user still needs to pass
The goal of source-build is to eventually package up and include .NET Core (including ASP.NET Core) into various Linux - and other *nix - distributions. They generally support many, many more platforms than .NET Core currently does. And it's more common to build on the same architecture as the target architecture rather than cross compiling. They build for ppc64le on ppc64le, for example, instead of cross compiling for ppc64le on x64. In that context, adding special cases for every architecture and computing/passing the right |
My understanding was shaped by https://github.com/aspnet/AspNetCore/blob/master/docs/BuildFromSource.md#build-properties which suggests that we let the user (or build script) explicitly specify what to build for. That being said, my initial concerns are abated because:
Although the changes look correct, I'm still not convinced it's difficult to specify these variables as part of the build script by the user. Which other platforms are we planning to support in 5.0? |
Thanks for this pointer. I wasn't aware of this position. I still think my change is a better idea because does the right thing to correctly build ASP.NET Core on arm64, even outside of source-build. cc @crummel @dleeapho @dseefeld. Any thoughts on what the right approach here might be? Is it expected to pass
If it helps, I have been testing 3.0 with this change 😄 . All my normal end-to-end tests with the SDK pass on arm64. I think the plan is to carry this change as a local patch in 3.1 for source-build.
I would hope there wouldn't be any change here. It only changes the behaviour when you are building on an arm64 box. The CI machines are x64 and so this change should be functionally no-op (aside from any bugs I introduced).
I think I agree. It's not difficult, just more error prone and a point of divergence from x64 where
I have no plans for any other platforms whatsoever. I just want to keep things flexible where possible. |
This is generally our preference. Source-build tries not to rely on defaults because we aren't in control of when they change (this is less of a problem now that most repos are on Arcade and have the same defaults). |
@JunTaoLuo Any thoughts? What would it take to get this to be mergable? Would you like me to drop all changes to |
It looks like the preference for source build is to specify these explicitly so I think we should not be taking the changes in builds.sh and TargetArchitecture. We can merge the Microsoft.AspNetCore.App.Runtime changes though. |
c5fe5c7
to
27ea93b
Compare
This commit allows ASP.NET Core to be built on arm64 machines directly, without relying on cross-compilation. There's a few changes in here: 1. Ask msbuild to look into the BuildArchitecture By default, our build systems assums the machine is x64. This modifies the build configuration to check the architecture of the currently running build machine, and set BuildArchitecture to that. 2. Fix crossgen in Microsoft.AspNetCore.App.Runtime We run crossgen for supported architectures (including x64 and arm64). For that, we need a jit that we can point crossgen to. Generally, we can rely on the build scripts to find the right `libclrjit.so`. However, arm64 has multiple `libclirjit.so`, for different use-cases. There's one for arm64 (for running on arm64) and there's another one for cross-compiling for arm64 on x64. We need to figure out and use the right one explicitly rather than assuming the right one gets picked up. See dotnet/core-setup#8468 for similar changes made in core-setup. This also needs dotnet#14790 to fully work on arm64.
27ea93b
to
e2946a2
Compare
@JunTaoLuo Please let me know if this is okay to merge. |
CI looks good to me; "Pull Request Labeller" failed, but I guess that isn't about the code at all. |
Thanks @omajid ! |
Thanks for reviewing and merging this! |
@crummel Just as an FYI: source-build invokes the ASP.NET Core build with
|
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453
This chnage allows source-build to be cloned and built on arm64 machines. It shouldn't affect cross compilation at all, however. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453 To support using darc on arm64, this commit removes the customizations of LD_LIBRARY_PATH and instead forces darc to run again a .NET Core 3.0 runtime. This in-turn makes darc pick up the right libgit2sharp automatically for the right set of platforms and architectures. There's a number of existing build configuration that are conditionalized on arm64, such as setting up a root file system. Those they are actually only meant to be invoked when cross-compiling for arm64 (on x86_64). This commit modifies those conditions to not apply when building on an arm64 machine.
This change allows source-build to be cloned and built on arm64 machines. It shouldn't affect cross compilation at all, however. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453 There's a number of existing build configuration that are conditionalized on arm64, such as setting up a root file system. Those they are actually only meant to be invoked when cross-compiling for arm64 (on x86_64). This commit modifies those conditions to not apply when building on an arm64 machine.
This change allows source-build to be cloned and built on arm64 machines. It shouldn't affect cross compilation at all, however. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453 There's a number of existing build configuration that are conditionalized on arm64, such as setting up a root file system. Those they are actually only meant to be invoked when cross-compiling for arm64 (on x86_64). This commit modifies those conditions to not apply when building on an arm64 machine.
This change allows source-build to be cloned and built on arm64 machines. It shouldn't affect cross compilation at all, however. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453 There's a number of existing build configuration that are conditionalized on arm64, such as setting up a root file system. Those they are actually only meant to be invoked when cross-compiling for arm64 (on x86_64). This commit modifies those conditions to not apply when building on an arm64 machine.
* Enable building on arm64 This change allows source-build to be cloned and built on arm64 machines. It shouldn't affect cross compilation at all, however. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453 There's a number of existing build configuration that are conditionalized on arm64, such as setting up a root file system. Those they are actually only meant to be invoked when cross-compiling for arm64 (on x86_64). This commit modifies those conditions to not apply when building on an arm64 machine. * Patch fixup.
* Enable building on arm64 This change allows source-build to be cloned and built on arm64 machines. It shouldn't affect cross compilation at all, however. I have tested this on RHEL 8 aarch64. This change includes backports of the following pull-requests which have been merged into their respective master branches: - dotnet/aspnetcore#14790 - dotnet/aspnetcore#15354 - dotnet/installer#4102 - dotnet/core-setup#8468 - dotnet/corefx#40453 There's a number of existing build configuration that are conditionalized on arm64, such as setting up a root file system. Those they are actually only meant to be invoked when cross-compiling for arm64 (on x86_64). This commit modifies those conditions to not apply when building on an arm64 machine. * Patch fixup.
This commit allows ASP.NET Core to be built on arm64 machines directly, without relying on cross-compilation.
There's a few changes in here:
Extendbuild.sh
to handle non-x64 architectures.By default, the build assumes the machine is x64. Instead query the machine (usinguname
) to find out what the actual build machine is.A similar change was made in arcade: Handle non-x86 in dotnet-install.sh on *nix arcade#4132Ask msbuild to handle the BuildArchitecture
By default, our build systems assums the machine is x64. This modifies the build configuration to check the architecture of the currently running build machine, and set BuildArchitecture to that.
Ideally, we could just useProcessorArchitecture
on all cases and avoid any hardcoding ofx64
, but that would break Visual Studio. Visual Studio runs msbuild as a x86 process and we need to explicitly tell msbuild to target x64 in those cases.We ran into a similar issue in corefx: Revert "Enable build on hosted arm64" corefx#40427The final fix was: Enable build on hosted arm64 corefx#40453Fix crossgen in Microsoft.AspNetCore.App.Runtime
We run crossgen for supported architectures (including x64 and arm64). For that, we need a jit that we can point crossgen to. Generally, we can rely on the build scripts to find the right
libclrjit.so
. However, arm64 has multiplelibclirjit.so
, for different use-cases. There's one for arm64 (for running on arm64) and there's another one for cross-compiling for arm64 on x64. We need to figure out and use the right one explicitly rather than assuming the right one gets picked up.See Enable building for arm64 on arm64 core-setup#8468 for similar changes made in core-setup.
This also needs #14790 to fully work on arm64.