maplibre-native
maplibre-native copied to clipboard
maplibre
Node release workflow #2 (alternative)
This is a merge of work done in https://github.com/maplibre/maplibre-gl-native/pull/378 and work done by @mnutt in https://github.com/mnutt/maplibre-gl-native/tree/publish-github
It is still a work in progress but I think this is in a mostly working state.
Notes
- I think something needs to be done with the node-pre-gyp used in this. It is pointing to a repository bring that brings node 18 support. this was added in by @mnutt so I'm not too familiar with it, but one of the build seemed to fail without it.
I guess the reason node-pre-gyp fails on this PR and not the one we just merged is because this workflow uses the node-pre-gyp "--target" switch, where the other one uses a node matrix.
When using the target switch, node-pre-gyp relies on https://github.com/mapbox/node-pre-gyp/blob/master/lib/util/abi_crosswalk.json to match version numbers to abi, and right now it has not been updated to support node 18, even though they have a PR for it https://github.com/mapbox/node-pre-gyp/pull/649
Looking more, it looks like there has been some discussion around this, https://github.com/mapbox/node-pre-gyp/issues/647 , but node-pre-gyp seems pretty much unmaintained https://github.com/mapbox/node-pre-gyp/issues/657
I found in another thread how to update abi_crosswalk.json https://github.com/mapbox/node-pre-gyp/blob/master/contributing.md , so for now I ran that and uploaded my own version to @acalcutt/node-pre-gyp
@acalcutt I think it's a good idea to fork the node-pre-gyp, as Mapbox doesn't maintain most of these and PRs can be there for a very long time - if the repo is somewhat maplibre-only we can also put it under maplibre/* on npm/github, but that can always happen down the line. An update of the license would be good though. Is it ready for review?
Is there any other package that we could use instead (longer term)? We should try to avoid the number of such packages that we maintain under the MapLibre umbrella.
@ntadej , the other package would be Mapbox's but as I just mentioned, they don't have a good track record of merging PRs in my experience or working on these packages in general, so we might as well have our own as it won't fall behind. I'm all for cutting down on the number of deps wherever possible, and in GL-JS we basically cut the number if half ~200 -> ~100, so there is probably a lot of room for improvement in native as well. Noticed i.e. "ejs" which can be replaced with string literals
What I mean is, is really Mapbox the only company that set up such a release workflow of pre-building C++ code and packaging it as node package?
@ntadej , likely not, but they might have had the foresight to be the first to make an open-source library that everyone else could use, which would explain the 1k stars on the repo.
Great to see it passing throughout - what does this need before it's ready?
Should be ready, I am just testing it now on my fork (minus the arm) to make sure the publish part is working here https://github.com/acalcutt/maplibre-gl-native/actions/runs/3115836678
EDIT: I forgot to change to my node-pre-gyp in this test so it failed... so one more test in a minute... https://github.com/acalcutt/maplibre-gl-native/actions/runs/3115976004/jobs/5053406349
Should be ready now. I haven't gotten to test publishing on arm because i don't have it, but the ci tests look good on it here so it should work.
you have me wondering how the next pre-release goes. I curious if the arm64 build works.
I wonder why the builds are separate? Is this the preferred node way? Or should we prepare universal builds?
@ntadej , I attempted to go the universal build direction, because it then would be faster to run it all on m1 machines, but got a lot of issues with dependencies like glfw https://github.com/maplibre/maplibre-gl-native/pull/502 as they presumably also have to be universal - it also made it much harder to keep node 14 support, as it doesn't support arm natively and has to go through rosetta 2, so it depends a bit if we'd like to keep that.
OK, as long as it works, I guess it's fine like that for now. I wonder if we should set up some release testing pipelines.
It did publish as expected to github, but not to npm. The ubuntu build failed the first time and I had to retry it, so it could have influenced it.
it looks like i forgot to remove
&& matrix.node == '18'
from the npm publishes, so thats probably why that didn't work.
Great, let's fix it and try again. Would you mind adding a entry in the node changelog for the arm support?