Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

build: fix build for SmartOS #8534

Closed
wants to merge 1 commit into from

Conversation

misterdjules
Copy link

This change in V8: https://code.google.com/p/v8/source/detail?r=22210
has introduced a method named OS::GetCurrentThreadId which fails to
compile on OSes where a "gettid" syscall does not exist.
This build issue has been fixed upstream by another change:
https://code.google.com/p/v8/source/detail?r=23459. This commit
integrates this fix. It's still not clear if this is good enough for the
long term, see https://code.google.com/p/v8/issues/detail?id=3620 for
more information.

The other build issue was due to the fact that alloca.h is not included
by other system includes on SmartOS, which is assumed by V8.

@misterdjules
Copy link
Author

Built and tested on SmartOS, MacOS X, Linux and Windows 7.

@indutny
Copy link
Member

indutny commented Oct 9, 2014

LGTM

@indutny
Copy link
Member

indutny commented Oct 9, 2014

Landed in node-forward/node@011319e

This change in V8: https://code.google.com/p/v8/source/detail?r=22210
has introduced a method named OS::GetCurrentThreadId which fails to
compile on OSes where a "gettid" syscall does not exist.

This build issue has been fixed upstream by several changes:
- https://code.google.com/p/v8/source/detail?r=23459.
- https://codereview.chromium.org/649553002
- https://codereview.chromium.org/642223003

Another minor fix to the upstream changes was also necessary.
See https://code.google.com/p/v8/issues/detail?id=3620 for
more information.

The other build issue was due to the fact that alloca.h is not included
by other system includes on SmartOS, which is assumed by V8.

Built and tested on Linux, MacOS X, Windows and SmartOS.
@misterdjules
Copy link
Author

@tjfontaine PR updated with latest changes from https://code.google.com/p/v8/issues/detail?id=3620.

@misterdjules
Copy link
Author

Just for the record, all changes have been integrated upstream, except the alloca build fix, which is not needed by more recent V8 versions.

@tjfontaine
Copy link

this is landed for now with the intent to do another v8 upgrade this week

@tjfontaine tjfontaine closed this Nov 5, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants