Prebuilt binaries about node-oom-heapdump HOT 16 CLOSED

Great, I'll start on this first thing Monday. It will be a learning exercise for me too!

from node-oom-heapdump.

Hi @spmiller,

Thanks for the appreciation!
I would certainly be interested in pre-built binaries; i'm not familiar with how this process would/should work, so i would need to investigate this further.
A patch/PR would really help here!

Let me know if you need help/review code 👍
TravisCI is already used, so that might help.

See https://travis-ci.org/github/blueconic/node-oom-heapdump

from node-oom-heapdump.

Hi @paulrutter, I opened #11 for this. I've tested this on my fork, and it seems to work pretty well on both Linux and Windows (though I haven't a MacOS machine to test with).

This will only work if we create a Github release for each release, which I don't think is part of your current workflow (we need to store the binaries somewhere and Github seems as good a place as any). I can have a go at creating another Github action to automatically create a Github release whenever a release tag is created if you would like to keep your workflow the same?

Edit: I must have been blind, you are creating releases. In that case I don't think any further changes are required.

from node-oom-heapdump.

Hi @spmiller,

Thanks for your quick work! We indeed create Github releases for each release, with a version tag.
I'm in the middle of finishing a release sprint, after that i'll have more time to review the PR.

from node-oom-heapdump.

Sounds good. Best of luck for your sprint!

from node-oom-heapdump.

Thanks for merging! Perhaps a beta release first to try it out?

from node-oom-heapdump.

The action has run successful, but i'm not seeing the artifacts on https://github.com/blueconic/node-oom-heapdump/releases/tag/1.3.0. Could this be because it's a pre-release?

from node-oom-heapdump.

Ugh, I can't see any documentation that says prereleases can't have assets uploaded. I'll try debugging on my fork. Sorry about this ☹️

from node-oom-heapdump.

Not a problem, even when 1.3.0 is used (npm install node-oom-heapdump) will fallback to building from source.
So nothing is broken ATM.

from node-oom-heapdump.

The weird thing is:

https://github.com/blueconic/node-oom-heapdump/runs/803163441?check_suite_focus=true

Vs your fork:

https://github.com/spmiller/node-oom-heapdump/runs/803237340?check_suite_focus=true

See the difference in output in the upload task.

from node-oom-heapdump.

I think i found it; the PR contained the "remove_path" in the package. After it was removed, the binaries appeared.
It looks like it's working now:

PS C:\temp\prebuilt> npm install node-oom-heapdump                                                                      
> [email protected] install C:\temp\prebuilt\node_modules\gc-stats
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using needle for node-pre-gyp https download
node-pre-gyp WARN Tried to download(403): https://node-binaries.s3.amazonaws.com/gcstats/v1.4.0/Release/node-v72-win32-x64.tar.gz
node-pre-gyp WARN Pre-built binaries not found for [email protected] and [email protected] (node-v72 ABI, unknown) (falling back to source compile with node-gyp)
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
  gcstats.cc
  win_delay_load_hook.cc
     Creating library C:\temp\prebuilt\node_modules\gc-stats\build\Release\gcstats.lib and object C:\temp\prebuilt\node
  _modules\gc-stats\build\Release\gcstats.exp
  gcstats.vcxproj -> C:\temp\prebuilt\node_modules\gc-stats\build\Release\\gcstats.node
  Copying C:\temp\prebuilt\node_modules\gc-stats\build\Release\/gcstats.node to C:/temp/prebuilt/node_modules/gc-stats/
  build/gcstats/v1.4.0/Release/node-v72-win32-x64\gcstats.node
          1 file(s) copied.

> [email protected] install C:\temp\prebuilt\node_modules\node-oom-heapdump
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using needle for node-pre-gyp https download
[node-oom-heapdump] Success: "C:\temp\prebuilt\node_modules\node-oom-heapdump\build\Release\node_oom_heapdump_native.node" is installed via remote

+ [email protected]
added 141 packages from 37 contributors and audited 141 packages in 10.013s

1 package is looking for funding
  run `npm fund` for details

found 3 low severity vulnerabilities
  run `npm audit fix` to fix them, or `npm audit` for details

Too bad the gc-stats is not having all the right binaries in place, still falling back to building from source though.

from node-oom-heapdump.

Closing as fixed.
Thanks @spmiller!

from node-oom-heapdump.

Thanks for debugging @paulrutter!

I think i found it; the PR contained the "remove_path" in the package. After it was removed, the binaries appeared.

Hmm, this doesn't make sense to me -- I had thought that remote_path was only used by node-pre-gyp to download the binaries. It shouldn't have had any effect on the upload action in the workflow?

I am slightly concerned that the upload step is unreliable when working with pre-releases. I'll keep playing in my fork and I'll see if I can get to the bottom of it.

Too bad the gc-stats is not having all the right binaries in place, still falling back to building from source though.

At least submitting a PR for them shouldn't take me as much time this time round :)

from node-oom-heapdump.

Hmm, that's a bit concerning indeed. I tested a pre-release in your forked repo, which worked fine.
I read a bit more about Github Actions, and only found parameters to disable it for pre-releases. This would mean that it, by default, works the same as for regular releases.

A PR for gc-stats would be nice.
Let me know if you have any more info on the subject.

from node-oom-heapdump.

Hey @paulrutter, I've been playing around in my fork and I can't reproduce the borked release that happened here. I hope it was just a once-off; if we change the workflow to trigger when the release is edited as well as published then it would give it several chances to correct itself it it turns out to be unreliable.

It's worth noting that I'm using this release asset action instead of the official one because the official one doesn't support dynamically generated filenames (#4, #9. When one of those are implemented we should switch over as presumably it will be more reliable.

Re gc-stats, I tagged you on a comment there. They already have binaries being created, they just had an issue with one particular build. When [email protected] is released on NPM it will have all the binaries we need.

We've been using the new version of node-oom-heapdump here and it works great. Thanks again for your help and support with this!

from node-oom-heapdump.

Thanks for the update 👍
Glad it works for you!

from node-oom-heapdump.