GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Failed to compile on Node 10.0.0 · Issue #1490 · nodegit/nodegit

Failed to compile on Node 10.0.0 #1490

New issue

Copy link

New issue

Copy link

Closed

Failed to compile on Node 10.0.0#1490

Copy link

Description

sadasant

opened

on Apr 30, 2018

Issue body actions

nodegit fails to compile on Node 10.0.0 and NPM 5.6.0. Here's a way to reproduce the issue:

cd ~
mkdir test_repo
cd test_repo
npm init
npm i --save nodegit

Here are my logs:

https://gist.github.com/sadasant/4fce501d8cffda53bc8fb53f4950eaa3

Also happens with NPM 6.0.0:

https://gist.github.com/sadasant/cf598e5d232be909e426135a3de87847

May 1 update

Even though nodegit's issue is deeper that what I'm about to mention, the following issue is the first breaking point for building nodegit on Node 10:

There was a recent change on Node 10's Function's toString method. Here's an issue that I created: nodejs/node#20459

This change in Node 10 causes promisify-node to break, here's the relevant issue: nodegit/promisify-node#28

May 2 update

The exception happens when we try to run node node_modules/.bin/node-pre-gyp install --build-from-source. The issue seems to be that libssh2 is expected to be built using openssl-0.9.8: https://github.com/libssh2/libssh2/search?utf8=%E2%9C%93&q=openssl-0.9.8&type= However, node 10 moved openssl forward to 1.1.0: https://github.com/nodejs/node/search?utf8=%E2%9C%93&q=OpenSSL+has+been+updated+to+1.1.0h.&type=

Here are my logs: https://gist.github.com/sadasant/031d3f8bf1c4e5d2ac83b3841b7807fc

Reacted by Andy Fleming, Ali Taheri Moghaddar, Marie, Dimitar Shishkov, Ilian Alexiev, KillWolfVlad, Nathan Hardy, Rohit Gohri, Jacob Walser, Silas Rech and 19 more

Activity

sadasant commented on May 2, 2018
sadasant
on May 2, 2018
Author
More actions
So far, the issues seem to be:

promisify-node having a faulty RegExp.

nodegit's dependency on libssh2, which expects to be built on top of openssl 0.9.8, when Node 10 moved to openssl 1.1.0.
sadasant commented on May 2, 2018
sadasant
on May 2, 2018
Author
More actions
The relevant issues are:

For promisify-node: Node 10.0.0 Function 's toString method behaves differently than in previous versions promisify-node#28

For libssh2: Does this package supports openssl 1.1.0 ? libssh2/libssh2#250
DanielRuf commented on May 7, 2018
DanielRuf
on May 7, 2018
More actions
Same here.
RobinGeuze commented on May 9, 2018
RobinGeuze
on May 9, 2018
More actions
For the libssh2 part an upgrade to version 1.8.0 of the library should fix it. That version is listed on their site and is tagged in github, but it doesn't have a github release for some reason.
Reacted by ikeyan
MartinHelmut
added 2 commits that reference this issue on May 13, 2018
ci: for the time being, latest node build are deactivated
9f63e74
ci: for the time being, latest node build are deactivated
c333396
getify commented on May 19, 2018
getify
on May 19, 2018
More actions
Many of us have upgraded to node10 and now this is a huge blocker. Please increase the priority of fixing this build failure.
Reacted by Jacob Walser, Marie, Chris Wilson, Sajad Ghawami, Vitalii Vorobioff, Silas Rech, Daniel Schmidt, Riccardo Messineo, Dominik Widomski, ikeyan and 22 more
laomaiweng commented on May 31, 2018
laomaiweng
on May 31, 2018
More actions
libssh2 should build fine with nodegit's bundled OpenSSL 1.0.2.

However with Node 10 and OpenSSL 1.1.0, the build system fails to pick up the bundled OpenSSL 1.0.2 headers when building libssh2, and ends up using node-gyp's OpenSSL 1.1.0 headers in ~/.node-gyp/10.1.0/include/node/openssl. This is because node-gyp seems to prepend -I $(HOME)/.node-gyp/10.1.0/include/node to the CFLAGS when building, ignoring the -I ../vendor/openssl/openssl/include added by the nodegit build system.

If we could somehow instruct node-gyp to actually prepend -I ../vendor/openssl/openssl/include to the include dirs, before -I $(HOME)/.node-gyp/…, then libssh2 would pick up the compatible 1.0.2 headers. However I tried with include_dirs+ as suggested in the node-gyp reference, without luck. :(
Reacted by Jacob Walser
tbranyen commented on Jun 5, 2018
tbranyen
on Jun 5, 2018
Member
More actions
I've resolved the promisify-node blocker, and will investigate the compiler issue later today.
tbranyen commented on Jun 5, 2018
tbranyen
on Jun 5, 2018
Member
More actions
@laomaiweng that definitely is the fix for this particular issue. I tried using include_dirs! which looks like it should override the includes completely, but does not. I moved this block to the root of our GYP file and it still does not take. I have moved past this issue, by forking node-gyp and conditionally setting include_dirs behind a variable.

The next issue was with ToInt32 not being in v8 anymore. I've moved past that issue using Nan and now have a compiled version against Node 10. Here's what happens if I try and require it:

λ node > require('.') zsh: segmentation fault (core dumped) node

So now the fun times of figuring out why....

tbranyen commented on Jun 5, 2018

tbranyen

on Jun 5, 2018

Member

More actions

λ valgrind node -e 'require(".")'
==28122== Memcheck, a memory error detector
==28122== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==28122== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==28122== Command: node -e require(".")
==28122== 
==28122== Warning: set address range perms: large range [0x4cb4ce34000, 0x4cb6ce34000) (noaccess)
==28122== Invalid write of size 4
==28122==    at 0x5D63810: SSL_CTX_set_verify (in /usr/lib/libssl.so.1.1)
==28122==    by 0xD4258E3: git_openssl_stream_global_init (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0xD3FFDAF: init_once (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0x7508DEE: __pthread_once_slow (in /usr/lib/libpthread-2.26.so)
==28122==    by 0xD3FFE92: git_libgit2_init (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0xD1FC079: init (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0x568ABC: ??? (in /usr/bin/node)
==28122==    by 0x72A4AF: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) (in /usr/bin/node)
==28122==    by 0x78280F: ??? (in /usr/bin/node)
==28122==    by 0x782BE6: ??? (in /usr/bin/node)
==28122==    by 0x4CB4CE8427C: ???
==28122==    by 0x4CB4CE944F6: ???
==28122==  Address 0x140 is not stack'd, malloc'd or (recently) free'd
==28122== 
==28122== 
==28122== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==28122==  Access not within mapped region at address 0x140
==28122==    at 0x5D63810: SSL_CTX_set_verify (in /usr/lib/libssl.so.1.1)
==28122==    by 0xD4258E3: git_openssl_stream_global_init (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0xD3FFDAF: init_once (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0x7508DEE: __pthread_once_slow (in /usr/lib/libpthread-2.26.so)
==28122==    by 0xD3FFE92: git_libgit2_init (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0xD1FC079: init (in /home/tbranyen/git/nodegit/build/Release/nodegit.node)
==28122==    by 0x568ABC: ??? (in /usr/bin/node)
==28122==    by 0x72A4AF: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) (in /usr/bin/node)
==28122==    by 0x78280F: ??? (in /usr/bin/node)
==28122==    by 0x782BE6: ??? (in /usr/bin/node)
==28122==    by 0x4CB4CE8427C: ???
==28122==    by 0x4CB4CE944F6: ???

tbranyen commented on Jun 5, 2018
tbranyen
on Jun 5, 2018
Member
More actions
Judging by /usr/lib/libssl.so being listed in that trace, I'm guessing our statically linked vendor deps are not being respected and this may be causing the issue.
tbranyen commented on Jun 5, 2018
tbranyen
on Jun 5, 2018
Member
More actions
Tonight I'm going to try and use a GYP file for libssh2 like (https://github.com/peters/curl-for-windows/blob/master/libssh2.gyp) and mark it as a dependency to the nodegit target.
implausible commented on Jun 6, 2018
implausible
on Jun 6, 2018
Member
More actions
@tbranyen Take a look at what I did here, this might speed up the process https://github.com/implausible/nodegit/blob/remove/openssl/vendor/libgit2.gyp#L483
lenovouser commented on Jun 21, 2018
lenovouser
on Jun 21, 2018
More actions
Any progress?
11 remaining items
tbranyen commented on Sep 19, 2018
tbranyen
on Sep 19, 2018
Member
More actions
Testing this out, and if true, I'll ensure it gets merged today.
Reacted by Anders Evenrud and Mehdi Yeganeh
tbranyen commented on Sep 19, 2018
tbranyen
on Sep 19, 2018
Member
More actions
Okay so here's what I see. This does in fact get the build to work, excellent and minimal set of steps to do it too! But, I'm seeing a number of test failures. From what I can tell they are all related to promisify-node with the exception: Error: Callback is required and must be a Function.. So we'll need to resolve those errors first.

Then there's the whole appveyor build passing we'll need to get through, and hopefully after all that we'll have a stable Node 10 version.
Croydon
added a commit that references this issue on Sep 19, 2018
Patch LockMasterSetStatus
e56b722
tbranyen commented on Sep 19, 2018
tbranyen
on Sep 19, 2018
Member
More actions
@Croydon do you wanna open the PR? I could help you with it? I see you're already working on something.

Edit: Nevermind found it and it's referenced above: #1519
Croydon
added 2 commits that reference this issue on Sep 19, 2018
Patch LockMasterSetStatus
7f4fd05
Patch LockMasterSetStatus
5547fc1
Croydon commented on Sep 20, 2018
Croydon
on Sep 20, 2018
Contributor
More actions
@myshark On what platform(s) did you try it? AppVeyor (Windows) keeps failing (see my pull request)
Croydon
added a commit that references this issue on Sep 20, 2018
Patch LockMasterSetStatus
e14aa05
mehdi-yeganeh commented on Sep 20, 2018
mehdi-yeganeh
on Sep 20, 2018
More actions
@Croydon, Wow you are so fast guys, I cloned https://github.com/Croydon/nodegit.git and tested it on MacOS (Sierra 10.12.6) and CentOS (7) both is ok and worked. If you need some help, i`m here.
Croydon
added 3 commits that reference this issue on Sep 25, 2018
Patch LockMasterSetStatus
fa48c61
Patch LockMasterSetStatus
1b65147
Patch LockMasterSetStatus
63e31e6
implausible
closed this as completedin #1519 on Oct 2, 2018
MartinHelmut
added a commit that references this issue on Feb 4, 2019
ci: for the time being, latest node build are deactivated
a9eae5d
smithmr commented on Jun 17, 2019
smithmr
on Jun 17, 2019
More actions
I had this issue for a couple of days and fixed it by running this npm config set python {path to python2.7}. I was running with node 10.

Metadata

Assignees

No one assigned

Labels

No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Failed to compile on Node 10.0.0 #1490

Description

May 1 update

May 2 update

Activity

sadasant commented on May 2, 2018

sadasant commented on May 2, 2018

DanielRuf commented on May 7, 2018

RobinGeuze commented on May 9, 2018

getify commented on May 19, 2018

laomaiweng commented on May 31, 2018

tbranyen commented on Jun 5, 2018

tbranyen commented on Jun 5, 2018

tbranyen commented on Jun 5, 2018

tbranyen commented on Jun 5, 2018

tbranyen commented on Jun 5, 2018

implausible commented on Jun 6, 2018

lenovouser commented on Jun 21, 2018

11 remaining items

tbranyen commented on Sep 19, 2018

tbranyen commented on Sep 19, 2018

tbranyen commented on Sep 19, 2018

Croydon commented on Sep 20, 2018

mehdi-yeganeh commented on Sep 20, 2018

smithmr commented on Jun 17, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions