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(ios): fix pod install --project-directory=ios failing #37992

Closed
wants to merge 3 commits into from

Conversation

tido64
Copy link
Collaborator

@tido64 tido64 commented Jun 21, 2023

Summary:

image

pod install --project-directory=ios currently fails when new arch is enabled: react-native-async-storage/async-storage#910

Changelog:

[IOS] [FIXED] - Fix pod install --project-directory=... when New Arch is enabled

Test Plan:

git clone https://github.com/react-native-async-storage/async-storage.git
cd async-storage
gh pr checkout 910
yarn
RCT_NEW_ARCH_ENABLED=1 pod install --project-directory=example/ios

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Jun 21, 2023
@cipolleschi
Copy link
Contributor

sorry that this is happening.. :(

  1. CircleCI is still red
  2. Do you mind adding a CircleCI test to keep this in check?

We can't really use the React Native repo to keep that checked, but we can create a job that prepare the template and rearranges the files so that we can invoke pod install with that variation.

what do you think?

@tido64
Copy link
Collaborator Author

tido64 commented Jun 21, 2023

sorry that this is happening.. :(

No worries, I'm sorry I didn't catch this earlier 😄

  1. CircleCI is still red

Btw, did someone make changes to Danger bot? The changelog format should be correct but it's still complaining.

  1. Do you mind adding a CircleCI test to keep this in check?

We can't really use the React Native repo to keep that checked, but we can create a job that prepare the template and rearranges the files so that we can invoke pod install with that variation.

what do you think?

Would it be enough to modify this slightly to run pod install from the parent directory:

bundle install
bundle exec pod install

@kelset
Copy link
Contributor

kelset commented Jun 21, 2023

Btw, did someone make changes to Danger bot? The changelog format should be correct but it's still complaining.

It might be because you need the ":" in the title for "Changelog" -> "Changelog:"

@cipolleschi
Copy link
Contributor

Would it be enough to modify this slightly to run pod install from the parent directory:
react-native/.circleci/config.yml
Lines 907 to 908 in b3bb553

        bundle install 
        bundle exec pod install

you mean with something like:

bundle install 
+ cd ..
-bundle exec pod install
+bundle exec pod install --project-directory=ios

We can try, but I am not sure it will work.
My understanding is that we need to run pod install from the same directory where the Podfile is. So we should actually move the podfile to the parent directory first. Am I right?

@tido64
Copy link
Collaborator Author

tido64 commented Jun 21, 2023

We can try, but I am not sure it will work. My understanding is that we need to run pod install from the same directory where the Podfile is. So we should actually move the podfile to the parent directory first. Am I right?

No, the whole purpose of --project-directory is to be able to point at the directory that contains Podfile. Let me try it in a draft PR.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,958,132 +5
android hermes armeabi-v7a 8,219,318 +3
android hermes x86 9,471,795 +2
android hermes x86_64 9,314,263 +3
android jsc arm64-v8a 9,519,066 +3
android jsc armeabi-v7a 8,657,595 +2
android jsc x86 9,603,606 +1
android jsc x86_64 9,850,183 +3

Base commit: 7a2a327
Branch: main

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@github-actions
Copy link

This pull request was successfully merged by @tido64 in eb5f23c.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions bot added the Merged This PR has been merged. label Jun 23, 2023
@tido64 tido64 deleted the tido/fix-codegen branch June 23, 2023 10:41
facebook-github-bot pushed a commit that referenced this pull request Jun 26, 2023
Summary:
Exercise `pod install --project-directory=ios` when building the generated iOS project to make sure we don't regress in the future.

See also #37992, #35754, #34215, https://github.com/facebook/react-native/issues/33909…

## Changelog:

[INTERNAL] [ADDED] - Exercise `pod install --project-directory=ios` when testing the iOS template

Pull Request resolved: #37996

Test Plan: CI should pass.

Reviewed By: dmytrorykun

Differential Revision: D46972815

Pulled By: cipolleschi

fbshipit-source-id: 69617b09ac599eba3dde3ddddcf08db95bfc4da3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants