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

fix: pick correct TARGET_BUILD_DIR for run-ios #1032

Merged
merged 2 commits into from
Mar 4, 2020
Merged

fix: pick correct TARGET_BUILD_DIR for run-ios #1032

merged 2 commits into from
Mar 4, 2020

Conversation

FDiskas
Copy link
Contributor

@FDiskas FDiskas commented Mar 3, 2020

Summary:

Related: facebook/react-native#21303
Related: facebook/react-native#14423

Test Plan:

I'm getting wrong build target because some pods are also specified a target and this was in xcode env. The grep just matches first but not always the right TARGET. So instead of matching use JSON parsing.

@FDiskas
Copy link
Contributor Author

FDiskas commented Mar 3, 2020

In my case xcode ENV causing this error was:

...
PODS_ROOT = /Users/***/client-app/ios/Pods\n' +
PODS_TARGET_SRCROOT = /Users/***/client-app/ios/Pods/../../node_modules/react-native\n' +
PRECOMPS_INCLUDE_HEADERS_FROM_BUILT_PRODUCTS_DIR = YES\n' +
PRECOMP_DESTINATION_DIR = /Users/***/client-app/ios/build/Pods.build/***-Dev-Debug-iphonesimulator/React.build/PrefixHeaders\n' +
PRESERVE_DEAD_CODE_INITS_AND_TERMS = NO\n' +
PRODUCT_BUNDLE_IDENTIFIER = org.cocoapods.React\n' +
PRODUCT_MODULE_NAME = React\n' +
PRODUCT_NAME = React\n' +
PRODUCT_SETTINGS_PATH = \n' +
...
TARGETNAME = React\n' +
TARGET_BUILD_DIR = /Users/***/client-app/ios/build/***-Dev-Debug-iphonesimulator/React\n' +

And that React - i'm not sure where it come from.

@FDiskas
Copy link
Contributor Author

FDiskas commented Mar 3, 2020

Also there more build settings in react-native init - there is a differences https://www.diffchecker.com/B6LgFARh

@FDiskas
Copy link
Contributor Author

FDiskas commented Mar 3, 2020

Tested on my broken project (now works) and also tested on react-native boilerplate also works

@janicduplessis
Copy link
Contributor

@leethree Could you verify if this fixes your issue?

@FDiskas
Copy link
Contributor Author

FDiskas commented Mar 3, 2020

Yes it does

@FDiskas
Copy link
Contributor Author

FDiskas commented Mar 3, 2020

More info about WRAPPER_EXTENSION setting

For building an application the value of this variable will be app and for a unit test it will be xctest. This will then create a variable reference to either $(CURRENT_PROJECT_VERSION_app) or $(CURRENT_PROJECT_VERSION_xctest) and assign the respective value associated with that to CURRENT_PROJECT_VERSION.

Reference: https://pewpewthespells.com/blog/xcconfig_guide.html

@thymikee thymikee changed the title ci: fix wrong TARGET_BUILD_DIR fix: pick correct TARGET_BUILD_DIR for run-ios Mar 4, 2020
@thymikee thymikee merged commit 45edfd0 into react-native-community:master Mar 4, 2020
@FDiskas FDiskas deleted the bugfix/wrong-target-build-dir branch March 4, 2020 09:02
@emac3
Copy link

emac3 commented Apr 3, 2020

would have been nice to have this for cli ^3.0.0 (react-native ^0.61.0), 'cause this fixes my issue where I had the wrong TARGET_BUILD_DIR: "/Users//Library/Developer/Xcode/DerivedData/<project_name>/Build/Products/Debug-iphonesimulator/React/<app_name>.app" (instead of "/Users//Library/Developer/Xcode/DerivedData/<project_name>/Build/Products/Debug-iphonesimulator/<app_name>.app")

@FDiskas
Copy link
Contributor Author

FDiskas commented Apr 3, 2020

React > v0.62.0 has this

@thymikee
Copy link
Member

thymikee commented Apr 3, 2020

I think we could cherry-pick this to 3.x

hudon pushed a commit to entrc/cli that referenced this pull request Apr 17, 2020
…y#1032)

* ci: fix wrong TARGET_BUILD_DIR

* ci: better version of detecting right build settings
thymikee pushed a commit that referenced this pull request Jun 19, 2020
* ci: fix wrong TARGET_BUILD_DIR

* ci: better version of detecting right build settings

Co-authored-by: Vytenis <projektas@gmail.com>
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