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

Support io.js 1.0 #78

Closed
wants to merge 1 commit into from
Closed

Support io.js 1.0 #78

wants to merge 1 commit into from

Conversation

bajtos
Copy link

@bajtos bajtos commented Jan 16, 2015

Fix the condition used to check whether streams are modern, correctly detect io.js 1.0 as a modern engine.

This fixes the install script used by phantomjs which hangs inside ncp at the moment.

I was not able to reproduce the problem locally inside ncp, thus there is no test to cover this change :(

@mmalecki @AvianFlu Could you please review, land and release this patch as soon as reasonably possible? ncp is used e.g. by Karma test runner (karma-phantomjs-launcher), thus most front-end people cannot upgrade to io.js until this fix is landed.

Fix the condition used to check whether streams are modern, correctly
detect io.js 1.0 as a modern engine.
@bajtos
Copy link
Author

bajtos commented Jan 16, 2015

FWIW, the test failure seems like a timing problem, tests sometimes fail sometimes pass when run on my machine.

@bajtos
Copy link
Author

bajtos commented Jan 16, 2015

Workaround for phantomjs, until this patch is landed: install phantomjs globally, e.g. via brew install phantomjs. npm install phantomjs will pick the global version and skip the failing install.js script.

@AvianFlu
Copy link
Owner

Sorry, didn't see this until just now.

Any insight on those across-the-board test failures from Travis?

@bajtos
Copy link
Author

bajtos commented Jan 22, 2015

Any insight on those across-the-board test failures from Travis?

IIRC, the test "ncp modified files copies change source file mtime and copy" fails on some runs and then passes on subsequent runs. I assume it's a timing issue in the test, but I don't have time to investigate it myself.

@bajtos
Copy link
Author

bajtos commented Jan 22, 2015

You can try to re-run the CI jobs, possibly repeatedly, and hopefully after few runs the tests will be green.

stefanpenner added a commit to ember-cli/ember-cli that referenced this pull request Jan 27, 2015
stefanpenner added a commit to ember-cli/ember-cli that referenced this pull request Jan 27, 2015
kielni pushed a commit to kielni/ember-cli that referenced this pull request Feb 5, 2015
rmg added a commit to strongloop/strong-cached-install that referenced this pull request Feb 7, 2015
Necessary for ncp to work with iojs, which is necessary for
generator-loopback tests to pass on iojs.
johanneswuerbach pushed a commit to johanneswuerbach/ember-cli that referenced this pull request Feb 9, 2015
@bajtos
Copy link
Author

bajtos commented Feb 18, 2015

Closing in favour of #80 and/or cpr.

@bajtos bajtos closed this Feb 18, 2015
@bajtos bajtos deleted the fix/support-iojs branch February 18, 2015 08:45
bajtos pushed a commit to strongloop/strong-cached-install that referenced this pull request Feb 24, 2015
 * Upgrade ncp to ^2.0 to support io.js (Miroslav Bajtoš)

 * deps: use patched ncp pending AvianFlu/ncp#78 (Ryan Graham)

 * Fix bad CLA URL in CONTRIBUTING.md (Ryan Graham)
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.

2 participants