You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I added node-test-commit-plinux to node-test-commit Oct 14, and I can see that it is in later CI runs so it must be because of the timing. What I saw in the runs was build 8 being ok, and build 9 having the regression. However, what obviously happened was that you launched the CI run before I stitched it in, then the actual commit went in between runs 8 and 9. That does point out one nice thing in favor of the test and commit as a single job (which we had to back away from) as it makes it clearer where failures start.
Its looking like this is actually an issue where the buffer size at the V8 level is not big enough for ppc because the code size ends up being bigger. We are putting together a chromium review for the change and will add the link here when available. Once its in google master I'll see if its possible for us to float it in the Node repo until we pull in a version of V8 that includes the fix (likely 4.7)
One you have the fix in the V8 master, it would be great if you also submitted a merge request to get upstream to back-port it to 4.6 as per this process. That would avoid having to float a patch.
Yep; I think this is already included in my PR for vee-eight-4.7: #3481. I haven't seen the merge for 4.6 yet; is there an issue tracking the merge-request? It would be good to pick up this fix for v5.0.
Activity
mhdawson commentedon Oct 15, 2015
CI run on PR with test change https://ci.nodejs.org/job/node-test-commit-plinux/23/
ofrobots commentedon Oct 16, 2015
@mhdawson Here's the original CI run for that commit: https://ci.nodejs.org/job/node-test-pull-request/509/. Are the pLinux tests not part of the
test-pull-requestCI builds as documented here?mhdawson commentedon Oct 16, 2015
I added node-test-commit-plinux to node-test-commit Oct 14, and I can see that it is in later CI runs so it must be because of the timing. What I saw in the runs was build 8 being ok, and build 9 having the regression. However, what obviously happened was that you launched the CI run before I stitched it in, then the actual commit went in between runs 8 and 9. That does point out one nice thing in favor of the test and commit as a single job (which we had to back away from) as it makes it clearer where failures start.
Its looking like this is actually an issue where the buffer size at the V8 level is not big enough for ppc because the code size ends up being bigger. We are putting together a chromium review for the change and will add the link here when available. Once its in google master I'll see if its possible for us to float it in the Node repo until we pull in a version of V8 that includes the fix (likely 4.7)
ofrobots commentedon Oct 16, 2015
One you have the fix in the V8 master, it would be great if you also submitted a merge request to get upstream to back-port it to 4.6 as per this process. That would avoid having to float a patch.
mtbrandy commentedon Oct 16, 2015
V8 fix under review here: https://codereview.chromium.org/1415463002/
mhdawson commentedon Oct 22, 2015
@ofrobots, pulling over to 4.6 and 4.7 is already in progress. Just planning to float until the full update we take for v8
ofrobots commentedon Oct 22, 2015
Yep; I think this is already included in my PR for
vee-eight-4.7: #3481. I haven't seen the merge for 4.6 yet; is there an issue tracking the merge-request? It would be good to pick up this fix for v5.0.ofrobots commentedon Oct 22, 2015
I see that a patch is already floating for master (4.6) in #3474.
v8: pull fix for builtin code size on PPC
v8: pull fix for builtin code size on PPC
v8: pull fix for builtin code size on PPC
deps: fix openssl for MSVS 2015
deps: fix openssl for MSVS 2015
deps: fix openssl for MSVS 2015
v8: pull fix for builtin code size on PPC