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

node-pty doesn't build on Node 12 (V8 7.4) #279

Closed
machellerogden opened this issue Apr 24, 2019 · 13 comments · Fixed by #288
Closed

node-pty doesn't build on Node 12 (V8 7.4) #279

machellerogden opened this issue Apr 24, 2019 · 13 comments · Fixed by #288
Assignees
Labels
debt help wanted Issues identified as good community contribution opportunities
Milestone

Comments

@machellerogden
Copy link

Environment details

  • OS: macOS
  • OS version: 10.14.3
  • node-pty version: 0.8.1

Issue description

In node 10, we saw deprecation notices during the build but node-pty still worked.

As of today's release of Node 12, node-pty no longer builds.

I will take a stab at an update and submit a PR if I have any luck.

21:29 $ npm i node-pty@0.8.1

> node-pty@0.8.1 install /Users/mogden/repos/github.com/machellerogden/node-pty-test/node_modules/node-pty
> node scripts/install.js

  CXX(target) Release/obj.target/pty/src/unix/pty.o
In file included from ../src/unix/pty.cc:20:
In file included from ../../nan/nan.h:222:
In file included from ../../nan/nan_converters.h:67:
../../nan/nan_converters_43_inl.h:22:1: warning: 'ToBoolean' is deprecated: ToBoolean can never throw. Use Local version. [-Wdeprecated-declarations]
X(Boolean)
^
../../nan/nan_converters_43_inl.h:18:12: note: expanded from macro 'X'
      val->To ## TYPE(isolate->GetCurrentContext())                            \
           ^
