Skip to content
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

feat: use pkg-prebuilds #507

Merged
merged 30 commits into from
Nov 26, 2023
Merged

Conversation

Julusian
Copy link
Contributor

@Julusian Julusian commented May 5, 2023

@Julusian Julusian mentioned this pull request May 5, 2023
@Julusian
Copy link
Contributor Author

Julusian commented May 5, 2023

Once the build is complete, there will be a bunch of artifacts, the one called all-prebuilds is the useful one, and is a simple zip containing the artifacts from all of the other builds.

When releasing a version of the library, what I do is:

  1. push changes. this could just be to master, or the tag to release
  2. once the builds have run, download all-prebuilds from the correct workflow.
  3. extract all-prebuilds ending up with a directory structure like:
    image (6)
    The key bit here, is a prebuilds folder containing all the HID-* folders
  4. Final quick test.
  5. (optional) npm publish --dry-run to make sure it will publish a sensible set of files
  6. npm publish (or equivalent command)

@todbot
Copy link
Contributor

todbot commented May 5, 2023

Sorry if I was unclear, and this is really more of a general Github Actions question that I've never found a good answer for: When testing a change that's primarily a Github Action change, how do you go about trying it out without polluting the git history with lots of little changes? And then, explicitly for this case: how do I pull this PR into my own space and test it?

@todbot
Copy link
Contributor

todbot commented May 5, 2023

And to clarify, I don't want to run these Github Actions locally, but actually run them on Github's infrastructure, but isolated from this repo.

@Julusian
Copy link
Contributor Author

Julusian commented May 5, 2023

Sorry if I was unclear, and this is really more of a general Github Actions question that I've never found a good answer for: When testing a change that's primarily a Github Action change, how do you go about trying it out without polluting the git history with lots of little changes?

I generally do it in a branch, then either squash the branch when merging it so it becomes a single commit in master, or manually copy the resulting changes into a new branch.
For example, Julusian/node-freetype2#4 became a single git commit:
image

There might be better ways of testing without iterating so much, but I havent found them yet

And then, explicitly for this case: how do I pull this PR into my own space and test it?

The gh cli tool probably has something to help here, but you can checkout a PR to your own branch name with:

git fetch origin pull/507/head:local-testing-branch
git checkout local-testing-branch

You can then push that branch to here or another repository.

@todbot
Copy link
Contributor

todbot commented May 5, 2023

The gh cli tool probably has something to help here, but you can checkout a PR to your own branch name with:

I was just looking at that. Thanks.

@todbot
Copy link
Contributor

todbot commented May 5, 2023

Okay, I have done tests on a handful of machines and architectures and looks like the shared libraries are being loaded correctly, and nothing is obviously broken. Woohoo!

Two more questions:

  • This change seems like a major rev, do you agree? If so, this becomes 3.x and new async stuff 4.x?
  • Normal merge or Squash merge? Do you want the history of your edits of this PR retained?

@Julusian
Copy link
Contributor Author

Julusian commented May 6, 2023

This change seems like a major rev, do you agree? If so, this becomes 3.x and new async stuff 4.x?

I think it would make sense to release both as 3.0, I don't see any benefit to releasing this as 3.0 then almost immediately doing a 4.0 for the async stuff
But yes, I do think it should be released in a major version change.

Normal merge or Squash merge? Do you want the history of your edits of this PR retained?

I would do a squash merge

@todbot
Copy link
Contributor

todbot commented Nov 26, 2023

Another question: With pkg-prebuilds does npm install node-hid fall back to compilation still if a prebuild isn't present? I'm mostly concerned about Raspberry Pi and other SBC users who use node-hid a fair bit but will now no longer have prebuilds for their archs. (if they ever did for those weird SBCs)

@Julusian
Copy link
Contributor Author

Julusian commented Nov 26, 2023

Yes, it is doing the same flow of using the install hook to verify if there is a usable prebuild and if not it will execute node-gyp

But it most likely isn't necessary for them to do a build, as the github actions changes included here does produce prebuilds for:

  • win x64
  • win ia32
  • mac x64
  • mac arm64
  • linux x64
  • linux arm64
  • linux armv7
  • linux x64 musl

all the linux builds are being made in docker using debian:10 images, which should minimise any potential glibc version issues.

This covers all of the Tier 1 supported platforms https://github.com/nodejs/node/blob/v18.x/BUILDING.md#platform-list

@todbot todbot merged commit 45a453e into node-hid:master Nov 26, 2023
@todbot
Copy link
Contributor

todbot commented Nov 26, 2023

Great! merged

@todbot
Copy link
Contributor

todbot commented Nov 26, 2023

And when it comes time to npm publish, I would first download the all-prebuilds artifact, unzip it as prebuilds in the node-hid directory and then publish? (Apologies if these are remedial questions, I've been out of the Node space for a while)

@Julusian
Copy link
Contributor Author

Yes that is correct
I updated your publishing documentation to reflect this new flow https://github.com/node-hid/node-hid/blob/master/Publishing.md

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.

None yet

3 participants