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.

Sign up for GitHub

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
Merged

Enable CI builds for aarch64-macos #3137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b62db50
chore(ci): build binary for aarch64-macos
yvt Jul 20, 2022
5cd4d85
fix(loader): pass `cc::Tool::args()`
yvt Jun 23, 2022
fc1c7da
feat: support grammar cross-compilation
yvt Jul 20, 2022
bc5689c
chore(ci): match `*-macos` by checking suffix
yvt Jul 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ jobs:
rust: stable
target: x86_64-pc-windows-msvc
cross: false
# - build: aarch64-macos
# os: macos-latest
# rust: stable
# target: aarch64-apple-darwin
- build: aarch64-macos
os: macos-latest
rust: stable
target: aarch64-apple-darwin
cross: false
skip_tests: true # x86_64 host can't run aarch64 code
# - build: x86_64-win-gnu
# os: windows-2019
# rust: stable-x86_64-gnu
Expand Down Expand Up @@ -100,6 +102,7 @@ jobs:

- name: Run cargo test
uses: actions-rs/cargo@v1
if: "!matrix.skip_tests"
with:
use-cross: ${{ matrix.cross }}
command: test
Expand All @@ -113,7 +116,7 @@ jobs:
args: --release --locked --target ${{ matrix.target }}

- name: Strip release binary (linux and macos)
if: matrix.build == 'x86_64-linux' || matrix.build == 'x86_64-macos'
if: matrix.build == 'x86_64-linux' || endsWith(matrix.build, 'macos')
run: strip "target/${{ matrix.target }}/release/hx"

- name: Strip release binary (arm)
Expand Down
24 changes: 17 additions & 7 deletions helix-loader/src/grammar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,12 @@ pub fn fetch_grammars() -> Result<()> {
run_parallel(grammars, fetch_grammar, "fetch")
}

pub fn build_grammars() -> Result<()> {
run_parallel(get_grammar_configs()?, build_grammar, "build")
pub fn build_grammars(target: Option<String>) -> Result<()> {
run_parallel(
get_grammar_configs()?,
move |grammar| build_grammar(grammar, target.as_deref()),
"build",
)
}

// Returns the set of grammar configurations the user requests.
Expand Down Expand Up @@ -124,13 +128,14 @@ fn get_grammar_configs() -> Result<Vec<GrammarConfiguration>> {

fn run_parallel<F>(grammars: Vec<GrammarConfiguration>, job: F, action: &'static str) -> Result<()>
where
F: Fn(GrammarConfiguration) -> Result<()> + std::marker::Send + 'static + Copy,
F: Fn(GrammarConfiguration) -> Result<()> + std::marker::Send + 'static + Clone,
{
let pool = threadpool::Builder::new().build();
let (tx, rx) = channel();

for grammar in grammars {
let tx = tx.clone();
let job = job.clone();

pool.execute(move || {
// Ignore any SendErrors, if any job in another thread has encountered an
Expand Down Expand Up @@ -240,7 +245,7 @@ where
}
}

fn build_grammar(grammar: GrammarConfiguration) -> Result<()> {
fn build_grammar(grammar: GrammarConfiguration, target: Option<&str>) -> Result<()> {
let grammar_dir = if let GrammarSource::Local { path } = &grammar.source {
PathBuf::from(&path)
} else {
Expand Down Expand Up @@ -273,10 +278,14 @@ fn build_grammar(grammar: GrammarConfiguration) -> Result<()> {
}
.join("src");

build_tree_sitter_library(&path, grammar)
build_tree_sitter_library(&path, grammar, target)
}

fn build_tree_sitter_library(src_path: &Path, grammar: GrammarConfiguration) -> Result<()> {
fn build_tree_sitter_library(
src_path: &Path,
grammar: GrammarConfiguration,
target: Option<&str>,
) -> Result<()> {
let header_path = src_path;
let parser_path = src_path.join("parser.c");
let mut scanner_path = src_path.join("scanner.c");
Expand Down Expand Up @@ -311,13 +320,14 @@ fn build_tree_sitter_library(src_path: &Path, grammar: GrammarConfiguration) ->
.opt_level(3)
.cargo_metadata(false)
.host(BUILD_TARGET)
.target(BUILD_TARGET);
.target(target.unwrap_or(BUILD_TARGET));
Comment on lines -314 to +323
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

let compiler = config.get_compiler();
let mut command = Command::new(compiler.path());
command.current_dir(src_path);
for (key, value) in compiler.env() {
command.env(key, value);
}
command.args(compiler.args());

if cfg!(windows) {
command
Expand Down
3 changes: 2 additions & 1 deletion helix-term/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ fn main() {

if std::env::var("HELIX_DISABLE_AUTO_GRAMMAR_BUILD").is_err() {
fetch_grammars().expect("Failed to fetch tree-sitter grammars");
build_grammars().expect("Failed to compile tree-sitter grammars");
build_grammars(Some(std::env::var("TARGET").unwrap()))
.expect("Failed to compile tree-sitter grammars");
}

println!("cargo:rerun-if-changed=../runtime/grammars/");
Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ FLAGS:
}

if args.build_grammars {
helix_loader::grammar::build_grammars()?;
helix_loader::grammar::build_grammars(None)?;
return Ok(0);
}

Expand Down