Update Node.js to v0.10.33; remove unneeded openssl.gyp patch

[hansmbakker]

Please review, test and merge this commit.

I updated Node,js - it contains these changes:

• 2014.10.20 Version 0.10.33 (Stable)
  • openssl: Update to 1.0.1j (Addressing multiple CVEs)
  • uv: Update to v0.10.29
  • child_process: properly support optional args (cjihrig)
  • crypto: Disable autonegotiation for SSLv2/3 by default (Fedor Indutny, Timothy J Fontaine, Alexis Campailla)
    This is a behavior change, by default we will not allow the negotiation to SSLv2 or SSLv3. If you want this behavior, run Node.js with either --enable-ssl2 or --enable-ssl3 respectively.
    This does not change the behavior for users specifically requesting SSLv2_method or SSLv3_method. While this behavior is not advised, it is assumed you know what you're doing since you're specifically asking to use these methods.
• 2014.09.16 Version 0.10.32 (Stable), 0fe0d1
  • npm: Update to 1.4.28
  • v8: fix a crash introduced by previous release (Fedor Indutny)
  • configure: add --openssl-no-asm flag (Fedor Indutny)
  • crypto: use domains for any callback-taking method (Chris Dickinson)
  • http: do not send 0\r\n\r\n in TE HEAD responses (Fedor Indutny)
  • querystring: fix unescape override (Tristan Berger)
  • url: Add support for RFC 3490 separators (Mathias Bynens)
• The patch for openssl.gyp was not needed anymore, so I removed it.

[Dr-Bean]

You beat me to it ;) Couple things:
Have you checked if the other patches are still needed? There have been quite a lot of changes in .32 wrt cross-compiling so they might not be needed anymore. I think I got away with just the async patch, but I haven't completed testing yet;
SPK_REV can't go down, only up;
Lastly, have you seen any changes wrt #1223 using the DSM4.3 toolchains?

[hansmbakker]

About those patches - it compiles also after removing all but the async patch. However - when looking in the files referenced by the patches - only in the openssl.gyp I saw the change from the patch applied. So only that patch was clearly redundant. The other patches were to disable some code on ARM, and I didn't see those #ifdef __ARM__ statements in the unpatched files yet.
@Diaoul Are patches only to fix compilation issues, so that if it compiles without the patch, it is safe? Or do they also disable functionality that is not available on ARM? If so, I'm not sure that it is safe to remove the other patches as well.

I thought that SPK_REV is only per SPK_VERS- now SPK_VERS is updated, so I thought SPK_REV should start counting from 1 again

About the npm issue I should check.

[Dr-Bean]

Are patches only to fix compilation issues, so that if it compiles, it is safe? Or do they also disable functionality that is not available on ARM? If so, I'm not sure that it is safe to remove the others as well.

Both: fixing compilation issues and fixing functionality for one or more arches (not disabling functionality per se, that really depends on circumstances)
During investigation of 1223, I noticed that a lot of changes were geared to fixing compilation issues on ARM. It might be the patches are no longer needed due to changes elsewhere. With the (limited) testing I did up to now, I didn't see any obvious reason for them now.
SPK_REV always needs to go up, the device won't recognize it as an update otherwise.

[hansmbakker]

Both: fixing compilation issues and fixing functionality for one or more arches (not disabling functionality per se, that really depends on circumstances)

The openssl.gyp, cryptlib.c and mem_clr.c patches I added to fix openssl compilation on ARM, so I think those can be safely removed if it compiles.

However, the pthread patch I don't know about. Do you have an idea about that?
And am I right that for you the async patch was still needed to compile?

[Dr-Bean]

Yes, the async fix is still needed: it won't compile otherwise.
The pthread patch I'm not 100% on either. It compiles without it, but I don't have a specific test to check functionality. I can install a package via npm, run it etc, but I've only done a small test on an ARM arch right now.
I guess it wouldn't hurt to leave it in, but I'd rather not do that if we don't know the reason why it was added in the first place (we really should document what specific reason there is for a certain patch...)

[hansmbakker]

I found these threads

• https://groups.google.com/forum/#!topic/libuv/I7n4hPFgewo
• http://forum.synology.com/enu/viewtopic.php?f=35&t=25847#p255907

They says that the pthread_condattr_setclock was not available in some version of libpthread and that removing that call was needed to make it compile.

Since it compiles now, it is safe for us to remove the patch, but I'm only compiling for armadaxp - should we leave it in for other architectures?

[Dr-Bean]

Yes, after looking at it, I gathered that it was related to compiling.
Easiest thing to do is check, so I've just started compiling all-archs (using 4.3 TC and 5.0 TC), with just the async patch. I'll come back with the results later today or tomorrow.

[hansmbakker]

That's really helpful, thank you :)

Unfortunately I only have a VM with Debian in it - it is really slow so I'm glad you offered to compile the other archs

I'll test whether npm works; I'll wait with committing the last changes (setting rev number to 3, adding changelog and removing patches).

[hansmbakker]

