Enable building on arm64 #1300
Merged
Enable building on arm64 #1300
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omajid
commented
Oct 9, 2019
omajid
commented
Oct 9, 2019
omajid
commented
Oct 9, 2019
omajid
commented
Oct 9, 2019
omajid
commented
Oct 9, 2019
This was referenced Oct 9, 2019
omajid
force-pushed
the
build-on-rhel8-arm64
branch
from
14c2e94
to
d222dd0
Compare
omajid
commented
Nov 7, 2019
| buildArch=x64 | ||
| ;; | ||
| esac | ||
|
|
There was a problem hiding this comment.
I got this chunk from here: dotnet/arcade#4132
omajid
commented
Nov 7, 2019
|
@adaggarwal is working on merging 3.0 to 3.1 (#1355), after that 3.1 will be our primary development branch. I think this is okay to stay against release/3.0, after 3.0.1 (#1260) is in we'll take this and add it to the 3.1 merge - if that sounds good to you Aditya? |
Yep. That sounds reasonable. |
omajid
force-pushed
the
build-on-rhel8-arm64
branch
from
d222dd0
to
a8df810
Compare
omajid
commented
Nov 12, 2019
| @@ -11,6 +11,8 @@ | |||
| <BuildCommandArgs>$(BuildCommandArgs) --configuration $(Configuration)</BuildCommandArgs> | |||
| <BuildCommandArgs>$(BuildCommandArgs) --ci</BuildCommandArgs> | |||
| <BuildCommandArgs>$(BuildCommandArgs) -bl</BuildCommandArgs> | |||
| <BuildCommandArgs>$(BuildCommandArgs) -bl</BuildCommandArgs> | |||
| <BuildCommandArgs>$(BuildCommandArgs) --arch $(Platform)</BuildCommandArgs> | |||
There was a problem hiding this comment.
I tried using TargetArchitecture but we call ./build.sh which actually passes -p:TargetArchitecture=x64 unless we set --arch
omajid
commented
Nov 12, 2019
patches/aspnetcore/0009-Support-global.json-on-arm64-as-well.patch
Outdated
Show resolved
Hide resolved
omajid
commented
Nov 12, 2019
patches/aspnetcore/0010-Support-building-for-arm64-on-arm64-nix.patch
Outdated
Show resolved
Hide resolved
omajid
force-pushed
the
build-on-rhel8-arm64
branch
2 times, most recently
from
d1a5e9d
to
5431455
Compare
|
I rebased it onto the latest |
|
I think that should be a reasonably easy fix, Darc needs the update here and then we can change our Darc version. |
omajid
force-pushed
the
build-on-rhel8-arm64
branch
from
5431455
to
89f0edb
Compare
|
The version here should already be correct. How do I find out what darc version that is included in? Currently, source-build pulls down this version: |
|
Oh, right, I can just put the new version in I tried it out but even |
|
Sounds like it was just two PRs going in without being tested against each other, should be fixed after #1365 goes in. |
omajid
force-pushed
the
build-on-rhel8-arm64
branch
from
f511b3a
to
967e294
Compare
|
I have rebased this against the |
crummel
reviewed
Nov 20, 2019
| <DotnetDebToolVersion>2.0.0</DotnetDebToolVersion> | ||
| <MicrosoftNETTestSdkVersion>15.8.0</MicrosoftNETTestSdkVersion> | ||
| - <MicrosoftSourceLinkVersion>1.0.0-beta2-18618-05</MicrosoftSourceLinkVersion> | ||
| + <MicrosoftSourceLinkVersion>1.0.0-beta2-19367-01</MicrosoftSourceLinkVersion> |
There was a problem hiding this comment.
This is interesting - this version should be getting overridden by our built SourceLink version anyway. I'll take a look at that.
There was a problem hiding this comment.
I added the entire git format-patch from the core-sdk PR: dotnet/installer#4102. It could very well be that source-build doesn't care about this property. Do you want me to test out removing this? Or leave as it is, like the change that was merged into core-sdk?
Btw, the same applies for all the new patches added by this PR. They are the git format-patch output from the individual repos, tweaked to apply to source-build's release/3.1, but without any other changes.
There was a problem hiding this comment.
I checked and the override is working in my local build, so we must have a new enough version of SourceLink in source-build if this is working for you. I updated it not too long ago so that makes sense. I'd prefer leaving this change out if we can just because version properties like this tend to conflict quite a bit.
omajid
commented
Nov 20, 2019
| @@ -200,7 +203,7 @@ | |||
| <ExtraPackageVersionPropsPackageInfo Include="MicrosoftNETCoreAppRuntimePackageVersion" Version="%24(MicrosoftNETCoreDotNetAppHostPackageVersion)" /> | |||
| <ExtraPackageVersionPropsPackageInfo Include="MicrosoftNETCoreAppRuntimeVersion" Version="%24(MicrosoftNETCoreDotNetAppHostPackageVersion)" /> | |||
| <ExtraPackageVersionPropsPackageInfo Include="MicrosoftNETCoreAppHostPackageVersion" Version="%24(MicrosoftNETCoreDotNetAppHostPackageVersion)" /> | |||
| <ExtraPackageVersionPropsPackageInfo Include="MicrosoftAspNetCoreAppRuntimePackageVersion" Version="%24(MicrosoftAspNetCoreAppRuntimeLinuxX64PackageVersion)" /> | |||
| <ExtraPackageVersionPropsPackageInfo Include="MicrosoftAspNetCoreAppRuntimePackageVersion" Version="%24(MicrosoftAspNetCoreAppRuntimeLinux$(Platform)PackageVersion)" /> | |||
There was a problem hiding this comment.
There is a change in capitalization here. It goes from ...X64... to ...x64.... But msbuild properties are case in-sensitive, so it doesn't result in a functional change.
omajid
commented
Nov 20, 2019
| From 84d274a8f3d416b0a5bd999e3d1c43ae1535e38f Mon Sep 17 00:00:00 2001 | ||
| From: Omair Majid <[email protected]> | ||
| Date: Wed, 23 Oct 2019 15:43:57 -0400 | ||
| Subject: [PATCH] Support global.json on arm64 as well |
There was a problem hiding this comment.
This is dotnet/aspnetcore#14790.
omajid
commented
Nov 20, 2019
| From e2946a26c11be7f7f0c223721a5b14f58f2ea240 Mon Sep 17 00:00:00 2001 | ||
| From: Omair Majid <[email protected]> | ||
| Date: Mon, 11 Nov 2019 13:37:40 -0500 | ||
| Subject: [PATCH] Support building for arm64 on arm64 (*nix) |
There was a problem hiding this comment.
This is dotnet/aspnetcore#15354
omajid
commented
Nov 20, 2019
| From 4b5c617203cfb9d2c1b12995e12d819fba6d7b6f Mon Sep 17 00:00:00 2001 | ||
| From: Omair Majid <[email protected]> | ||
| Date: Tue, 8 Oct 2019 17:02:29 -0400 | ||
| Subject: [PATCH] Enable building on arm64 machines |
There was a problem hiding this comment.
This is dotnet/installer#4102
omajid
commented
Nov 20, 2019
| From c02027bfee1c523006430183d37c1b61072d5ed8 Mon Sep 17 00:00:00 2001 | ||
| From: Omair Majid <[email protected]> | ||
| Date: Fri, 4 Oct 2019 16:08:59 -0400 | ||
| Subject: [PATCH] Enable building for arm64 on arm64 |
There was a problem hiding this comment.
This is dotnet/core-setup#8468
omajid
commented
Nov 20, 2019
| From d36274c31cc30a946c023b7a5bb5c6fa1ff86625 Mon Sep 17 00:00:00 2001 | ||
| From: Omair Majid <[email protected]> | ||
| Date: Tue, 20 Aug 2019 13:40:19 -0400 | ||
| Subject: [PATCH] Enable build on hosted arm64 |
There was a problem hiding this comment.
This is dotnet/corefx#40453
omajid
force-pushed
the
build-on-rhel8-arm64
branch
from
967e294
to
db34848
Compare
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.
crummel
force-pushed
the
build-on-rhel8-arm64
branch
from
db34848
to
03496f2
Compare
|
Rebased on the 3.0 => 3.1 merge, it was pretty clean but let me know if anything looks weird. |
dseefeld
approved these changes
Nov 22, 2019
|
Nvm, looks like network issues, I think I misread the error messages. |
|
Sorry about that, I think I fixed the patches now. |
|
Thanks! I am going to try out this locally on an arm64 box. |
|
The OSX failure is expected so we can merge this as soon as it looks good to you @omajid. |
|
I am seeing Looks like |
|
Actually, looks like the |
|
Sorry about the noise. Everything looks good here. 👍 to merge this. |
|
Awesome, thanks for doing this! |
|
🎉 🎉 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this change, I can build source-build on RHEL 8 on arm64:
I also needed this change toThe following is redundant because thearcade-services:arcade-servicessubmodule was removed:Some open questions:
I am working on opening PRs to get these patches merged in the orginial repos. What should I do for the
release/3.0andrelease/3.1branches? Should I try and get the fixes in the original repos too? Or is it okay to carry these patches for 3.0 and 3.1?There's a ton of hardcoded
x64assumptions. Is there a longer term plan to fix them? Is this something I need to look into?There's a number of assumptions that cross compile and
armmean that the tools building this are x64 (only).Any thoughts or comments?