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

mason-js port #67

Closed
wants to merge 22 commits into from
Closed

mason-js port #67

wants to merge 22 commits into from

Conversation

millzpaugh
Copy link

Move to mason-js instead of mason bash!

@springmeyer springmeyer mentioned this pull request Mar 10, 2018
10 tasks
@springmeyer
Copy link
Contributor

Looking really really good @millzpaugh - so excited this came together so nicely. Per the clang-tidy error I'm working on fixing this over at #68.

@springmeyer
Copy link
Contributor

Let's hold on this until mapbox/node-cpp-skel#111 is merged (and then use that as a guide land this - since we ended up with more changes in mapbox/node-cpp-skel#111). Assigning to myself here for next actions.

@springmeyer springmeyer self-assigned this Mar 27, 2018
@springmeyer
Copy link
Contributor

Let's hold on this until mapbox/node-cpp-skel#111 is merged (and then use that as a guide land this - since we ended up with more changes in mapbox/node-cpp-skel#111). Assigning to myself here for next actions.

Nevermind. I think mapbox/node-cpp-skel#111 is going to take a little more time and there is value to testing this next. So, next action is to have @millzpaugh take this branch for a spin and:

  • pair with @mapsam to try publishing a dev build from this branch
  • pair with @mapsam ensure that binary works downstream (and that mason-js is not invoked for "from binary" install, so there should be no mason_packages placed on the system in that scenario)

@millzpaugh
Copy link
Author

millzpaugh commented Mar 29, 2018

node-pre-gyp debugging

When running make in this vtquery branch, @mapsam and I encountered an error in node-pre-gyp. (see below) This is a blocker to publishing a binary of this branch to test in mapbox-maps.

V=1 ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=true --loglevel=error
node-pre-gyp ERR! configure error 
node-pre-gyp ERR! stack Error: Failed to execute 'node-gyp configure --error_on_warnings=true --loglevel=error --module=/Users/annmillspaugh/mapbox/vtquery/lib/binding/vtquery.node --module_name=vtquery --module_path=/Users/annmillspaugh/mapbox/vtquery/lib/binding --node_abi_napi=node-v48' (Error: spawn node-gyp ENOENT)
node-pre-gyp ERR! stack     at ChildProcess.<anonymous> (/Users/annmillspaugh/mapbox/vtquery/node_modules/node-pre-gyp/lib/util/compile.js:77:29)
node-pre-gyp ERR! stack     at emitOne (events.js:96:13)
node-pre-gyp ERR! stack     at ChildProcess.emit (events.js:188:7)
node-pre-gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:202:12)
node-pre-gyp ERR! stack     at onErrorNT (internal/child_process.js:348:16)
node-pre-gyp ERR! stack     at _combinedTickCallback (internal/process/next_tick.js:74:11)
node-pre-gyp ERR! stack     at process._tickCallback (internal/process/next_tick.js:98:9)
node-pre-gyp ERR! stack     at Function.Module.runMain (module.js:577:11)
node-pre-gyp ERR! stack     at startup (node.js:159:18)
node-pre-gyp ERR! stack     at node.js:444:3
node-pre-gyp ERR! System Darwin 16.5.0
node-pre-gyp ERR! command "/Users/annmillspaugh/.nvm/versions/node/v6.0.0/bin/node" "/Users/annmillspaugh/mapbox/vtquery/node_modules/.bin/node-pre-gyp" "configure" "build" "--error_on_warnings=true" "--loglevel=error"
node-pre-gyp ERR! cwd /Users/annmillspaugh/mapbox/vtquery
node-pre-gyp ERR! node -v v6.0.0
node-pre-gyp ERR! node-pre-gyp -v v0.9.0
node-pre-gyp ERR! not ok 
Failed to execute 'node-gyp configure --error_on_warnings=true --loglevel=error --module=/Users/annmillspaugh/mapbox/vtquery/lib/binding/vtquery.node --module_name=vtquery --module_path=/Users/annmillspaugh/mapbox/vtquery/lib/binding --node_abi_napi=node-v48' (Error: spawn node-gyp ENOENT)
make: *** [release] Error 1

Initially we thought it was a versioning error re: https://github.com/mapbox/node-pre-gyp/blob/master/lib/util/versioning.js#L288, but then realized that our dev version for the package was invalid semver, i.e. 0.2.0alpha vs 0.2.0-alpha and was being parsed incorrectly.

@springmeyer any insight would be 👍 !

@springmeyer
Copy link
Contributor

@millzpaugh thanks for this ticket. That is an odd error. The most telling part of the error is:

(Error: spawn node-gyp ENOENT)

Which means that node attempted to call node-gyp and the ENOENT means it was not found. So, node-pre-gyp fell back to a source compile (since no pre-published vtquery binary was available) and a source compile needs to call node-gyp and node-gyp was not available on the PATH. This is not something that should happen because node-gyp is bundled inside a node install (and node-pre-gyp does some backflips to find it. So it seems like node-pre-gyp failed to find node-gyp in all the likely places and had to fall back to just calling it without using a specific path and just hoping it was there per (so this line was hit). And that failed.

So, overall this seems to indicate something wrong with your nodejs install. I notice your node install is /Users/annmillspaugh/.nvm/versions/node/v6.0.0/bin/node. Node v6.0.0 was released ~ 2 years ago (https://nodejs.org/en/blog/release/v6.0.0/). So, maybe try upgrading to the latest node v6 version (v6.14.0) and see if the problem persists?

@mapsam
Copy link
Contributor

mapsam commented Mar 29, 2018

Good eye @springmeyer. Sorry I didn't think about that @millzpaugh! Can confirm make works locally for me with the following versions:

  • node: v6.11.1
  • npm: 5.6.0

AND

  • node: v4.8.4
  • npm: 2.15.11

Copy link
Contributor

@mapsam mapsam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@millzpaugh @springmeyer I just updated with master and things are looking 👍 - feel free to merge this in whenever you are ready!

@springmeyer
Copy link
Contributor

Thanks for rebasing @mapsam! I recall there were a few changes in node-cop-skel that we landed after working on this branch so before merging we’ll want to do a quick diff of the key build files to ensure this is using the latest skel patterns exactly.

@mapsam
Copy link
Contributor

mapsam commented Jun 6, 2018

@springmeyer good point - let me know if you'd like me to look through that or if you/@millzpaugh plan on taking care of it!

@springmeyer
Copy link
Contributor

@mapsam I'm inclined to pause on this and come up with a plan for porting all our modules to mason-js, and hitting this then. So, I'm going to close this for now.

@springmeyer springmeyer closed this Jun 7, 2018
@mapsam
Copy link
Contributor

mapsam commented Jun 8, 2018

👍 good call - thanks for closing!

@mapsam mapsam deleted the mason-js-port branch June 8, 2018 08:23
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.

3 participants