FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix missing unistd.h on windows by john-yan · Pull Request #3532 · nodejs/node · GitHub

/ node Public

test: fix missing unistd.h on windows - #3532

Closed
john-yan wants to merge 1 commit into
nodejs:masterfrom
john-yan:master
Closed

test: fix missing unistd.h on windows#3532
john-yan wants to merge 1 commit into
nodejs:masterfrom
john-yan:master

Conversation

Copy link
Copy Markdown

PL-URL: #3531

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

Copy link
Copy Markdown
Contributor

cc @bnoordhuis probably

Copy link
Copy Markdown
Author

@bnoordhuis Would you please also land it on v4.x? Thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am happy to use that.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There are two aspects to C++11 support: compiler support (okay on OS X) and libc++ support (which is lacking.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

Thanks, landed in 4139f2a.

Copy link
Copy Markdown
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 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 commented Oct 29, 2015

Copy link
Copy Markdown
Member

Landed in v4.x-staging in 2fc13e5

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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


Back | FazBrowse Home | New Git URL