Enable building .NET Core osx-arm64 in CI - #43187
Conversation
|
Tagging subscribers to this area: @ViktorHofer |
1caab77 to
2e72c12
Compare
| @@ -1,5 +1,10 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| if [ "$1" = "OSX" ] && [ "$2" = "arm64" ]; then | |||
There was a problem hiding this comment.
Not sure this is a great place to begin doing this. This should probably go in the yaml as it's tied to AzDO pools, and in documentation we should specify it as a dependency for osx-arm64.
There was a problem hiding this comment.
I'm not sure I understand the rationale. Isn't this file just for installing dependencies for CI.
There was a problem hiding this comment.
Yeah, this just feels like hiding things in scripts. This is not an installation. At least it's not obvious to me that an installation script hardcodes a path to something that's only true in AzDO (for example, what if a customer runs this expecting to brew install the deps in their box).
There was a problem hiding this comment.
@hoyosjs & I are in disagreement here... Any other opinions... Before we try to drive to consensus?
There was a problem hiding this comment.
The build step that calls this is called Install Build Dependencies. It seems logical to configure dependencies here too. If we must I can add another configure dependencies step, but it seems overkill.
There was a problem hiding this comment.
azdo seems overly specific. Are you OK with setup-native-dependencies-4ci.sh & Setup Build Dependencies?
There was a problem hiding this comment.
I chose azdo just because it's that: the xcode path for AzDO agents. It's not the same path as helix test agents necessarily or anything (my box doesn't have that path even though I use the same version of XCode now). However, I don't feel too strongly, mostly care about "setup" and "ci/not-local".
There was a problem hiding this comment.
Wouldn't be people that run this to help them set native dependencies locally?
There was a problem hiding this comment.
I hadn't seen any instructions suggesting using this for developer setup.
Edit:
I guess since this is likely temporary I can just make it a separate build step.
There was a problem hiding this comment.
I added azDO to the CI install-native-dependencies.sh command line and made the xcode-select conditional on the azDO argument. I also added brief header comments.
I initial went down the path of renaming install -> setup, but I felt like it added confusion. setup in my mind doesn't imply install. However setup/configurations is a subset of install. So I reverted it all and went with this approach.
Hope this is a satisfactory compromise.
|
Also unclear if related, somehow we ended with this job: - job: installer_coreclr__OSX_arm64_Release
displayName: Installer Build and Test coreclr OSX_arm64 Release
dependsOn:
- checkout
- coreclr__product_build_OSX_arm64_release
- libraries_build_OSX_arm64_Debug@safern do release tests for installer depend on debug libraries bits? |
|
The error remaining is just we are missing passing the architecture to the installer part of the build. The autodetection thinks it's x64 (given that's the host's architecture). |
In PR some of them do, because we build some flavors of libraries in Debug on PR and Release for CI and official build. There is a variable that controls this... |
|
I am running an internal build of this here https://dev.azure.com/dnceng/internal/_build/results?buildId=847071&view=results. I suspect I will need to add something additional to publish the new packages, but that could be a subsequent PR. |
|
Looks like the internal build completed fine. Looks to me (a novice) like the osx-arm64 packages are properly publishing. CI on this PR and the internal run are green. This is ready for a final review. |
|
I'm going to merge this. I'll work on a follow up PR to add the If there is anything else which you would like resolved. Feel free to comment on this or that PR, and I will work to get it resolved. |
|
/cc @richlander This should enable nightly runtime osx-arm64 builds. |
| - Linux_arm64 | ||
| - Linux_musl_arm64 | ||
| - ${{ if eq(variables['includeOsxOuterloop'], true) }}: | ||
| - OSX_arm64 |
There was a problem hiding this comment.
I should've caught this... I think we don't have a helix queue setup for OSX_arm64, so I guess this broke the libraries outerloop build. I can fix this in a separate PR and we can enable them once we have a queue for it.
There was a problem hiding this comment.
since we still dont have stable hardware for this we dont expect to enable test runs for osx_arm64 just yet. Can we leave it disabled till we have that?
There was a problem hiding this comment.
I should've caught this...
@safern Thanks for being genereous, but this was clearly my fault. I forget there are three main pipelines all the time. I get the rolling outerloop and the official build pipeline confused and map them to a single pipeline...
There was a problem hiding this comment.
Yeah, we have so many ymls that it is sometimes hard to get it right and the easier way to add a new flavor is to try and look where we're already building osx_x64 and just add entries for the new platform there, so confusing for all I think 😄
safern
left a comment
There was a problem hiding this comment.
This didn't enable mono on that new platform. @akoeplinger @steveisok do we want to do that?
Overall looks good to me. Thanks for doing this @sdmaclea

Enables building of .NET Core osx-arm64
Fixes #41131
/cc @mangod9