-
Notifications
You must be signed in to change notification settings - Fork 97
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
build-git-installers: also build on macOS/ARM #608
Conversation
When building the payload for the macOS installer, we already encode the CPU architecture ("x86_64") in the name of the build directory, there is no need to also encode the CPU vendor ("intel"). This will simplify the upcoming changes to support building for Apple Silicon, too. Signed-off-by: Johannes Schindelin <[email protected]>
Fix a typo. Signed-off-by: Johannes Schindelin <[email protected]>
The `die` function is not a shell built-in, it has to be defined in every step that uses it. While at it, enable the `e` flag (error if even one command fails) and `x` (trace the commands as they are executed). Signed-off-by: Johannes Schindelin <[email protected]>
This prepares the `macos-installer` code for building on Apple Silicon. The paths and a couple of flags need to be slightly different, but nothing big. The changes to the `build-git-installers` workflow: - The keen reader will notice that `CFLAGS` & friends had to be moved from environment variables (which the ARM64 `make` apparently ignores) to `config.mak`. - A couple of jobs needed to be turned into matrix jobs to handle both x86_64 and arm64 CPU architectures. - Accommodate for Homebrew being installed into `/opt/homebrew` on Apple Silicon instead of `/usr/local`, with the `bin` directory of the former _overriding_ the latter. - Homebrew on Apple Silicon apparently does not link `gcc` anymore, so let's just hard-code `gcc-13` for now. Signed-off-by: Johannes Schindelin <[email protected]>
The t5559 failures will be addressed via #609. |
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.
LGTM
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.
Great work! ✨
@@ -764,6 +804,10 @@ jobs: | |||
- name: Install macOS | |||
if: contains(matrix.component.os, 'macos') | |||
run: | | |||
# avoid letting Homebrew's `git` in `/opt/homebrew/bin` override `/usr/local/bin/git` | |||
test arm64 != "$(uname -m)" || |
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.
Super nitpicky, but why not use test x86_64
as you did above?
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.
Honestly, I'm not quite sure what is more correct: Is /opt/homebrew/
used because it's ARM64? Or is /usr/local/
only used for x86_64? So far, we only have those two CPU architectures to deal with, which means that test arm64 !=
and test x86_64 =
would both work, but... 😄
This modifies the
build-git-installers
GitHub workflow such that it also builds a macOS/ARM64 package. Example run is here: https://github.com/microsoft/git/actions/runs/6273987160/job/17040468903. @jeffhostetler kindly validated on their M1.This fixes #565.