Skip to content

Conversation

@fhinkel
Copy link
Member

Checklist
  • make -j4 test (UNIX)
  • commit message follows commit guidelines
Affected core subsystem(s)

deps V8

Description of change

Cherry-pick a51f429 from V8 upstream. Fixes case-insensitive regex problem in #7708.

/cc @nodejs/v8

Original commit message: [regexp] Fix case-insensitive matching for one-byte subjects. The bug occurs because we do not canonicalize character class ranges before adding case equivalents. While adding case equivalents, we abort early for one-byte subject strings, assuming that the ranges are sorted. Which they are not. [email protected] BUG=v8:5199 Review-Url: https://codereview.chromium.org/2159683002 Cr-Commit-Position: refs/heads/master@{nodejs#37833} Fixes: nodejs#7708
@nodejs-github-botnodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Jul 22, 2016
@targos
Copy link
Member

LGTM

@bnoordhuis
Copy link
Member

@fhinkel
Copy link
MemberAuthor

Slave went offline during the build ERROR: Connection was broken: java.io.IOException: Connection aborted:

@jasnell
Copy link
Member

LGTM with green ci

@ofrobots
Copy link
Contributor

new CI: https://ci.nodejs.org/job/node-test-pull-request/3458/.

The Power big endian machines continue to have recurring problems in the CI. /cc @mhdawson: Do these machines have enough disk space?

@ofrobots
Copy link
Contributor

Another attempt at the CI: https://ci.nodejs.org/job/node-test-pull-request/3484/

@fhinkel
Copy link
MemberAuthor

fhinkel commented Aug 1, 2016

PPC failed (10 days ago PPC passed but FreeBSD and Windows had problems) :

/home/iojs/build/workspace/node-test-commit-plinux/nodes/ppcbe-ubuntu1404/out/Release/obj.target/node/src/node_main.o: file not recognized: File truncated collect2: error: ld returned 1 exit status 

@ofrobots
Copy link
Contributor

LGTM. I think this is ready to land.

@nodejs/build, @mhdawson: Can anything be done about the Power machines in the CI, or should we be ignoring them? They have been causing a lot of churn and wasted time.

@jbergstroem
Copy link
Member

@ofrobots: there's a lot of free space on those machines. That error seems relatively new (at least looking week to week) -- we just need to establish if its really a regression or build-related.

@jbergstroem
Copy link
Member

@ofrobots I've cleaned cache in case it was corrupt for some reason.

@ofrobots
Copy link
Contributor

ofrobots commented Aug 1, 2016

I have seen this very error as a flake on a few PRs in the past week or so (maybe more):

...node/src/node_main.o: file not recognized: File truncated collect2: error: ld returned 1 exit status 

If this is not file-system related, then it is puzzling why it is failing this way. I am assuming that the compiler and linker on this system are sane. At any rate this error doesn't seem related to the PRs I have seen it on.

EDIT: could you also verify that /tmp has sufficient space on this system?

@ofrobots
Copy link
Contributor

All green: https://ci.nodejs.org/job/node-test-pull-request/3519/.

For posterity, the master (V8 5.1) version of the PR is #7833.

ofrobots pushed a commit that referenced this pull request Aug 3, 2016
Original commit message: [regexp] Fix case-insensitive matching for one-byte subjects. The bug occurs because we do not canonicalize character class ranges before adding case equivalents. While adding case equivalents, we abort early for one-byte subject strings, assuming that the ranges are sorted. Which they are not. [email protected] BUG=v8:5199 Review-Url: https://codereview.chromium.org/2159683002 Cr-Commit-Position: refs/heads/master@{#37833} Fixes: #7708 PR-URL: #7834 Ref: #7833 Reviewed-By: targos - Michaël Zasso <[email protected]> Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]> Reviewed-By: jasnell - James M Snell <[email protected]> Reviewed-By: ofrobots - Ali Ijaz Sheikh <[email protected]>
@ofrobots
Copy link
Contributor

Landed on v6.x as af63871, with V8 version bumped to 5.0.71.59.

@ofrobotsofrobots closed this Aug 3, 2016
@cjihrigcjihrig mentioned this pull request Aug 11, 2016
BethGriggs pushed a commit to ibmruntimes/node that referenced this pull request Aug 18, 2016
Original commit message: [regexp] Fix case-insensitive matching for one-byte subjects. The bug occurs because we do not canonicalize character class ranges before adding case equivalents. While adding case equivalents, we abort early for one-byte subject strings, assuming that the ranges are sorted. Which they are not. [email protected] BUG=v8:5199 Review-Url: https://codereview.chromium.org/2159683002 Cr-Commit-Position: refs/heads/master@{#37833} Fixes: nodejs/node#7708 PR-URL: nodejs/node#7834 Ref: nodejs/node#7833 Reviewed-By: targos - Michaël Zasso <[email protected]> Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]> Reviewed-By: jasnell - James M Snell <[email protected]> Reviewed-By: ofrobots - Ali Ijaz Sheikh <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@fhinkel@targos@bnoordhuis@jasnell@ofrobots@jbergstroem@nodejs-github-bot