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

use g++-4.8 (support C++11) and start testing against Node.js version 4.0 on Travis CI #105

Merged
merged 3 commits into from
Sep 26, 2015

Conversation

Mithgol
Copy link
Contributor

@Mithgol Mithgol commented Sep 10, 2015

This pull request adds Node.js version 4.0 testing.

I've noticed that the tests fail on io.js and Node 4.0.

There are two separate problems here.

The first problem is that the iconv package (needed for testing) won't build.

I've thought (initially) that these engine versions need a newer toolchain environment setting (CXX=g++-4.8) as implied by travis-ci/travis-ci#4771 and netiam/experiments@b700b51, but the actual problem seems to be something else.

The second problem is that the iconv-lite's behaviour is itself not quite right.

I have performed some manual testing on Windows in Node.js v4.0.0, and the results don't seem correct:

(screenshot)

The Pile of Poo Test™ seems to fail.

The results of such test were good enough in Node.js v0.12.x and v0.10.x where the result of Buffer('\uD83D\uDCA9', 'cp866').toString('cp866') was '??' (i.e. one question mark for each half of the surrogate pair).

@Mithgol
Copy link
Contributor Author

Mithgol commented Sep 11, 2015

The first of the above problems seems to be a problem of iconv and, as such, there it should be dealt with. (bnoordhuis/node-iconv#132)

@bnoordhuis
Copy link

See my comment here: the problem is with the default C++ compiler, it's too old to understand C++11.

@Mithgol
Copy link
Contributor Author

Mithgol commented Sep 11, 2015

Thanks, the first problem is gone.

Travis CI job logs for iojs and Node.js v4.0.0 now seem to indicate that iconv-lite can no longer extend Node's native set of encodings correctly.

@Mithgol Mithgol changed the title try on Node.js version 4.0 using Travis CI services use g++-4.8 (support C++11) and start testing against Node.js version 4.0 on Travis CI Sep 11, 2015
@dougwilson
Copy link
Contributor

It also looks like Node.js 4.0 can no longer natively decode CESU-8.

@bnoordhuis
Copy link

Yes, that was removed in nodejs/node@70d1f32 ("deps: update v8 to 4.4.63.9"):

Notable backwards incompatible changes are the removal of the smalloc module and dropped support for CESU-8 decoding. CESU-8 support can be brought back if necessary by doing UTF-8 decoding ourselves.

Long story short: V8 string functions no longer accept CESU-8 as input. Node would have to implement its own UTF-8-to-UTF-16 encoder.

@Mithgol
Copy link
Contributor Author

Mithgol commented Sep 12, 2015

Created some Buffers:

(screenshot)

Their lengths are fine, but their contents seem quite random.

I suspect that the new version of Node's Buffer constructor (introduced by a rename in nodejs/node@1057d11 and developed elsewhere before that) just isn't as extendable as its previous version. I've opened an issue (nodejs/node#2835) about that.

ashtuchkin added a commit that referenced this pull request Sep 26, 2015
use g++-4.8 (support C++11) and start testing against Node.js version 4.0 on Travis CI
@ashtuchkin ashtuchkin merged commit cee3d68 into ashtuchkin:master Sep 26, 2015
@ashtuchkin
Copy link
Owner

Thanks @Mithgol! I fixed the CESU-8 and added warnings for extendNodeEncodings. Seems we cannot do anything at the moment with the latter issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants