-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
v8: fix build on solaris platforms #1079
Conversation
`v8/3c7e4403` introduced a different cast which broke building on Illumos. Revert to previous behavior for V8_OS_SOLARIS. Found on SmartOS while building with gcc 4.9.0.
Is there a fix for this upstream? |
@jbergstroem Patches uploaded to the V8 bug tracker are normally ignored. You have to install depot_tools, run |
@bnoordhuis Ah. I just assumed magic would happen because it was set as assigned. I'll give it a go. |
Seems like a reasonable change for a big impact, thanks @jbergstroem |
Review going on here now: https://codereview.chromium.org/990063002 |
Looks like the patch landed upstream. |
In what version will it become available to us? @bnoordhuis are we good to float this until then? |
Looks like it landed in master, if I'm not mistaken. That would be v8 4.3, or about 11? weeks out. Quite a ways. |
But how many v8 upgrades are going to happen in that 11 weeks? Seems like a long time to wait for a two line change. |
According to our plans, one for 4.2, and one for 4.3, both when they ship in stable chrome (at the end of each dev cycle). Seems like it's reasonable to float it, but I'm sure @bnoordhuis can weigh in much better. |
It's okay to back-port (edit: meant float) it, it'll probably apply cleanly. Maybe @jbergstroem can ping the V8 people and ask them to back-port it to the 4.1 and 4.2 branches. |
Looks like the usual failures, and one random timeout on OS X. I'm going to land this. |
`v8/3c7e4403` introduced a different cast which broke building on Illumos. Revert to previous behavior for V8_OS_SOLARIS. Found on SmartOS while building with gcc 4.9.0. V8-Issue: https://code.google.com/p/v8/issues/detail?id=3935 V8-Patch: https://codereview.chromium.org/990063002 PR-URL: #1079 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Thanks! Landed in 8c4f0df |
I have asked v8 people for a backport, but haven't gotten any response. Will follow up. |
Looks like its getting merged into 4.1 (and 4.2): https://chromium.googlesource.com/v8/v8.git/+/f2d617d6b962b8a6709b2564fe1d576572801237 |
Not sure what our policy is about patching v8 but my general standpoint is to avoid it at almost all costs. What that out the way, carrying this "one-liner" (patch sent upstream and currently in progress, but I'm unsure when or in what branch this will land) adds support for building io.js on smartos/illumos which also passes the test suite.