-
Notifications
You must be signed in to change notification settings - Fork 166
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
Build failing for node v6.5.0 on windows #691
Comments
Oh, this is likely because I'm using upstream node directly and not our fork. Since we've usually changed the runtime to shared in our patches.... like https://github.com/mapbox/node/pull/9/files. Now, for node v4/5 I think I did not change this, or it looks like I may have missed it. So I forgot about it for node v6. So, perhaps we do still need to fork and build with Looking through the node v6 build scripts it looks like there is /cc @BergWerkGIS |
hmm, after mapbox/node-cpp11@ef06f92 and restarting the appveyor job I'm still seeing the same error... @BergWerkGIS help :) |
made some progress today. Detailed at mapnik/mapnik#3510 (comment). But now blocked on this error I'm unsure the cause of:
|
This error check has been disabled thus masking that the build has failed and |
WRT Patching the call to I suggest we carry the patch and the props file over to https://github.com/mapbox/node-cpp11/blob/master/windows/build_node.bat |
The directory structure for
|
|
A hint that failing |
Mixing compiler/runtime versions is generally a bad thing and should be avoided (*). To be investigated:
Next steps:
(*) Additional reads about compiler versions and mixing |
next action here is to modify the node-cpp11 builds to use the right directory structure for libs and headers that new node-gyp expects. That is the core problem. That was manually fixed by @BergWerkGIS for node v4/5. We should return to this in 2017 and fix node-cpp11 to build the right structure for >= v4 and up automatically. @BergWerkGIS let's hit this in 2017. |
Update: got a branch that confirms what we've seen above: moving back to stock node.exe works (as far as builds and tests) when the Next action is to followup on @BergWerkGIS's next steps above:
I vote for the shortcut first :) @BergWerkGIS let's sync up in the morning on this. |
hm, hitting a wall with λ npm install mapnik@3.6.0 npm WARN package.json investigate-gpx@0.0.1 No license field. - > mapnik@3.6.0 install c:\mb\investigate-gpx\node_modules\mapnik > node-pre-gyp install --fallback-to-build node-pre-gyp ERR! Tried to download(undefined): https://mapbox-node-binary.s3.amazonaws.com/mapnik/v3.6.0/node-v46-win32-x64-Release.tar.gz node-pre-gyp ERR! Pre-built binaries not found for mapnik@3.6.0 and node@4.8.1 (node-v46 ABI) (falling back to source compile with node-gyp) But binary package (https://mapbox-node-binary.s3.amazonaws.com/mapnik/v3.6.0/node-v46-win32-x64-Release.tar.gz) does exist. |
BTW, same with λ npm install gdal npm WARN package.json investigate-gpx@0.0.1 No license field. - > gdal@0.9.4 preinstall c:\mb\investigate-gpx\node_modules\gdal > npm install node-pre-gyp npm WARN engineStrict Per-package engineStrict (found in this package's package.json) npm WARN engineStrict won't be used in npm 3+. Use the config setting `engine-strict` instead. node-pre-gyp@0.6.34 node_modules\node-pre-gyp ├── semver@5.3.0 ├── nopt@4.0.1 (abbrev@1.1.0, osenv@0.1.4) ├── rc@1.2.1 (ini@1.3.4, strip-json-comments@2.0.1, deep-extend@0.4.1, minimist@1.2.0) ├── mkdirp@0.5.1 (minimist@0.0.8) ├── tar-pack@3.4.0 (uid-number@0.0.6, once@1.4.0, debug@2.6.3, fstream@1.0.11, readable-stream@2.2.9, fstream-ignore@1.0.5) ├── rimraf@2.6.1 (glob@7.1.1) ├── npmlog@4.0.2 (set-blocking@2.0.0, console-control-strings@1.1.0, gauge@2.7.3, are-we-there-yet@1.1.2) ├── tar@2.2.1 (inherits@2.0.3, block-stream@0.0.9, fstream@1.0.11) └── request@2.81.0 (aws-sign2@0.6.0, forever-agent@0.6.1, oauth-sign@0.8.2, tunnel-agent@0.6.0, is-typedarray@1.0.0, caseless@0.12.0, stringstream@0.0.5, safe-buffer@5.0.1, aws4@1.6.0, isstream@0.1.2, json-stringify-safe@5.0.1, extend@3.0.0, performance-now@0.2.0, uuid@3.0.1, qs@6.4.0, combined-stream@1.0.5, mime-types@2.1.15, tough-cookie@2.3.2, form-data@2.1.4, hawk@3.1.3, http-signature@1.1.1, har-validator@4.2.1) > gdal@0.9.4 install c:\mb\investigate-gpx\node_modules\gdal > node-pre-gyp install --fallback-to-build node-pre-gyp ERR! Tried to download(undefined): https://mapbox-node-binary.s3.amazonaws.com/gdal/v0.9.4/node-v46-win32-x64.tar.gz node-pre-gyp ERR! Pre-built binaries not found for gdal@0.9.4 and node@4.8.1 (node-v46 ABI) (falling back to source compile with node-gyp) |
@BergWerkGIS can you try passing |
Finally got node-mapnik-bench running. On my laptop:
results in {
"source": "c:\\mb\\node-mapnik-bench\\testcases\\geojson\\us-counties-polygons.geojson",
"version": "v3.6.0",
"options": {
"maxzoom": 12
},
"time": {
"start": 1492707090468,
"xml": 1492707091320,
"bridge": 1492707091398,
"info": 1492707091398,
"load": 1492707091399,
"copy": 1492707189108
},
"sink": "noop://",
"memory": {
"max_rss": "93.54MB",
"max_heap": "39.49MB",
"max_heap_total": "57.25MB"
},
"tile_count": 312008
} Memory usage gradually increased to ~70MB for |
Awesome results @BergWerkGIS! What exact version of node.exe was this with? |
|
Awesome and surprising @BergWerkGIS. I presume that node.exe links totally different CRT libs (and library names) since it is built with vs 2013. Do you get the same positive result with node v6? As far as next steps, I'm thinking your results from node-mapnik-bench are all we need to move forward with this plan besides a gut check on what exactly the |
Finally made node-mapnik-bench work with /cc @mapsam probably a blue sky task? Some of the things I had to do to get going (from the top of my head, unfortunately I didn't take notes):
Maybe some of the above problems also stem from Anyway
yielded: {
"source": "C:\\b\\testcases\\geojson\\us-counties-polygons.geojson",
"version": "v3.6.0",
"options": {
"maxzoom": 12
},
"time": {
"start": 1493024100177,
"xml": 1493024100958,
"bridge": 1493024101015,
"info": 1493024101016,
"load": 1493024101016,
"copy": 1493024152762
},
"sink": "noop://",
"memory": {
"max_rss": "76.71MB",
"max_heap": "23.94MB",
"max_heap_total": "44.04MB"
},
"tile_count": 312007
} I didn't notice any weird memory usage related behavior in |
I dug and found this can be reviewed by following nodejs/node#9385 (comment) to nodejs/node@527db40...e97723b, which is a backport of nodejs/node#7487 (and a few other followups that landed in master) My understanding is:
While unconventional, this appears to work great and proves that, at least in the case of node addons like node-mapnik, the boundary crossing issues described at https://www.softwariness.com/articles/visual-cpp-runtime-libraries/ do not apply (nothing like std::string is allocated in node and deallocated by node-mapnik). |
To recap:
Working on getting this working at #758 |
Been trying to get node v6 working on windows. What I've done is:
Errors are new and curious:
The text was updated successfully, but these errors were encountered: