-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
[iOS] fixes Xcode path issues containing whitespace when building Release #24810
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code analysis results:
shellcheck
found some issues.
scripts/react-native-xcode.sh
Outdated
@@ -57,7 +57,7 @@ REACT_NATIVE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" | |||
# in node_modules. | |||
PROJECT_ROOT=${PROJECT_ROOT:-"$REACT_NATIVE_DIR/../.."} | |||
|
|||
cd $PROJECT_ROOT | |||
cd "$PROJECT_ROOT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SC2164: (warning) Use cd ... || exit in case cd fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
This pull request was successfully merged by Pierre Reimertz in f0770b6. When will my fix make it into a release? | Upcoming Releases |
Summary
Some users (me included) have projects located in quirky places, resulting whitespaces in the path.
An example, , I symlink into my iCloud drive which result in the following path
This commit quotes the variable to make sure it gets properly passed to the CLI.
For reference, similar issue that has been resolved: https://github.com/react-native/react-native/commit/7d4e94edccfc2f642fcbd1d6aa00756f02e3a525
Test Plan
I ran current tests without issues.
Any suggestions how to write a test that catch these issues in the future? I couldn't find any tests related to xcode builds.
Changelog
[iOS] [Fixed] - Fixes whitespace issues when building Release from Xcode