Update node-haste and replace fast-path with node-haste's version to fix Windows compatibility by janicduplessis · Pull Request #6260 · react/react-native

janicduplessis · 2016-03-03T05:36:53Z

This is the last bits needed to fix Windows compatibility on master, most of the work was done in node-haste.

Test plan
Run npm test
Run the packager using Windows and Mac

facebook-github-bot · 2016-03-03T05:37:19Z

By analyzing the blame information on this pull request, we identified @cpojer, @davidaurelio and @martinbigio to be potential reviewers.

davidaurelio · 2016-03-03T08:36:37Z

@facebook-github-bot shipit

facebook-github-bot · 2016-03-03T08:37:48Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

davidaurelio · 2016-03-03T08:39:08Z

@janicduplessis will have to take care of the node_modules updates internally first. Will look into it in a few. Maybe we could even get Travis back to green this morning before merging this

davidaurelio · 2016-03-03T09:58:23Z

This has internal errors. no idea whether it’s flakiness or really broken :-(

janicduplessis · 2016-03-03T11:21:46Z

Hmm the tests on circleci fails with this

Error: Cannot find module 'node-haste/lib/fastpath'
at Function.Module._resolveFilename (module.js:339:15)
at Function.Module._load (module.js:290:25)

Looks like the node-haste version doesn't get updated properly.

janicduplessis · 2016-03-03T11:30:37Z

Also the UIExplorer app is broken since a few commits, the tests on travis seems to fail because of that. If you have any internal tests that depend on that is may be why it fails.

davidaurelio · 2016-03-03T12:32:35Z

The UIExplorer have been reverted in the meantime afaik. @bestander is having a look at this now

bestander · 2016-03-03T12:47:58Z

@janicduplessis I would appreciate if you rebase.
CI should be stable-ish now

facebook-github-bot · 2016-03-03T13:32:16Z

@janicduplessis updated the pull request.

bestander · 2016-03-03T14:40:01Z

JS does not look good in tests

janicduplessis · 2016-03-03T16:53:43Z

I can confirm that it stills install node-haste@2.4 even if I changed the package.json. Is that caused by npm shrinkwrap? I'm not sure how that works as I've never used it before.

├─┬ node-haste@2.4.0 │ ├── graceful-fs@4.1.3 │ ├─┬ sane@1.3.3 │ │ ├─┬ exec-sh@0.2.0 │ │ │ └── merge@1.2.0

From the npm install logs.

ide · 2016-03-03T19:44:49Z

@janicduplessis package.json is completely ignored when shrinkwrap is present.

facebook-github-bot · 2016-03-04T02:48:17Z

@janicduplessis updated the pull request.

janicduplessis · 2016-03-04T03:57:56Z

Aww, e2e test on iOS fails with

 [Info] Collecting info for testables... (4131 ms)
run-test EndToEndTestTests.xctest (iphonesimulator9.2, iPhone 4s, application-test)
[Info] Installed 'org.reactjs.native.example.EndToEndTest'. (18420 ms)
[Info] Launching test host and running tests ... (0 ms)
-[EndToEndTestTests testRendersWelcomeScreen][3:20:55 AM] <START> request:/index.ios.bundle?platform=ios&dev=true
[3:20:55 AM] <START> find dependencies
Error: Unable to find file with path: /private/tmp/react-native-uiz3Ujzd/EndToEndTest/ios/index.ios.js
at Fastfs.readFile (/private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/lib/fastfs.js:141:15)
at /private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/lib/Module.js:165:49
at Cache.get (/private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/lib/Cache/index.js:64:103)
at Module.read (/private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/lib/Module.js:164:26)
at Module.getDependencies (/private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/lib/Module.js:120:19)
at /private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/lib/DependencyGraph/ResolutionRequest.js:29:17
at /private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/node_modules/throat/index.js:11:22
at new Promise (/private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/core-js/modules/es6.promise.js:197:7)
at run (/private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/node_modules/throat/index.js:10:22)
at /private/tmp/react-native-uiz3Ujzd/EndToEndTest/node_modules/react-native/node_modules/node-haste/node_modules/throat/index.js:39:16

janicduplessis · 2016-03-04T04:41:06Z

Ok the issue seems to happen because of a regression in node-haste 2.5.0 it tries to find the index of a new app in /Users/janic/Developer/TestApp/node_modules/react-native/packager/index.ios.js when it is in /Users/janic/Developer/TestApp/index.ios.js.

janicduplessis · 2016-03-04T05:51:30Z

Ok I was able to track down the bug to this change in node-haste, I'll check what is up with that.

facebookarchive/node-haste@9673f86#diff-1fdf421c05c1140f6d71444ea2b27638L198

janicduplessis · 2016-03-04T06:30:02Z

Also I'm not sure if I updated the npm-shrinkwrap.json file properly the diff seems a bit too huge :)

I did that using npm 2.14

rm -rf node_modules
npm install
npm install --save node-haste@2.5.0
npm uninstall --save fast-path
npm shrinkwrap

Not running npm shrinkwrap at the end didn't update the shrinkwrap file.

mkonicek · 2016-03-07T23:59:16Z

@janicduplessis I'm just merging #6346 which updates to a newer version. Should this PR be rebased or closed (not sure about the change in packager/react-packager/index.js)?

janicduplessis · 2016-03-08T00:02:38Z

I'm pretty sure the change in packager/react-packager/index.js is still needed. I'll rebase and test once #6346 is merged.

skevy · 2016-03-08T00:15:39Z

I think you're right @janicduplessis. Thanks for looking into this :)

Windows compatibility

facebook-github-bot · 2016-03-11T09:00:22Z

@janicduplessis updated the pull request.

davidaurelio · 2016-03-11T09:50:25Z

@facebook-github-bot shipit

facebook-github-bot · 2016-03-11T09:51:25Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

davidaurelio · 2016-03-11T14:04:17Z

@bestander @mkonicek you might want to cherry pick this for the release. It makes windows compat better

…fix Windows compatibility Summary:This is the last bits needed to fix Windows compatibility on master, most of the work was done in node-haste. **Test plan** Run npm test Run the packager using Windows and Mac cc cpojer davidaurelio Closes#6260 Reviewed By: dmmiller, bestander Differential Revision: D3005397 Pulled By: davidaurelio fb-gh-sync-id: e16847808ebfa8b234315b2093dba204c9c1e869 shipit-source-id: e16847808ebfa8b234315b2093dba204c9c1e869

…fix Windows compatibility Summary:This is the last bits needed to fix Windows compatibility on master, most of the work was done in node-haste. **Test plan** Run npm test Run the packager using Windows and Mac cc cpojer davidaurelio Closesreact/react-native#6260 Reviewed By: dmmiller, bestander Differential Revision: D3005397 Pulled By: davidaurelio fb-gh-sync-id: e16847808ebfa8b234315b2093dba204c9c1e869 shipit-source-id: e16847808ebfa8b234315b2093dba204c9c1e869

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 3, 2016

janicduplessis changed the title ~~Update node-haste and replace fast-path with node-haste's verrsion to fix Windows compatibility~~Update node-haste and replace fast-path with node-haste's version to fix Windows compatibilityMar 3, 2016

facebook-github-bot added GH Review: accepted and removed GH Review: review-needed labels Mar 3, 2016

janicduplessis closed this Mar 3, 2016

janicduplessis reopened this Mar 3, 2016

davidaurelio assigned bestander Mar 3, 2016

janicduplessis force-pushed the fix-windows branch from 8928b0a to 9e9a381Compare March 4, 2016 02:47

janicduplessis mentioned this pull request Mar 4, 2016
Fix relative entry paths facebookarchive/node-haste#49
Merged

janicduplessis mentioned this pull request Mar 7, 2016
cannot find entry file ...UIExplorerApp.android.js in any of roots #6331
Closed

janicduplessis mentioned this pull request Mar 8, 2016
Issue to track 0.22 stability #6366
Closed
7 tasks

Update node-haste and replace fast-path with node-haste's version to fix
b7c5ded
Windows compatibility

janicduplessis force-pushed the fix-windows branch from 9e9a381 to b7c5dedCompare March 11, 2016 09:00

ghost closed this in fd816b1Mar 11, 2016

janicduplessis deleted the fix-windows branch March 14, 2016 03:04

ck227 mentioned this pull request Mar 17, 2016
[Windows] Process 'command 'E:\android-ndk-r10e\ndk-build.cmd'' finished with non-zero exit value 2 #3107
Closed

This pull request was closed.

Uh oh!

Conversation

janicduplessis commented Mar 3, 2016

Uh oh!

facebook-github-bot commented Mar 3, 2016

Uh oh!

davidaurelio commented Mar 3, 2016

Uh oh!

facebook-github-bot commented Mar 3, 2016

Uh oh!

davidaurelio commented Mar 3, 2016

Uh oh!

davidaurelio commented Mar 3, 2016

Uh oh!

janicduplessis commented Mar 3, 2016

Uh oh!

janicduplessis commented Mar 3, 2016

Uh oh!

davidaurelio commented Mar 3, 2016

Uh oh!

bestander commented Mar 3, 2016

Uh oh!

facebook-github-bot commented Mar 3, 2016

Uh oh!

bestander commented Mar 3, 2016

Uh oh!

janicduplessis commented Mar 3, 2016

Uh oh!

ide commented Mar 3, 2016

Uh oh!

facebook-github-bot commented Mar 4, 2016

Uh oh!

janicduplessis commented Mar 4, 2016

Uh oh!

janicduplessis commented Mar 4, 2016

Uh oh!

janicduplessis commented Mar 4, 2016

Uh oh!

janicduplessis commented Mar 4, 2016

Uh oh!

mkonicek commented Mar 7, 2016

Uh oh!

janicduplessis commented Mar 8, 2016

Uh oh!

skevy commented Mar 8, 2016

Uh oh!

facebook-github-bot commented Mar 11, 2016

Uh oh!

davidaurelio commented Mar 11, 2016

Uh oh!

facebook-github-bot commented Mar 11, 2016

Uh oh!

davidaurelio commented Mar 11, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants