search

emacs-tree-sitter / elisp-tree-sitter

Emacs Lisp bindings for tree-sitter
https://emacs-tree-sitter.github.io
MIT License
815 stars 73 forks source link

Fix build #275

Closed ubolonton closed 2 months ago

ubolonton commented 3 months ago

Trying to fix a couple of build issues.

ubolonton commented 3 months ago

@jcs090218 bin/test fails for me on macOS 12.6.3 (Monterey) with this (exit code 1):

Cursor doesn’t currently support :depth property

If I modify bin/test to run Emacs directly instead of through eask, it works. I think it crashes on the first failed test, not sure why. I think you can troubleshoot that better.

On another note, all macOS jobs skip the test step. Maybe it's due due to incorrect matrix settings?

ubolonton commented 3 months ago

@jcs090218 Builds break on Azure Pipelines. It's up to you to decide whether to keep them, or to use GitHub Actions only.

ubolonton commented 3 months ago

bin/test also failed on Ubuntu, but got silenced (by continue-on-error: true?) https://github.com/emacs-tree-sitter/elisp-tree-sitter/actions/runs/8316133478/job/22755231757?pr=275#step:12:28

jcs090218 commented 3 months ago

Sorry for the late reply; this somehow goes under my radar. 🤔 I'm a little busy recently. I'll take a look ASAP.

jcs090218 commented 3 months ago

If I modify bin/test to run Emacs directly instead of through eask, it works. I think it crashes on the first failed test, not sure why. I think you can troubleshoot that better.

This is a bug from Eask. I've fixed it now.

On another note, all macOS jobs skip the test step. Maybe it's due due to incorrect matrix settings?

After seeing the git blame, I don't remember I've changed the matrix settings. The only thing I've changed is to use macos-12 https://github.com/emacs-tree-sitter/elisp-tree-sitter/commit/e2191d167ab0dfa2acc27bb38b804b45b49eb23b. 🤔 BTW, why do we have if: ${{ !matrix.target }} everywhere? Are those condition statements necessary?

@jcs090218 Builds break on Azure Pipelines. It's up to you to decide whether to keep them, or to use GitHub Actions only.

Azure Pieplines long time ago (before I start maintaining this repo). 🤔 I think it's good to just use GitHub Actions. 👍

bin/test also failed on Ubuntu, but got silenced (by continue-on-error: true?)

It's been a while, so I'm not exactly sure. I think continue-on-error was added because I was trying to get the release workflow working. But since you are back, we can remove the continue-on-error from L66 and L77 that was originally added in https://github.com/emacs-tree-sitter/elisp-tree-sitter/commit/4b03970f222fa8638d425e84f0880f2749e13dd1.

ubolonton commented 3 months ago

This is a bug from Eask. I've fixed it now.

🎉

🤔 BTW, why do we have if: ${{ !matrix.target }} everywhere? Are those condition statements necessary?

IIRC, the intention was to set target only when building for another platform (e.g. building for aarch64-apple-darwin on Intel macOS), so we didn't run tests there.

@jcs090218 Builds break on Azure Pipelines. It's up to you to decide whether to keep them, or to use GitHub Actions only.

Azure Pieplines long time ago (before I start maintaining this repo). 🤔 I think it's good to just use GitHub Actions. 👍

Cool!

But since you are back

I'm not really back though. This is still low in my priority list. I'm just trying to do some quick fixes here and there.

jcs090218 commented 2 months ago

It seems like the CI has passed. Can we merge this?