VS 2015 support #478
VS 2015 support #478
Conversation
|
https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/85/ which includes a VS2015 host |
03614ae to
9e8f118
Compare
|
new build: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/88/ I don't understand the Python stack trace for the OSX failures, anybody care to try and enlighten me on that one? |
|
Re: python: something in here is adding a non-string-y item to the |
|
@rvagg Can you mention the gyp commit (e.g. r1234) it's been upgraded to? I'll see if I can reproduce the OS X failure on my 10.8 machine. EDIT: No, seems to be working for me. |
|
@bnoordhuis I would have put a version in there if I knew where to find one, I went looking but alas.. Any hints on where to find a version for this? |
|
@rvagg I don't know where you got it from but if it's from subversion, then it's the svn commit. If it's git, it's the number from git-svn-id. If this upgrade is to today's HEAD, that would make it r2024. @chrisdickinson The stack trace suggests it's from here but it looks to me that additional_settings[k] has been sanitized at that point. Strange. |
|
@bnoordhuis I thought so too at first, but additional_settings gets updated with the contents of env. |
|
I printed them out and it is failing on a |
|
note this machine only has the commandline tools, not full xcode, which I'm guessing is partly to blame here, still a bug though |
54f093d to
21dd523
Compare
|
figured it out, needs a new entry in common.gypi: building here: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/90/ |
|
LGTM. Maybe you can move the change to common.gypi into a separate commit with an explanation of why it's needed (and move it before the gyp upgrade to keep The iojs-win2012r2-msvs2015 failure seems to be related to this. I'm not sure why OpenSSL does that but I think we can just rip it out, the only part of OpenSSL that uses stdio (in our configuration) are the apps. @piscisaureus or @seishun Agree/disagree? |
|
Good guess @bnoordhuis, when I fix the define to exclude MSVS 2015 it compiles fine and tests pass with only 1 failure (a TLS failure mind you...). See 7b66043. Re-running tests in CI here: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/94/nodes=iojs-win2012r2-msvs2015/console |
|
oh, that was an all-pass that time, neat. |
|
@rvagg It looks like it's running the message tests only. |
|
|
Ugh, github on phone, I shouldn't try and be productive. Will look in to this tomorrow. |
|
I don't like the idea of floating patches to OpenSSL to support a preview version of Visual Studio. Maybe it's better to wait for OpenSSL to fix it on their end? |
|
Yes, point taken. I was surprised that it wasn't already fixed tbh. I'll leave this PR open so ppl know we are tracking towards it but waiting for it to be easier. |
Fixes builds where environment variable has backslashes
|
It is fixed in OpenSSL 1.0.2. Patch is from 2 July 2014 http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=aab3560b65b9254d17770bb6fe3ca7edd7451429 |
|
OpenSSL 1.0.2 discussion: #589 |
Make the order of name, email and other additions simpler to copy paste and/or match with git commit messages. Useful when working with `Reviewed-By`. PR-URL: nodejs#1966 Reviewed-By: Christian Tellnes <christian@tellnes.no> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
tmpdir creation only happens for tests that need it. So failure to refresh the temporary directory should result in a failed test. PR-URL: nodejs#1976 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Futher discussion at nodejs/node-v0.x-archive#25294 PR-URL: nodejs#1926 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
The accessors run inside an implicit HandleScope, there is no need to create a new one. PR-URL: nodejs#2001 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Re-add the wrapper class id to AsyncWrap instances so they can be tracked directly in a heapdump. Previously the class id was given without setting the heap dump wrapper class info provider. Causing a segfault when a heapdump was taken. This has been added, and the label_ set to the given provider name so each instance can be identified. The id will not be set of the passed object has no internal field count. As the class pointer cannot be retrieved from the object. In order to properly report the allocated size of each class, the new pure virtual method self_size() has been introduces. PR-URL: nodejs#1896 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#1989 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
PR-URL: nodejs#1943 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
PR-URL: nodejs#1994 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Defer reading until user-land has a chance to add listeners. This allows the TLS wrapper to listen for _tlsError and trigger a clientError event if the socket already has data that could trigger. Fixes: nodejs#1114 PR-URL: nodejs#1496 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
require() now checks that the path exists before searching further in it. PR-URL: nodejs#1920 Reviewed-By: Isaac Z. Schlueter <i@izs.me> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Add link to Jenkins server in Collaborator Guide. PR-URL: nodejs#1995 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
PR-URL: nodejs#2000 Reviewed-By: Yosuke Furukawa <yosuke.furukawa@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
|
rvagg@0e54add that works for me (VS 2015 RC on latest Windows 10 build). But v8 cannot compile because of a bug in VS. There is a workaround since v4.4.42. |
|
@targos, I would not consider that as a bug since in the linked Connect bug report, both OP (David) and VC team member (Jonathan) are in agreement that there is no violation of standard. Note that the other bug in RC is fixed in recent compiler revision (build 19.00.23008, June 10, 2015) http://webcompiler.cloudapp.net/; it compiles with this code: struct User {
void *operator new(size_t, unsigned);
void operator delete(void *);
};
int main()
{
auto q = new (0) User;
return 0;
} |
|
@jasonwilliams200OK I was indeed talking about that other bug. They say it's fixed for RTM but is there a way to get the latest version of the C++ compiler before the release? |
|
I think no (that web-compiler is emerged as part of a research project http://rise4fun.com/), but if I venture an educated guess, VS RTM will be available few weeks before Windows 10 release (July 29, 2015), which is about any day now. :) |
vcbuild.bat calls python configure before setting GYP_MSVS_VERSION, so SelectVisualStudioVersion (tools\gyp\pylib\gyp\MSVSVersion.py) defaults to 'auto' and selects VS 2005. vcbuild sets the environment in the current shell, so this issue would manifest itself only on the first invocation of the script in any given shell windows. Reviewed-By: Julien Gilli <jgilli@fastmail.fm> PR-URL: nodejs/node-v0.x-archive#20109
|
pulled in nodejs/node-v0.x-archive@a58b174 as 396b4c6 CI run https://jenkins-iojs.nodesource.com/job/libuv+any-pr+multi/111/ |
|
Did something get messed up with this PR? GitHub shows 315 commits and 6686 files changed. |
|
yeah, it's really old and targetting the v1.x branch, hence the superfluous commits. It's just the head one that matters. |
|
that's because the PR targets nodejs:v1.x and it is not possible to change the target branch afterwards. Only the last 2 commits are relevant. |
|
@rvagg |
Not quite working yet, it gets to the openssl-cli compile and fails with a linking error:
This PR upgrades GYP to the latest which understands 2015, and has a lot more changes because there hasn't been an upgrade in a while I suppose.
I've also edited vcbuild.bat to do the appropriate checks.