Skip to content

test: fix missing unistd.h on windows #3532

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

john-yan
Copy link

PL-URL: #3531

@Fishrock123 Fishrock123 added c++ Issues and PRs that require attention from people who are familiar with C++. windows Issues and PRs related to the Windows platform. labels Oct 26, 2015
@Fishrock123
Copy link
Contributor

cc @bnoordhuis probably

@john-yan
Copy link
Author

@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));
Copy link
Member

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

Copy link
Author

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.

Copy link
Author

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.

Copy link
Member

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.)

Copy link
Author

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bnoordhuis
Copy link
Member

bnoordhuis pushed a commit that referenced this pull request Oct 28, 2015
PR-URL: #3532
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis
Copy link
Member

Thanks, landed in 4139f2a.

@john-yan
Copy link
Author

hello @jasnell , would you please help land it on v4.x? thanks.

rvagg pushed a commit to rvagg/io.js that referenced this pull request Oct 29, 2015
PR-URL: nodejs#3532
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@rvagg rvagg mentioned this pull request Oct 29, 2015
jasnell pushed a commit that referenced this pull request Oct 29, 2015
PR-URL: #3532
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jasnell pushed a commit that referenced this pull request Oct 29, 2015
PR-URL: #3532
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
john-yan pushed a commit to ibmruntimes/node that referenced this pull request Oct 29, 2015
PR-URL: nodejs/node#3532
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jasnell
Copy link
Member

jasnell commented Oct 29, 2015

Landed in v4.x-staging in 2fc13e5

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants