Skip to content
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

Enable CI builds for aarch64-macos #3137

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

yvt
Copy link
Contributor

@yvt yvt commented Jul 21, 2022

Closes #1647.

A preview CI run based on this patch and the built artifacts can be found at https://github.com/yvt/helix/actions/runs/2709143241.

yvt added 3 commits July 21, 2022 12:37
The tests are conditionally disabled for this target because the x86_64 CI
host is unable to run AArch64 binaries. (There is no officially-supported
reverse Rosetta 2.)
Certain targets, such as `aarch64-apple-*`, require additional compiler
flags to cross-compile for the intended target.
@yvt yvt changed the title Re-enable CI builds for aarch64-macos Enable CI builds for aarch64-macos Jul 21, 2022
@yvt yvt mentioned this pull request Jul 21, 2022
@the-mikedavis the-mikedavis self-requested a review July 26, 2022 05:13
@archseer
Copy link
Member

Do we need to do anything special to support this in the homebrew tap? https://github.com/helix-editor/homebrew-helix/blob/master/Formula/helix.rb

@archseer
Copy link
Member

@yvt
Copy link
Contributor Author

yvt commented Jul 27, 2022

I believe switching url and sha256 by CPU architectures is all we need to do.

on_macos do
  if Hardware::CPU.arm?
    url "https://github.com/helix-editor/helix/releases/download/#{version}/helix-#{version}-aarch64-macos.tar.xz"
    sha256 "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
  end
  if Hardware::CPU.intel?
    url "https://github.com/helix-editor/helix/releases/download/#{version}/helix-#{version}-x86_64-macos.tar.xz"
    sha256 "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
  end
end

.github/workflows/release.yml Outdated Show resolved Hide resolved
Comment on lines -314 to +323
.target(BUILD_TARGET);
.target(target.unwrap_or(BUILD_TARGET));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would target be different from BUILD_TARGET? Both come from TARGET environment variables in build scripts, BUILD_TARGET from the helix-loader crate's build script and target from helix-term's

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When helix_loader::build_grammars is being used for cross compilation.

BUILD_TARGET represents the target on which the code of helix-loader runs. helix-term's build script (which has helix-loader as build-dependency) needs to use helix-loader to cross-compile for helix-term's target, which may be different from BUILD_TARGET.

E.g., when cross-compiling helix-term to an aarch64 target on an x86_64 host, two binaries of helix-loader are built:

  • One binary for aarch64 target for use by helix-term's main crate (normal dependency)

    • helix-loader's build script runs with TARGET = "aarch64"
    • Hence, env!("BUILD_TARGET") = "aarch64"
    • helix-term's main crate code passes target = None because it wants to compile grammars for the current host = helix-loader's build target
  • One binary for x86_64 target for use by helix-term's build script (build dependency)

    • helix-loader's build script runs with TARGET = "x86_64"
    • Hence, env!("BUILD_TARGET") = "x86_64"
    • helix-term's build script passes target = "aarch64" because it wants to cross-compile grammars for helix-term's build target, not the host on which the build script runs

Simplifies a conditional expression in the CI workflow configuration.

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
@yvt yvt requested a review from the-mikedavis July 29, 2022 00:53
@yvt yvt mentioned this pull request Jul 29, 2022
6 tasks
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me then :)

I tried out your example run artifacts on an aarch64-macos machine and it all works great 🎉

Thanks for working on this

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yvt!

@archseer archseer merged commit aa4394c into helix-editor:master Aug 2, 2022
@yvt yvt deleted the patch/ci-aarch64-macos branch September 4, 2022 08:21
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apple Silicon arm64 release
3 participants