Node support #93

facekapow · 2016-01-24T16:45:36Z

Adds a lot more support for node. Specifically: dns.js (basic, only A records), console.js, process.stdout, process.stderr, stream.js. Couple of notes:

I had to implement stream.js myself because stream-browserify kept crapping out and throwing ReferenceErrors everywhere.
Also about stream.js: I have to implement stream.Transform.
Again, stream.js: I had to copy-paste for stream.Duplex since it inherits both stream.Readable and stream.Writable (if anyone knows how to do multiple inheritance in ES6, please let me know how).
I threw in Promises for some of the functions that take callbacks (they still accept callbacks, though). I figure it's not gonna be long until Node uses them, however, should they just be left as plain old callbacks? If I leave them, should I put Promises on EventEmitter, too?

…derr

…s a hostname

…to node-support

vitkarpov · 2016-01-24T17:12:20Z

js/__loader.js

+      runtime.stdio.defaultStdio.writeError(String(chunk));
+      callback();
+    }
+  }


@facekapow why does this file contain es5 and es6 style both? looks odd. Is this okay?

You mean with the callbacks? That's because that's the way the internal API is made in Node (plus, there's really no way to Promise that). It's fine.

Wait, sorry, I think I misunderstood you. The original file by @iefserge was ES5, and my additions were in ES6. It works fine, though.

Actually I mean that this changes are in es6 style while the whole file is in es5. Just a note. That's ok, I suppose.

Yep, that's right :)

yeah, I think it's fine, might make sense to upgrade the loader to ES6 at some point too https://github.com/runtimejs/runtime-module-loader

facekapow · 2016-01-24T17:21:38Z

I was able to create a working TCP echo server using this branch:

'use strict';

const runtime = require('runtimejs');
const net = require('net');

const server = net.createServer((socket) => {
  console.log('client...');
  socket.on('end', () => console.log('client end...'));
  socket.write('hello...\r\n');
  socket.pipe(socket);
});
server.listen(9000, () => console.log('echo server online...'));

iefserge · 2016-01-24T17:27:26Z

This is awesome! I'm looking through code and going to leave my comments.

1,2. There is a https://www.npmjs.com/package/readable-stream package, that's basically node streams in userland (stream-browserify uses it internally (https://github.com/substack/stream-browserify/blob/master/index.js#L28-L32), maybe we could use that instead? I think implementation of streams is ok, but it's going to be much harder to make sure it works the same way as in node.
3. As far as I know, multiple inheritance is not supported by ES6 extends construct. Something like Object.assign for prototype?
4. I'd say we shouldn't modify or extend any Node APIs, to avoid any compatibility issues.

facekapow · 2016-01-24T18:22:12Z

I tried replacing ./modules/stream.js with readable-stream (and of course, I installed the package), which gave me another error:

Uncaught exception: /node_modules/runtimejs/modules/inherits.js:24: TypeError: Object prototype may only be an Object or null: undefined
TypeError: Object prototype may only be an Object or null: undefined
    at Object.inherits (/node_modules/runtimejs/modules/inherits.js:24:27)
    at /node_modules/runtimejs/node_modules/readable-stream/lib/_stream_readable.js:62:6
    at /node_modules/runtimejs/node_modules/readable-stream/lib/_stream_readable.js:976:3
    at Module.require (__loader:73:9)
    at /node_modules/runtimejs/node_modules/readable-stream/readable.js:13:48
    at /node_modules/runtimejs/node_modules/readable-stream/readable.js:6:28
    at /node_modules/runtimejs/node_modules/readable-stream/readable.js:13:3
    at Module.require (__loader:73:9)
    at Loader.require (__loader:247:25)
    at __loader:290:25

So, for now, I'm going to stick with my implementation and just work on it to support the Node API.
I've removed the Promises from the API and just left the callbacks.

facekapow · 2016-01-24T19:35:05Z

@iefserge Thanks for readable-stream, the problem I was having was that directly replacing it in the __loader was causing errors.

iefserge · 2016-01-24T19:41:43Z

@facekapow ah, you have already seen that ;)

My PR might be broken. Managed to get readable-stream kinda working, there is an issue with the echo tcp server. It prints hello... successfully but doesn't echo anything after that. At the same time it works with your implementation perfectly, do you think it might be net-related issue?

I just feel like node streams are so complex and there are so many versions of them, so it's going to be a nightmare to support own implementation. wdyt?

facekapow · 2016-01-24T19:47:49Z

Yeah, I just noticed the problem, when I tested it after committing... :{ I'm going to have to roll back that commit for now. Honestly, I think our implementation would be fine. Any issues that popup would just be fixed. I don't think it'd be such a nightmare...

It's possible it might be net related (since _read does nothing, that might be it)...

iefserge · 2016-01-24T21:05:03Z

@facekapow one thing I noticed is that console.log prints to the tty instead of the console. default stdio should probably print to the console too.

Node support streams (try 2)

iefserge · 2016-02-06T23:47:33Z

@facekapow I think it would be fine to split http into separate PR, so we can merge this first faster

facekapow · 2016-03-23T15:27:39Z

Sorry it took me so long to update, but I finally removed http.js.

facekapow · 2016-03-24T12:50:23Z

I'm currently working on implementing the VM module.

facekapow · 2016-03-25T15:01:59Z

@iefserge I keep getting error: 'v8::Local<T>::Local(S*) [with S = void; T = v8::Context]' is private when trying to compile my code in native-object.cc. My code:

v8::Local<v8::Value> ctx_tmp;
auto maybe_val = arg1->ToObject()->GetPrivate(context, kCtx);
maybe_val.ToLocal(&ctx_tmp);
v8::Local<v8::External> ext = ctx_tmp.As<v8::External>();
v8::Local<v8::Context> ctx = static_cast(v8::Local<v8::Context>)(ext->Value()); // I keep getting the error here
// ext->Value() by itself is okay, it must be something to do with the v8::Local<v8::Context>

Any idea why this is happening? I've tried everything.

iefserge · 2016-03-25T15:18:40Z

@facekapow hmm, let's chat on gitter

facekapow · 2016-03-25T15:29:22Z

OK

facekapow · 2016-03-25T19:10:08Z

I think these are all the modules I'm gonna add for now. @iefserge If you want to add anymore, let me know. Otherwise, can this merge?

…d process.termout and process.termerr

facekapow · 2016-03-25T19:44:24Z

Apart from making process.stdout and process.stderr output to the QEMU console, I also added process.termout and process.termerr as overrides that go straight to the graphical console.

piranna · 2016-03-25T19:46:42Z

Apart from making process.stdout and process.stderr output to the QEMU console, I also added process.termout and process.termerr as overrides that go straight to the graphical console.

Could you be able to explain this a bit more?

iefserge · 2016-03-25T21:42:54Z

js/component/dns-client/dns-packet.js

      results.push({hostname: host, record: 'CNAME', name: readHostname(reader).join('.')});
      break;
+    case queries.MX:
+      console.log(reader.readUint8());


@facekapow debug?

Sorry, I forgot.

iefserge · 2016-03-25T21:51:41Z

@facekapow looks like console.log outputs extra \n

iefserge · 2016-03-25T21:54:37Z

@piranna there are two consoles

console that QEMU outputs to the terminal (works through serial port) - console.log prints here by default
graphical console in QEMU window

…cess.termerr, remove debug console.log

facekapow · 2016-03-25T22:41:06Z

@iefserge About the eshttp module you mentioned earlier, I think a wrapper around it would make a pretty good http module 👍.

piranna · 2016-03-25T23:01:25Z

How is that they are not the same?
El 25/3/2016 22:54, "Serge" notifications@github.com escribió:

@piranna https://github.com/piranna there are two consoles

console that QEMU outputs to the terminal (works through serial
port) - console.log prints here by default

graphical console in QEMU window

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#93 (comment)

facekapow · 2016-03-25T23:17:01Z

@piranna The QEMU serial port console is the one that outputs to your system terminal. The graphical console is the one created by runtime.js in the QEMU window.

piranna · 2016-03-25T23:19:59Z

@piranna The QEMU serial port console is the one that outputs to your system terminal. The graphical console is the one created by runtime.js in the QEMU window.

I understand the difference, but I don't understand why there are different... Maybe I'm too much used to deal with QEmu and Linux where you get one or the other...

iefserge · 2016-03-26T12:56:16Z

graphical console has a command prompt, useful for debugging.

iefserge · 2016-03-26T12:58:30Z

js/__loader.js

@@ -290,13 +290,17 @@
  const stream = loader.require('stream');
  class StdoutStream extends stream.Writable {
    _write(chunk, encoding, callback) {
-      __SYSCALL.log(String(chunk));
+      var out = String(chunk);
+      if (out.substring(out.length-1) === '\n') out = out.substring(0, out.length-1);


hm, I think this is an issue with __SYSCALL.log, it shouldn't add \n at the end

It is, but if you remove the newline from __SYSCALL.log, the output at startup is output on the same line.

maybe add a __SYSCALL.write?

facekapow · 2016-03-26T15:19:11Z

There's only one little thing. Should I add a runtime.console? An instance of Console with its output hooked up to the graphical console? Or just leave everything the way it is?

iefserge · 2016-03-26T15:29:07Z

Two consoles would be pretty confusing, I think graphical one is more like a debug tool. Should be fine as it is.

facekapow · 2016-03-26T15:32:45Z

Ok.

facekapow · 2016-03-26T15:35:08Z

Any last minute bugs or anything?

iefserge · 2016-03-26T15:45:23Z

@facekapow this looks great, very exciting! Tests coverage would be cool, but there are still a few issues with the test runner, so I'd say it's fine to merge now 👍
I'll try to figure out tests (and linting) later at some point.

facekapow · 2016-03-26T16:13:46Z

Bump kernel and JS versions?

iefserge · 2016-03-26T16:30:24Z

@facekapow not yet, I'm looking into Travis CI based automatic deployment, because prebuilt binaries need to be released too

facekapow · 2016-03-26T16:50:59Z

Deleted my last comment, it was completely wrong, sorry.

iefserge · 2016-03-26T17:19:22Z

@facekapow yeah, I think using releases would be fine.

facekapow · 2016-03-26T17:31:29Z

Oh, ok. Guess I wasn't completely wrong.
Comment I deleted:

If you deployed through Github Releases, you could just use Travis CI's Github Releases. All you have to do to support Github Releases is add the extra path components to the download URL in runtime-cli.

iefserge · 2016-03-28T14:15:35Z

@facekapow this has been released 👍
Also configured Travis CI to build releases and publish tags to npm automatically. runtime-cli major update required though.

facekapow · 2016-03-28T21:21:02Z

@iefserge Great! ~~When I get home I'll patch up runtime-cli.~~

edit: nevermind, you already did.

facekapow and others added 5 commits January 22, 2016 21:49

Add console.js, errors.js, dns.js (basic), process.stdout, process.st…

fa68016

…derr

Update dns.js - Minor error

9447e03

net.js, stream.js (TODO: stream.Transform), dns.js allows localhost a…

f11b32e

…s a hostname

Merge branch 'node-support' of http://github.com/runtimejs/runtime in…

5125a7e

…to node-support

Fix commits

8a09924

vitkarpov reviewed Jan 24, 2016
View reviewed changes

Remove Promises, update dependencies

714b3b6

facekapow and others added 5 commits January 24, 2016 13:24

Forgot arrow function in net.js

effcc61

Fix resolve, reject leftovers

3443920

Fix resolve, reject leftovers (again)

72aaafe

Fix E_CANNOT_CLOSE

fa55ecc

Use readable-stream

a0434db

facekapow force-pushed the node-support branch from 9ea5bff to fa55ecc Compare January 24, 2016 20:11

iefserge and others added 3 commits January 25, 2016 01:25

Remove pause/resume overrides

fc5caf3

Merge pull request #96 from runtimejs/node-support-streams

d4588cc

Node support streams (try 2)

http.js (partial)

75666b5

Remove http.js

1dbd8da

Make process.stdout and process.stderr output to the QEMU console, ad…

653aa12

…d process.termout and process.termerr

iefserge reviewed Mar 25, 2016
View reviewed changes

Fixes: console newline, accidentally misnamed process.termout and pro…

e45d79f

…cess.termerr, remove debug console.log

iefserge reviewed Mar 26, 2016
View reviewed changes

__SYSCALL.write

749b11b

facekapow merged commit 6278e61 into master Mar 26, 2016

facekapow changed the title ~~Node support - Work In Progress~~ Node support Mar 26, 2016

iefserge deleted the node-support branch March 26, 2016 17:15

Node support #93

Node support #93

Conversation

facekapow commented Jan 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facekapow commented Jan 24, 2016

iefserge commented Jan 24, 2016

facekapow commented Jan 24, 2016

facekapow commented Jan 24, 2016

iefserge commented Jan 24, 2016

facekapow commented Jan 24, 2016

iefserge commented Jan 24, 2016

iefserge commented Feb 6, 2016

facekapow commented Mar 23, 2016

facekapow commented Mar 24, 2016

facekapow commented Mar 25, 2016

iefserge commented Mar 25, 2016

facekapow commented Mar 25, 2016

facekapow commented Mar 25, 2016

facekapow commented Mar 25, 2016

piranna commented Mar 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iefserge commented Mar 25, 2016

iefserge commented Mar 25, 2016

facekapow commented Mar 25, 2016

piranna commented Mar 25, 2016

facekapow commented Mar 25, 2016

piranna commented Mar 25, 2016

iefserge commented Mar 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facekapow commented Mar 26, 2016

iefserge commented Mar 26, 2016

facekapow commented Mar 26, 2016

facekapow commented Mar 26, 2016

iefserge commented Mar 26, 2016

facekapow commented Mar 26, 2016

iefserge commented Mar 26, 2016

facekapow commented Mar 26, 2016

iefserge commented Mar 26, 2016

facekapow commented Mar 26, 2016

iefserge commented Mar 28, 2016

facekapow commented Mar 28, 2016