For me, #1223 is not an issue - npm works without asking for additional certificates, etc. See https://gist.github.com/wind-rider/2f808f1c0161a25fa459. I have the latest DSM, but using spksrc the compilation is done using the 4.3 TC

Also I can't reproduce #1285 - see https://gist.github.com/wind-rider/e5debfa4d4926cfdfb3c.

The only thing I'm not sure about, is why setting the PATH is still needed. It looks like node is getting installed in /volume1/@appstore/node and that /usr/local/node is a symlink to it.

However, in install.sh there is a command PATH="${INSTALL_DIR}/bin:${PATH}" and it seems that it doesn't work after installation.

[Dr-Bean]

Ok, the pthread patch is needed for x86 arches with 4.3 TC, 5.0 TC looks to be fine without it. Best to leave that one in then, seeing as 4.3 TC is still the default.

As for the PATH: there is a Synology node package, which will always be first on PATH, because it'll be in /usr/bin or something. Best practice: call the correct node or npm executable with it's full path, or alternatively, put it on PATH manually/via a script when you launch your own program.
The install.sh and dsm-control.sh PATH variable can both be removed for that matter: they are never called.
/usr/local/node is a symlink to /volumex/@appstore/node. Using /usr/local/{package} prevents issues with finding the correct volume on which @appstore resides.

[hansmbakker]

Ok, thank you for your feedback.

I added the pthread patch back in, but I left the PATH commands in the .sh files, in case somebody would want to call node from the installer files.

The changes are committed to my branch.

[hansmbakker]

Maybe we can make such a documentation html page like https://github.com/SynoCommunity/spksrc/blob/develop/spk/debian-chroot/src/app/help/enu/index.html with some instructions on best practices, how to use it?

For me Node.js works now, but I still have some questions myself, for example:

• this PATH setting is not entirely clear to me
  • why would Synology's node come first if it is not installed
  • why can't we add the /usr/local/node/bin folder to the PATH at install time, so that we don't need to set it every time
• it is not entirely clear to me how to use node on Synology other than ssh'ing as root into the Diskstation.
  • I'd rather avoid ssh'ing as root, and run it as a normal user.
  • Also it would be convenient to have something that would watch a folder for a package.json to automatically install dependencies and that would automatically (re)start an app.js file upon changes etc.

[hansmbakker]

@Dr-Bean / @Diaoul: do you have any comments on above points?

I think they are not blockers for merging my patch, or does something still need to be improved?

@Diaoul: there are some opened issues about node/npm here and I can't reproduce them Node/npm based on this package on DSM 5 (latest updates installed) on a DS214. It would be pleasant if this package could be tested by more people so that these issues can be closed.

Also any input on how to use node effectively on synology is welcome.

[cytec]

@Wind-rider if i remember this right, there was a discussion about adding custom stuff to the user PATH on install time... long story short: don't do it. if someone wants to do it, it can be done very easily.
messing with the users PATH on install/uninstall is just another possible source for errors

if you want to be able to call it directly from the users path, you can still symlink it to /usr/local/bin/ on install time like i did it for ffmpeg: https://github.com/SynoCommunity/spksrc/blob/develop/spk/ffmpeg/src/dsm-control.sh

[Dr-Bean]

PR should be okay (I mean, I have the exact same changes to a local branch here ;), just haven't gotten round to answering your questions yet.

The documentation method to use could indeed be done similar to debian-chroot. I noticed Synology added my request to support the built-in help for third party packages (see latest developers guide), so that might be interesting for DSM5+ to check out. Otherwise, we could put up a page on the synocommunity website. @Diaoul, I don't know for sure if that functionality is available?
@Wind-rider, you might be able to come up with a piece of text on what it should contain?

On $PATH: cytec is right, we do not want to add anything to PATH during install time, and there's no reason to either.
Obviously, Synology's node package wouldn't be used if it's not installed, but there's no way to determine that, especially if you just run node on CLI.
We do not want to interfere with other packages, so either we use full paths or we set the correct path at runtime (take a look at the only node example I have on that: https://github.com/SynoCommunity/spksrc/pull/795/files#diff-c18590fb3b374ec2aa5f0e4f4e46a841R21): we add our own PATH, and add the system PATH after it.

Usage of node is the same as with any other application. Use CLI, or package your node application.
Using the package approach, you can add your own user during package installation. If you go for the CLI approach, (I think) you can log in with a normal useraccount via SSH and run node without any particular problems (although some permissions issues might come into play)

The watchfolder approach you described is especially something you'd want to combine with some kind of package imo. It's not something that we would add into the node package.

[hansmbakker]

I saw that Synology now also packaged v0.10.33 with the new DSM 5.1 version - maybe we should reconsider whether we want to keep maintaining our own version in spksrc?

[Diaoul]

Yep, lets drop this... It's sad that Synology doesn't give heads up on what kind of development they're doing, would save us some time.

I already asked them about that, repeatedly, but that doesn't seem to bother them.

[Diaoul]

Merging anyway so we have an up to date version in spksrc.

[hansmbakker]

@Diaoul thank you for asking them