tree-sitter-sql icon indicating copy to clipboard operation
tree-sitter-sql copied to clipboard

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

Open matthias-Q opened this issue 1 year ago • 8 comments

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 avatar Apr 21 '24 12:04 matthias-Q

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

matthias-Q avatar Apr 21 '24 16:04 matthias-Q

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!)

clason avatar Apr 22 '24 07:04 clason

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.

matthias-Q avatar Apr 22 '24 07:04 matthias-Q

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.

clason avatar Apr 22 '24 07:04 clason

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

matthias-Q avatar Apr 22 '24 18:04 matthias-Q

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

matthias-Q avatar Apr 22 '24 21:04 matthias-Q

@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.

DerekStride avatar Apr 25 '24 23:04 DerekStride

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]+

matthias-Q avatar Apr 29 '24 17:04 matthias-Q