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.

Sign up for GitHub

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

Jump to bottom

Setting current working directory for dev server. #7316

Closed
wants to merge 1 commit into from
Closed

Setting current working directory for dev server. #7316

wants to merge 1 commit into from

Conversation

jsdevel
Copy link
Contributor

@jsdevel jsdevel commented May 1, 2016 •
edited
Loading

  • This allows react-native to work for users on fedora.
  • react-native run-android was failing because the launch packager script was unable to find packager.sh (see source in man bash).
  • This change sets cwd for the dev server when run with run-android.

@ghost
Copy link

ghost commented May 1, 2016

By analyzing the blame information on this pull request, we identified @tadeuzagallo and @emilsjolander to be potential reviewers.

@ghost
Copy link

ghost commented May 1, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@jsdevel
Copy link
Contributor Author

jsdevel commented May 1, 2016 •
edited
Loading

Before Change

before-screenshot_2016-04-30-23-38-12

After Change

screenshot_2016-05-03-02-13-09

@jsdevel jsdevel mentioned this pull request May 1, 2016
Closed
@satya164
Copy link
Contributor

satya164 commented May 1, 2016

Seems tests are failing. Can you try rebasing against master?

@ghost
Copy link

ghost commented May 1, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 1, 2016
@jsdevel
Copy link
Contributor Author

jsdevel commented May 2, 2016

Hmm. I'm up to date with upstream master. Is there a way to re-run the failed build?

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from 828c810 to 3ce2c71 Compare May 2, 2016 08:47
@jsdevel
Copy link
Contributor Author

jsdevel commented May 2, 2016

@satya164 trying a different approach that should work in more environments.

@ghost
Copy link

ghost commented May 2, 2016

@jsdevel updated the pull request.

@jsdevel
Copy link
Contributor Author

jsdevel commented May 2, 2016

Interesting bit from man bash:

source filename [arguments]
              Read  and  execute  commands  from filename in the current shell
              environment and return the exit status of the last command  exe-
              cuted from filename.  If filename does not contain a slash, file
              names in PATH are used to find the  directory  containing  file-
              name.   The  file  searched  for in PATH need not be executable.
              When bash is  not  in  posix  mode,  the  current  directory  is
              searched  if no file is found in PATH.  If the sourcepath option
              to the shopt builtin command is turned  off,  the  PATH  is  not
              searched.

@jsdevel
Copy link
Contributor Author

jsdevel commented May 2, 2016

Build fixed @satya164. The ultimate fix is likely to set cwd when spawning the node process, but I feel that is a bit riskier considering that this will work for now.

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from 3ce2c71 to 70c21da Compare May 2, 2016 17:15
@ghost
Copy link

ghost commented May 2, 2016

@jsdevel updated the pull request.

@jsdevel
Copy link
Contributor Author

jsdevel commented May 2, 2016

So 2 of the builds in Travis are suddenly failing after rebasing upstream master (TEST_TYPE=objc and TEST_TYPE=js). I'm not sure exactly why, but it doesn't seem related to this change. Can those failed builds be restarted? I didn't see the rebuild button for those jobs.

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from 70c21da to 1b2c200 Compare May 3, 2016 09:07
@ghost
Copy link

ghost commented May 3, 2016

@jsdevel updated the pull request.

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from 1b2c200 to 4d3c052 Compare May 3, 2016 09:12
@ghost
Copy link

ghost commented May 3, 2016

@jsdevel updated the pull request.

@jsdevel jsdevel changed the title Fixing dev server on fedora. Setting current working directory for dev server. May 3, 2016
@jsdevel
Copy link
Contributor Author

jsdevel commented May 3, 2016

The ultimate fix is likely to set cwd when spawning the node process, but I feel that is a bit riskier considering that this will work for now.

Updated the PR to include this exact fix and it works great!

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from 4d3c052 to af2f552 Compare May 3, 2016 15:31
@ghost
Copy link

ghost commented May 3, 2016

@jsdevel updated the pull request.

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from af2f552 to 412b5cb Compare May 3, 2016 16:22
@ghost
Copy link

ghost commented May 3, 2016

@jsdevel updated the pull request.

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from 412b5cb to bb16e3d Compare May 3, 2016 16:48
@facebook-github-bot
Copy link
Contributor

@jsdevel updated the pull request.

@jsdevel
Copy link
Contributor Author

jsdevel commented May 3, 2016 •
edited
Loading

