Skip to content

feat(ci): add workflow to include parser and binary artifacts #253

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

matthias-Q
Copy link
Collaborator

@matthias-Q matthias-Q commented Apr 21, 2024

This PR adds a GH Workflow to build source artifacts including the parser.c, sql.so and a wasm file for Linux x86_64 and MacOS Arm64.

Along the way, I have fixed an issue in the parser.c (replaced strcpy with memcpy)

As of now, this does probably not work as intended, as in the final step the github cli tool is used to create a release and the trigger is the manual creation of the release. I have no idea how to fix that. I fails due to a collision of release tags. @DerekStride do you have any ideas?

This PR is aimed to solve #234

I have noticed some issues: When installing it via npm, we require the nan package. It was n the lock file, but to get it working I had to add a manual installation step beforehand.

@matthias-Q matthias-Q requested review from dmfay and DerekStride April 21, 2024 12:24
@matthias-Q
Copy link
Collaborator Author

I did some changes in the mean time and I think this could work.

We keep the workflow as outlined in the CONTRIBUTION.md:

  • create a release PR (after running commit-tag-version)*
  • the new workflow create_release.yml will only trigger when a tagged release on main has been made the follows the semver pattern. This removed the need for manually creating a release.

I tried to test that on my fork:

https://github.com/matthias-Q/tree-sitter-sql/actions/runs/8773987845

@clason
Copy link

clason commented Apr 22, 2024

For the record, upstream is planning to add support for that to the CLI (tree-sitter publish); see also https://github.com/tree-sitter/workflows.

(And isn't the wasm file platform independent? I thought that was the whole point!)

@matthias-Q
Copy link
Collaborator Author

Yeah, I think wasm is platform independent. I just included it in the build process for each system. So maybe I should add it separately.

I will take a look into tree-sitter publish. Thanks for the hint.

@clason
Copy link

clason commented Apr 22, 2024

So maybe I should add it separately.

That would make sense; it can take quite a long time (and a lot of memory).

I will take a look into tree-sitter publish. Thanks for the hint.

Not a thing yet ;) Just a heads-up. The workflows should help, though.

@matthias-Q
Copy link
Collaborator Author

I have separated the wasm build and cleaned up the pipeline a bit.

@matthias-Q
Copy link
Collaborator Author

I guess I should remove the notes.md and just use the CHANGELOG.md in the relase. The hashes are added as assets anyways.

@DerekStride
Copy link
Owner

@DerekStride do you have any ideas?

I think we have to create a draft release and upload artifacts to that on push to main.

Then it'd be a manual process to convert the draft to an actual release.

We'd end up with a bunch of draft releases which is probably fine but we could have a cleanup workflow.

@matthias-Q
Copy link
Collaborator Author

matthias-Q commented Apr 29, 2024

IIUC in my latest version, the release pipeline will only be triggered once a new tagged commit on main is made. It just makes the manual release MR unnecessary. But please double check that, as it is quite hard to test.
I am referring to this CI trigger:

on:
  push:
    branch:
      - main
    tags:
      - v[0-9]+.[0-9]+.[0-9]+

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.

3 participants