-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
test: fix missing unistd.h on windows #3532
Conversation
cc @bnoordhuis probably |
@bnoordhuis Would you please also land it on v4.x? Thanks. |
@@ -13,7 +20,8 @@ struct async_req { | |||
|
|||
void DoAsync(uv_work_t* r) { | |||
async_req* req = reinterpret_cast<async_req*>(r->data); | |||
sleep(1); // Simulate CPU intensive process... | |||
// Simulate CPU intensive process... | |||
std::this_thread::sleep_for(std::chrono::seconds(1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work on OS X due to lack of library support. Can I suggest something like this?
#ifdef _WIN32
Sleep(1000);
#else
sleep(1);
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy to use that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But since the latest v8 is also using c++11, I think OS X might be able to get around c++11 problems by using new compilers and libraries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two aspects to C++11 support: compiler support (okay on OS X) and libc++ support (which is lacking.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed the default libc++ library on OS X is outdated and doesn't support c++11. I wonder how OS X user compile the latest v8, which is part of nodejs and using c++11? they should have a solution to that. (I think they download the lib from somewhere)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, it ships with its own copy of libc++: https://chromium.googlesource.com/chromium/buildtools/+/master/third_party/libc++/
PR-URL: #3532 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Thanks, landed in 4139f2a. |
hello @jasnell , would you please help land it on v4.x? thanks. |
PR-URL: nodejs#3532 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #3532 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #3532 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs/node#3532 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Landed in v4.x-staging in 2fc13e5 |
PL-URL: #3531