<scratch space>:23:1: note: expanded from here
ToBoolean
^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2523:3: note: 'ToBoolean' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("ToBoolean can never throw. Use Local version.",
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
In file included from ../src/unix/pty.cc:20:
In file included from ../../nan/nan.h:222:
In file included from ../../nan/nan_converters.h:67:
../../nan/nan_converters_43_inl.h:40:1: warning: 'BooleanValue' is deprecated: BooleanValue can never throw. Use Isolate version. [-Wdeprecated-declarations]
X(bool, Boolean)
^
../../nan/nan_converters_43_inl.h:37:15: note: expanded from macro 'X'
  return val->NAME ## Value(isolate->GetCurrentContext());                     \
              ^
<scratch space>:30:1: note: expanded from here
BooleanValue
^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2561:3: note: 'BooleanValue' has been explicitly marked deprecated here
  V8_DEPRECATED("BooleanValue can never throw. Use Isolate version.",
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:307:29: note: expanded from macro 'V8_DEPRECATED'
  declarator __attribute__((deprecated(message)))
                            ^
In file included from ../src/unix/pty.cc:20:
In file included from ../../nan/nan.h:223:
In file included from ../../nan/nan_new.h:189:
../../nan/nan_implementation_12_inl.h:356:37: error: too few arguments to function call, expected 2, have 1
  return v8::StringObject::New(value).As<v8::StringObject>();
         ~~~~~~~~~~~~~~~~~~~~~      ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:5380:3: note: 'New' declared here
  static Local<Value> New(Isolate* isolate, Local<String> value);
  ^
In file included from ../src/unix/pty.cc:20:
In file included from ../../nan/nan.h:223:
In file included from ../../nan/nan_new.h:189:
../../nan/nan_implementation_12_inl.h:356:58: error: expected '(' for function-style cast or type construction
  return v8::StringObject::New(value).As<v8::StringObject>();
                                         ~~~~~~~~~~~~~~~~^
../../nan/nan_implementation_12_inl.h:356:60: error: expected expression
  return v8::StringObject::New(value).As<v8::StringObject>();
                                                           ^
In file included from ../src/unix/pty.cc:20:
In file included from ../../nan/nan.h:2722:
../../nan/nan_object_wrap.h:24:25: error: no member named 'IsNearDeath' in 'Nan::Persistent<v8::Object, v8::NonCopyablePersistentTraits<v8::Object> >'
    assert(persistent().IsNearDeath());
           ~~~~~~~~~~~~ ^
/usr/include/assert.h:93:25: note: expanded from macro 'assert'
    (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0)
                        ^
In file included from ../src/unix/pty.cc:20:
In file included from ../../nan/nan.h:2722:
../../nan/nan_object_wrap.h:127:26: error: no member named 'IsNearDeath' in 'Nan::Persistent<v8::Object, v8::NonCopyablePersistentTraits<v8::Object> >'
    assert(wrap->handle_.IsNearDeath());
           ~~~~~~~~~~~~~ ^
/usr/include/assert.h:93:25: note: expanded from macro 'assert'
    (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0)
                        ^
../src/unix/pty.cc:154:39: error: no matching member function for call to 'ToString'
  v8::String::Utf8Value file(info[0]->ToString());
                             ~~~~~~~~~^~~~~~~~
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2528:44: note: candidate function not viable: requires single argument 'context', but no arguments were provided
  V8_WARN_UNUSED_RESULT MaybeLocal<String> ToString(
                                           ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2544:35: note: candidate function not viable: requires single argument 'isolate', but no arguments were provided
                    Local<String> ToString(Isolate* isolate) const);
                                  ^
../src/unix/pty.cc:165:69: error: no matching member function for call to 'ToString'
    v8::String::Utf8Value arg(argv_->Get(Nan::New<v8::Integer>(i))->ToString());
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2528:44: note: candidate function not viable: requires single argument 'context', but no arguments were provided
  V8_WARN_UNUSED_RESULT MaybeLocal<String> ToString(
                                           ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2544:35: note: candidate function not viable: requires single argument 'isolate', but no arguments were provided
                    Local<String> ToString(Isolate* isolate) const);
                                  ^
../src/unix/pty.cc:165:38: warning: 'Get' is deprecated: Use maybe version [-Wdeprecated-declarations]
    v8::String::Utf8Value arg(argv_->Get(Nan::New<v8::Integer>(i))->ToString());
                                     ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:3412:3: note: 'Get' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version", Local<Value> Get(Local<Value> key));
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
../src/unix/pty.cc:176:69: error: no matching member function for call to 'ToString'
    v8::String::Utf8Value pair(env_->Get(Nan::New<v8::Integer>(i))->ToString());
                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2528:44: note: candidate function not viable: requires single argument 'context', but no arguments were provided
  V8_WARN_UNUSED_RESULT MaybeLocal<String> ToString(
                                           ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2544:35: note: candidate function not viable: requires single argument 'isolate', but no arguments were provided
                    Local<String> ToString(Isolate* isolate) const);
                                  ^
../src/unix/pty.cc:176:38: warning: 'Get' is deprecated: Use maybe version [-Wdeprecated-declarations]
    v8::String::Utf8Value pair(env_->Get(Nan::New<v8::Integer>(i))->ToString());
                                     ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:3412:3: note: 'Get' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use maybe version", Local<Value> Get(Local<Value> key));
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:322:29: note: expanded from macro 'V8_DEPRECATE_SOON'
  declarator __attribute__((deprecated(message)))
                            ^
../src/unix/pty.cc:181:39: error: no matching member function for call to 'ToString'
  v8::String::Utf8Value cwd_(info[3]->ToString());
                             ~~~~~~~~~^~~~~~~~
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2528:44: note: candidate function not viable: requires single argument 'context', but no arguments were provided
  V8_WARN_UNUSED_RESULT MaybeLocal<String> ToString(
                                           ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2544:35: note: candidate function not viable: requires single argument 'isolate', but no arguments were provided
                    Local<String> ToString(Isolate* isolate) const);
                                  ^
../src/unix/pty.cc:186:39: error: too few arguments to function call, single argument 'context' was not specified
  winp.ws_col = info[4]->IntegerValue();
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:187:39: error: too few arguments to function call, single argument 'context' was not specified
  winp.ws_row = info[5]->IntegerValue();
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:195:16: error: no matching member function for call to 'ToBoolean'
  if (info[8]->ToBoolean()->Value()) {
      ~~~~~~~~~^~~~~~~~~
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2524:63: note: candidate function not viable: requires single argument 'context', but no arguments were provided
                    V8_WARN_UNUSED_RESULT MaybeLocal<Boolean> ToBoolean(
                                                              ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2540:18: note: candidate function not viable: requires single argument 'isolate', but no arguments were provided
  Local<Boolean> ToBoolean(Isolate* isolate) const;
                 ^
../src/unix/pty.cc:230:35: error: too few arguments to function call, single argument 'context' was not specified
  int uid = info[6]->IntegerValue();
            ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:231:35: error: too few arguments to function call, single argument 'context' was not specified
  int gid = info[7]->IntegerValue();
            ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:315:39: error: too few arguments to function call, single argument 'context' was not specified
  winp.ws_col = info[0]->IntegerValue();
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:316:39: error: too few arguments to function call, single argument 'context' was not specified
  winp.ws_row = info[1]->IntegerValue();
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:360:34: error: too few arguments to function call, single argument 'context' was not specified
  int fd = info[0]->IntegerValue();
           ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:363:39: error: too few arguments to function call, single argument 'context' was not specified
  winp.ws_col = info[1]->IntegerValue();
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/unix/pty.cc:364:39: error: too few arguments to function call, single argument 'context' was not specified
  winp.ws_row = info[2]->IntegerValue();
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8.h:2565:3: note: 'IntegerValue' declared here
  V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
  ^
/Users/mogden/.node-gyp/12.0.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
4 warnings and 20 errors generated.
make: *** [Release/obj.target/pty/src/unix/pty.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/mogden/n/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:196:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:256:12)
gyp ERR! System Darwin 18.2.0
gyp ERR! command "/Users/mogden/n/bin/node" "/Users/mogden/n/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /Users/mogden/repos/github.com/machellerogden/node-pty-test/node_modules/node-pty
gyp ERR! node -v v12.0.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok
npm WARN node-pty-test@1.0.0 No description
npm WARN node-pty-test@1.0.0 No repository field.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! node-pty@0.8.1 install: `node scripts/install.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the node-pty@0.8.1 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/mogden/.npm/_logs/2019-04-24T02_29_19_741Z-debug.log
@Tyriar
Copy link
Member

Tyriar commented Apr 24, 2019

Looks like nan needs to be upgraded? Feel free to do a PR for this, I likely won't look into it until Electron reaches node 12

@Tyriar Tyriar added debt help wanted Issues identified as good community contribution opportunities labels Apr 24, 2019
@gpetrov
Copy link

gpetrov commented Apr 26, 2019

@Tyriar The just released Electron 5 has already node 12 in it:
https://electronjs.org/blog/electron-5-0

@Tyriar
Copy link
Member

Tyriar commented Apr 27, 2019

@gpetrov yep, it'll likely take some time for VS Code to get exploration builds together for v5. Again feel free to look into this if you need it faster than us.

@gpetrov
Copy link

gpetrov commented Apr 27, 2019

thanks @Tyriar, I will see what I can do. We are using node-pty on NWJS and it is much further than Electron. Already on Chrome 74 and Node 12.

I also noticed:

..\src\win\conpty.cc(249): error C2660: 'v8::Function::Call': function does not take 3 arguments [xxx\node_modules\node-pty\build\conpty.vcxproj]

Thanks for you great work btw - it helped us a lot to get a full functional terminal on Windows.

@Eugeny
Copy link
Contributor

Eugeny commented Apr 28, 2019

I've got it to compile in Eugeny@cb3b3f6, but there's a weird problem that seems to be related to calling exitCallback - all related tests are failing with Uncaught TypeError: number 0 is not a function and there's no stack trace.

Using Nan::Callback instead just crashes the process.

@gpetrov
Copy link

gpetrov commented Apr 28, 2019

Looking great @Eugeny !! Seems you've done all the hard work. Maybe @Tyriar can advise about the tests and the crash - looks like a small thing to fix but have to know the core of the failure.

@Tyriar
Copy link
Member

Tyriar commented Apr 29, 2019

@Eugeny hmm, nothing standing out as obviously wrong from the diff, the error sounds like exitCallback on the native side is being assigned to 0 instead of the callback, you didn't even change the line that pulls that from info[4] though.

@Eugeny
Copy link
Contributor

Eugeny commented Apr 29, 2019

@Tyriar I've checked ->IsFunction() just before the call and it was fine - the execution even proceeds beyond the ->Call() line, and the error message comes later, but the actuall callback is never executed.

I've tried saving the v8::Context with the callback just in case, to no avail

@Eugeny
Copy link
Contributor

Eugeny commented May 3, 2019

TBH I'm at loss here @Tyriar - spent a few hours tinkering with the callback and still got nothing. I'm not nearly as deep in Node native development, so there must be something obvious I'm missing

@Eugeny Eugeny mentioned this issue May 4, 2019
@Eugeny
Copy link
Contributor

Eugeny commented May 4, 2019

Found it - weirdly enough it was the uninitialized PSIZE_T in the same function

@fczuardi
Copy link

fczuardi commented Jun 1, 2019

I have tried to install node-pty using node 12.3.1 and 12.0.0, neither worked, with node 11.15.0 it works...

should I reopen this issue or file a new one?

@Tyriar
Copy link
Member

Tyriar commented Jun 1, 2019

Responded on your #319

clems4ever added a commit to criteo/mesos-term that referenced this issue Jul 12, 2019
node-pty does not build with v12 because of microsoft/node-pty#279
@xeroxstar
Copy link

xeroxstar commented Oct 20, 2019

I had same errors on my console but the code is working fine after the install is done and i am using node v10.13.0. I run this example just fine:

var os = require('os');
var pty = require('node-pty');

var shell = os.platform() === 'win32' ? 'powershell.exe' : 'bash';
var ptyProcess = pty.spawn(shell, ['vue create app'], {
    name: 'vue',
    cwd: process.env.HOME,
    env: process.env
});
ptyProcess.write('keydown');
ptyProcess.on('data', function(data) {
    process.stdout.write(data.toString().trim());
});

the output:
Vue CLI v3.11.0
┌───────────────────────────┐
│ Update available: 4.0.4 │
└───────────────────────────┘
? Please pick a preset: (Use arrow keys)

default (babel, eslint)
Manually select features? Please pick a preset:
default (babel, eslint)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt help wanted Issues identified as good community contribution opportunities
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants