Skip to content
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

src: env.cc should not include unistd.h on Windows #4145

Closed
mscdex opened this issue Dec 4, 2015 · 1 comment · May be fixed by sirinartk/node#3, adamlaska/node#21, jhudaz/node#3, adamlaska/node#22 or SeppPenner/node#2
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.

Comments

@mscdex
Copy link
Contributor

mscdex commented Dec 4, 2015

unistd.h is being included unconditionally in src/env.cc since 94b9948, but Windows does not have this header.

/cc @JungMinu

@mscdex mscdex added the lib / src Issues and PRs related to general changes in the lib or src directory. label Dec 4, 2015
@targos
Copy link
Member

targos commented Dec 4, 2015

Example of failure: https://ci.nodejs.org/job/node-compile-windows/480/label=win-vs2013/console
src\env.cc(4): fatal error C1083: Cannot open include file: 'unistd.h': No such file or directory [c:\workspace\node-compile-windows\label\win-vs2013\node.vcxproj]

cjihrig added a commit to cjihrig/node that referenced this issue Dec 4, 2015
94b9948 added unistd.h to src/env.cc in order to use getpid().
However, this doesn't exist on Windows. This commit conditionally
defines getpid() based on the OS.

Fixes: nodejs#4145
PR-URL: nodejs#4146
Reviewed-By: Brian White <[email protected]>
cjihrig added a commit that referenced this issue Dec 5, 2015
94b9948 added unistd.h to src/env.cc in order to use getpid().
However, this doesn't exist on Windows. This commit conditionally
defines getpid() based on the OS.

Fixes: #4145
PR-URL: #4146
Reviewed-By: Brian White <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016
94b9948 added unistd.h to src/env.cc in order to use getpid().
However, this doesn't exist on Windows. This commit conditionally
defines getpid() based on the OS.

Fixes: nodejs#4145
PR-URL: nodejs#4146
Reviewed-By: Brian White <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment