Hi, thanks for the patch. Some comments below:
-- Nadav Har'El [email protected] On Mon, Feb 6, 2017 at 8:45 AM, 'Rean Griffith' via OSv Development < [email protected]> wrote: > Signed-off-by: rean <[email protected]> > Can you please use a full name in the signed-off-by? Thanks. > --- > node/GET | 14 ++++++++++++-- > node/Makefile | 16 +++++++++++++--- > node/hello.js | 16 ++++++++++++++++ > node/patch-4.1.1 | 45 +++++++++++++++++++++++++++++++++++++++++++++ > node/patch-4.6.1 | 30 ++++++++++++++++++++++++++++++ > node/patch-6.9.1 | 42 ++++++++++++++++++++++++++++++++++++++++++ > node/patch-6.9.5 | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > node/patch-7.0.0 | 45 +++++++++++++++++++++++++++++++++++++++++++++ > node/patch-7.2.0 | 56 ++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++ > node/patch-7.5.0 | 56 ++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++ > 10 files changed, 366 insertions(+), 5 deletions(-) > create mode 100644 node/hello.js > create mode 100755 node/patch-4.1.1 > create mode 100755 node/patch-4.6.1 > create mode 100755 node/patch-6.9.1 > create mode 100755 node/patch-6.9.5 > create mode 100755 node/patch-7.0.0 > create mode 100755 node/patch-7.2.0 > create mode 100755 node/patch-7.5.0 > > diff --git a/node/GET b/node/GET > index 79a3b27..accd2fc 100755 > --- a/node/GET > +++ b/node/GET > @@ -10,6 +10,16 @@ then > exit 1 > fi > > -wget "https://github.com/nodejs/node/archive/v${1}.tar.gz"; -O - | tar > zxf - > +# Tarball we want > +PACKAGE="v${1}.tar.gz" > > -./patch "$1" > +# Do a conditional get if we don't have the tarball handy > +if [ ! -f ${PACKAGE} ]; then > + wget "https://github.com/nodejs/node/archive/${PACKAGE}"; > +fi > + > +tar -xzvf ${PACKAGE} > + > +echo "$1" > + > +./patch-${1} "$1" > diff --git a/node/Makefile b/node/Makefile > index 9674ecc..81e394f 100644 > --- a/node/Makefile > +++ b/node/Makefile > @@ -6,14 +6,24 @@ libnode-$(NODE_VERSION).so: $(LIB_NODE) > > $(LIB_NODE): > cd node-$(NODE_VERSION); ./configure > - $(MAKE) -C node-$(NODE_VERSION) > + $(MAKE) -C node-$(NODE_VERSION) -j4 > Why is this change necessary? scripts/build passes a "-j" option to the module's makefile, so you can do "scripts/build -j4" if you want. If this doesn't work (I haven't checked recently), we should fix it. A user can also have a default "MAKEFLAGS" variable if he wants to generally have "-j4" for every build. In any case, I think it's wrong to force a "-j4" - on some machines with very low memory this may be too much, and on some machines it might be too little (I often use -j8). > > # Support for OSv's combined kernel+module build system ('make > modules=node' > # and 'make clean') > -VERSION=4.1.1 > +#VERSION=4.1.1 > +#VERSION=4.6.1 > +#VERSION=6.9.1 > +#VERSION=7.0.0 > + > +# Specify default nodejs version > +ifndef VERSION > +VERSION=7.5.0 > +endif > Can you please update also node/README to tell people the new instructions (set VERSION, not NODE_VERSION, and no need to separately run GET? Also I think one needs to do "make module" and not just "make"? Obviously, better just use scripts/build...) > + > module: > if test ! -d node-$(VERSION); then ./GET $(VERSION); $(MAKE) > NODE_VERSION=$(VERSION); fi > echo '/libnode.so: $${MODULE_DIR}/libnode-$(VERSION).so' > > usr.manifest > : > bootfs.manifest > + echo '/hello.js: $${MODULE_DIR}/hello.js' >> usr.manifest > Currently, the "node" module does not include any example. Rather, we have separate modules like node-express-example which "require" the node module. Can you please add (in a separate patch) a separate "node-example" (for example) module with this hello.js, instead of adding it to the base nodejs? > clean: > - rm -rf node-$(VERSION) usr.manifest bootfs.manifest > libnode-$(VERSION).so > + rm -rf node-$(VERSION) usr.manifest bootfs.manifest > libnode-$(VERSION).so libnode-$(VERSION)-stripped.so > diff --git a/node/hello.js b/node/hello.js > new file mode 100644 > index 0000000..140ba89 > --- /dev/null > +++ b/node/hello.js > @@ -0,0 +1,16 @@ > +// Load the http module to create an http server. > +console.log("Attempting to run app"); > +var http = require('http'); > + > +// Configure our HTTP server to respond with Hello World to all requests. > +var server = http.createServer(function (request, response) { > + response.writeHead(200, {"Content-Type": "text/plain"}); > + response.end("Hello World\n"); > +}); > + > +// Listen on port 8002, IP defaults to 127.0.0.1 > +server.listen(8002); > + > +// Put a friendly message on the terminal > +console.log("Server running at http://127.0.0.1:8002/";); > + > diff --git a/node/patch-4.1.1 b/node/patch-4.1.1 > new file mode 100755 > index 0000000..3774af0 > --- /dev/null > +++ b/node/patch-4.1.1 > @@ -0,0 +1,45 @@ > +#!/bin/bash > + > +set -e > + > +cd "$(dirname "$0")" > + > +if test $# -ne 1 > +then > + echo "usage: $0 <node-version>" 1>&2 > + exit 1 > +fi > + > +cd "node-$1" > + > +ed -s node.gyp << EOF > +,s/\(['"]\)executable\1/\1shared_library\1/ > +g/'-Wl,--whole-archive/d > +w > +EOF > + > +ed -s common.gypi << EOF > +,s/'cflags':.*'-pthread'/&, '-fPIC'/ > +w > +EOF > + > +ed -s deps/uv/src/unix/thread.c << EOF > +/int uv_barrier_init/i > +#endif > +#if 1 > +. > +w > +EOF > + > +ed -s "$(find deps/uv -name uv-unix.h)" <<EOF > +/#if defined(__APPLE/a > +#endif > +#if 1 > +. > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/types.h <<EOF > +,s/nearbyint/rint/ > +w > +EOF > diff --git a/node/patch-4.6.1 b/node/patch-4.6.1 > new file mode 100755 > index 0000000..6a3379c > --- /dev/null > +++ b/node/patch-4.6.1 > @@ -0,0 +1,30 @@ > +#!/bin/bash > + > +set -x > +set -e > + > +cd "$(dirname "$0")" > + > +if test $# -ne 1 > +then > + echo "usage: $0 <node-version>" 1>&2 > + exit 1 > +fi > + > +cd "node-$1" > + > +ed -s node.gyp << EOF > +,s/\(['"]\)executable\1/\1shared_library\1/ > +g/'-Wl,--whole-archive/d > +w > +EOF > + > +ed -s common.gypi << EOF > +,s/'cflags':.*'-pthread'/&, '-fPIC'/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/types.h <<EOF > +,s/nearbyint/rint/ > +w > +EOF > What's the difference between this script and the previous one? Is there any difference? By the way, since these are full-fledged scripts, not patches, I think it will be nicer to have one which does different things for different versions (if necessary) instead of 5 different scripts with tiny differences. Also, I'll be very happy if instead of patching around OSv bugs, we'll solve them. E.g., why do we need to change nearbyint() to rint()? Are we missing nearbyint()? > diff --git a/node/patch-6.9.1 b/node/patch-6.9.1 > new file mode 100755 > index 0000000..13cc3a4 > --- /dev/null > +++ b/node/patch-6.9.1 > @@ -0,0 +1,42 @@ > +#!/bin/bash > + > +set -x > +set -e > + > +cd "$(dirname "$0")" > + > +if test $# -ne 1 > +then > + echo "usage: $0 <node-version>" 1>&2 > + exit 1 > +fi > + > +cd "node-$1" > + > +ed -s node.gyp << EOF > +,s/\(['"]\)executable\1/\1shared_library\1/ > +g/'-Wl,--whole-archive/d > +w > +EOF > + > +ed -s common.gypi << EOF > +,s/'cflags':.*'-pthread'/&, '-fPIC'/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/types.h <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/arm/simulator-arm.cc > <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/wasm/wasm-external-refs.h > <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > + > diff --git a/node/patch-6.9.5 b/node/patch-6.9.5 > new file mode 100755 > index 0000000..e76c394 > --- /dev/null > +++ b/node/patch-6.9.5 > @@ -0,0 +1,51 @@ > +#!/bin/bash > + > +set -x > +set -e > + > +cd "$(dirname "$0")" > + > +if test $# -ne 1 > +then > + echo "usage: $0 <node-version>" 1>&2 > + exit 1 > +fi > + > +cd "node-$1" > + > +ed -s node.gyp << EOF > +,s/\(['"]\)executable\1/\1shared_library\1/ > +g/'-Wl,--whole-archive/d > +w > +EOF > + > +ed -s common.gypi << EOF > +,s/'cflags':.*'-pthread'/&, '-fPIC'/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/types.h <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/arm/simulator-arm.cc > <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/wasm/wasm-external-refs.h > <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +# OSv always returns the path "/dev/console" for the ttyname for any fd > +# passed in > +# (https://github.com/cloudius-systems/osv/blob/master/libc/ > unistd/ttyname_r.c) > +# so it's possible for stdin(0), stdout(1) etc. to be passed into > uv_tty_init > +# and then closed later in uv_close, which would trigger this assert > +# unnecessarily on OSv > +[ -f deps/uv/src/unix/core.c ] && ed -s deps/uv/src/unix/core.c <<EOF > +,s/assert(fd > STDERR_FILENO);/;/ > I'm curious why this is a problem - maybe there's a bug in OSv that needs to fixing... Yes, for every fd which has isatty() we return "/dev/console" for ttyname_r(). Why is this surprising? How would a different name be better? > +w > +EOF > diff --git a/node/patch-7.0.0 b/node/patch-7.0.0 > new file mode 100755 > index 0000000..e400804 > --- /dev/null > +++ b/node/patch-7.0.0 > @@ -0,0 +1,45 @@ > +#!/bin/bash > + > +set -x > +set -e > + > +cd "$(dirname "$0")" > + > +if test $# -ne 1 > +then > + echo "usage: $0 <node-version>" 1>&2 > + exit 1 > +fi > + > +cd "node-$1" > + > +ed -s node.gyp << EOF > +,s/\(['"]\)executable\1/\1shared_library\1/ > +g/'-Wl,--whole-archive/d > +w > +EOF > + > +ed -s common.gypi << EOF > +,s/'cflags':.*'-pthread'/&, '-fPIC'/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/types.h <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/arm/simulator-arm.cc > <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/wasm/wasm-external-refs.cc > <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/wasm/wasm-interpreter.cc > <<EOF > +,s/nearbyint/rint/ > +w > +EOF > diff --git a/node/patch-7.2.0 b/node/patch-7.2.0 > new file mode 100755 > index 0000000..cc021e5 > --- /dev/null > +++ b/node/patch-7.2.0 > @@ -0,0 +1,56 @@ > +#!/bin/bash > + > +set -x > +set -e > + > +cd "$(dirname "$0")" > + > +if test $# -ne 1 > +then > + echo "usage: $0 <node-version>" 1>&2 > + exit 1 > +fi > + > +cd "node-$1" > + > +ed -s node.gyp << EOF > +,s/\(['"]\)executable\1/\1shared_library\1/ > +g/'-Wl,--whole-archive/d > +w > +EOF > + > +ed -s common.gypi << EOF > +,s/'cflags':.*'-pthread'/&, '-fPIC'/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/types.h <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/arm/simulator-arm.cc ] && ed -s > deps/v8/src/arm/simulator-arm.cc <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/wasm/wasm-external-refs.cc ] && ed -s > deps/v8/src/wasm/wasm-external-refs.cc <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/wasm/wasm-interpreter.cc ] && ed -s > deps/v8/src/wasm/wasm-interpreter.cc <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +# OSv always returns the path "/dev/console" for the ttyname for any fd > +# passed in > +# (https://github.com/cloudius-systems/osv/blob/master/libc/ > unistd/ttyname_r.c) > +# so it's possible for stdin(0), stdout(1) etc. to be passed into > uv_tty_init > +# and then closed later in uv_close, which would trigger this assert > +# unnecessarily on OSv > +[ -f deps/uv/src/unix/core.c ] && ed -s deps/uv/src/unix/core.c <<EOF > +,s/assert(fd > STDERR_FILENO);/;/ > +w > +EOF > diff --git a/node/patch-7.5.0 b/node/patch-7.5.0 > new file mode 100755 > index 0000000..cc021e5 > --- /dev/null > +++ b/node/patch-7.5.0 > @@ -0,0 +1,56 @@ > +#!/bin/bash > + > +set -x > +set -e > + > +cd "$(dirname "$0")" > + > +if test $# -ne 1 > +then > + echo "usage: $0 <node-version>" 1>&2 > + exit 1 > +fi > + > +cd "node-$1" > + > +ed -s node.gyp << EOF > +,s/\(['"]\)executable\1/\1shared_library\1/ > +g/'-Wl,--whole-archive/d > +w > +EOF > + > +ed -s common.gypi << EOF > +,s/'cflags':.*'-pthread'/&, '-fPIC'/ > +w > +EOF > + > +[ -f deps/v8/src/types.h ] && ed -s deps/v8/src/types.h <<EOF > +,s/nearbyint/rint/ > If we just add nearbyint(), all these silly changes will be gone :-) I'll open an issue about that. > +w > +EOF > + > +[ -f deps/v8/src/arm/simulator-arm.cc ] && ed -s > deps/v8/src/arm/simulator-arm.cc <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/wasm/wasm-external-refs.cc ] && ed -s > deps/v8/src/wasm/wasm-external-refs.cc <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +[ -f deps/v8/src/wasm/wasm-interpreter.cc ] && ed -s > deps/v8/src/wasm/wasm-interpreter.cc <<EOF > +,s/nearbyint/rint/ > +w > +EOF > + > +# OSv always returns the path "/dev/console" for the ttyname for any fd > +# passed in > +# (https://github.com/cloudius-systems/osv/blob/master/libc/ > unistd/ttyname_r.c) > +# so it's possible for stdin(0), stdout(1) etc. to be passed into > uv_tty_init > +# and then closed later in uv_close, which would trigger this assert > +# unnecessarily on OSv > +[ -f deps/uv/src/unix/core.c ] && ed -s deps/uv/src/unix/core.c <<EOF > +,s/assert(fd > STDERR_FILENO);/;/ > +w > +EOF > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "OSv Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
