DerekStride / tree-sitter-sql

SQL grammar for tree-sitter
http://derek.stride.host/tree-sitter-sql/
MIT License
147 stars 47 forks source link

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

Open matthias-Q opened 4 months ago

matthias-Q commented 4 months ago

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 commented 4 months ago

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

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

I tried to test that on my fork:

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

clason commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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

matthias-Q commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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