@nicklockwood @bestander @ide @chirag04 @tadeuzagallo @emilsjolander: Any chance I can get a review? This is a blocker for me on fedora right now.

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from bb16e3d to d232e29 Compare May 3, 2016 22:22
@ghost
Copy link

ghost commented May 3, 2016

@jsdevel updated the pull request.

@ide
Copy link
Contributor

ide commented May 3, 2016

Tests were passing except the JS test that is failing on master. LGTM so let's ship this and use the RC to see if it breaks anything.

@ide
Copy link
Contributor

ide commented May 3, 2016

@facebook-github-bot shipit

@jsdevel
Copy link
Contributor Author

jsdevel commented May 3, 2016

Thanks @ide !

@ghost ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels May 3, 2016
@facebook-github-bot
Copy link
Contributor

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

@satya164
Copy link
Contributor

satya164 commented May 4, 2016

@jsdevel Sorry, I was quite busy. Thanks @ide for taking this up.

@jsdevel
Copy link
Contributor Author

jsdevel commented May 4, 2016

no worries @satya164 !

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from d232e29 to fc5e1ec Compare May 4, 2016 04:50
@facebook-github-bot
Copy link
Contributor

@jsdevel updated the pull request.

@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from fc5e1ec to 404e6b7 Compare May 4, 2016 05:38
@ghost
Copy link

ghost commented May 4, 2016

@jsdevel updated the pull request.

@jsdevel
Fixing dev server current working directory.
a254927 
* This allows `react-native` to work for users on fedora.
* `react-native run-android` was failing because the launch packager script was unable to find packager.sh (see `source` in `man bash`).
* This change sets cwd for the dev server when run with `run-android`.
@jsdevel jsdevel force-pushed the fixing-packager-source-on-fedora branch from 404e6b7 to a254927 Compare May 4, 2016 06:54
@ghost
Copy link

ghost commented May 4, 2016

@jsdevel updated the pull request.

@ghost ghost closed this in d4cc5b5 May 4, 2016
@jsdevel jsdevel deleted the fixing-packager-source-on-fedora branch May 4, 2016 14:49
ptmt pushed a commit to ptmt/react-native that referenced this pull request May 9, 2016
@jsdevel @ptmt
Setting current working directory for dev server.
d6bee56 
Summary:
* This allows `react-native` to work for users on fedora.
* `react-native run-android` was failing because the launch packager script was unable to find packager.sh (see `source` in `man bash`).
* This change sets cwd for the dev server when run with `run-android`.
Closes facebook#7316

Differential Revision: D3255866

fb-gh-sync-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
fbshipit-source-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
zebulgar pushed a commit to nightingale/react-native that referenced this pull request Jun 18, 2016
@jsdevel @zebulgar
Setting current working directory for dev server.
219a18f 
Summary:
* This allows `react-native` to work for users on fedora.
* `react-native run-android` was failing because the launch packager script was unable to find packager.sh (see `source` in `man bash`).
* This change sets cwd for the dev server when run with `run-android`.
Closes facebook#7316

Differential Revision: D3255866

fb-gh-sync-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
fbshipit-source-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
samerce pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
@jsdevel @samerce
Setting current working directory for dev server.
5dc3ff4 
Summary:
* This allows `react-native` to work for users on fedora.
* `react-native run-android` was failing because the launch packager script was unable to find packager.sh (see `source` in `man bash`).
* This change sets cwd for the dev server when run with `run-android`.
Closes facebook#7316

Differential Revision: D3255866

fb-gh-sync-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
fbshipit-source-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
@jsdevel
Setting current working directory for dev server.
4695f84 
Summary:
* This allows `react-native` to work for users on fedora.
* `react-native run-android` was failing because the launch packager script was unable to find packager.sh (see `source` in `man bash`).
* This change sets cwd for the dev server when run with `run-android`.
Closes facebook/react-native#7316

Differential Revision: D3255866

fb-gh-sync-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
fbshipit-source-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
@jsdevel
Setting current working directory for dev server.
c68947e 
Summary:
* This allows `react-native` to work for users on fedora.
* `react-native run-android` was failing because the launch packager script was unable to find packager.sh (see `source` in `man bash`).
* This change sets cwd for the dev server when run with `run-android`.
Closes facebook/react-native#7316

Differential Revision: D3255866

fb-gh-sync-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
fbshipit-source-id: 88f6b18f7c61636ce8fecef77f7f4e60b1d9a637
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsdevel @satya164 @facebook-github-bot @ide