fix(ios): fix pod install --project-directory=...

[tido64]

Summary

Jokes aside, I think the breaking change was only introduced on the 0.71-stable branch. So no need to fix anything on main.

Changelog

[IOS] [FIXED] - Fix pod install --project-directory=...

[kelset]

oh wait I just realized this PR is directly for 0.71-stable; @tido64 did you replicate this change for main branch as a separate PR, or there's no need there?

[tido64]

oh wait I just realized this PR is directly for 0.71-stable; @tido64 did you replicate this change for main branch as a separate PR, or there's no need there?

It's not needed on main afaict. The offending change was only introduced on the 0.71-stable branch.

[kelset]

sounds good - I've added it to the discussion, the CI issues on iOS are related to a problem with state of branch post release, so nothing to worry about. We'll be able to squash&merge when preparing next patch release 👍

[tido64]

sounds good - I've added it to the discussion, the CI issues on iOS are related to a problem with state of branch post release, so nothing to worry about. We'll be able to squash&merge when preparing next patch release 👍

Thanks ❤️

[cortinico]

The offending change was only introduced on the 0.71-stable branch.

What's the offending change out of curiousity?

[tido64]

The offending change was only introduced on the 0.71-stable branch.

What's the offending change out of curiousity?

8f2c7ff

[cipolleschi]

As done like this, it breaks RNTester, unfortunately. Should we find a better way to handle this? 🤔

[tido64]

How does it break RNTester? I printed out the output and it looks correct to me?

 Generating Pods project
+"/~/react-native/packages/rn-tester/../../package.json"
Setting REACT_NATIVE build settings
Setting CLANG_CXX_LANGUAGE_STANDARD to c++17 on /~/react-native/packages/rn-tester/RNTesterPods.xcodeproj
Pod install took 12 [s] to run
Integrating client project
Pod installation complete! There are 67 dependencies from the Podfile and 55 total pods installed.

The installation also succeeded…

[cipolleschi]

my bad the RNTester and CircleCI issue is due to hermes-engine versions... 🤦

[cipolleschi]

On the joke, I think it is very spot on, actually.

Can we create a job in CI that tests that use case? I think is important that we make sure we don't regress. PErhaps we can move the discussion in another place, but what would be a good example/way to test it?
Would be enough to do something like:

npx react-native init NewApp --skip-install
cd NewApp # to move into the NewApp folder
yarn install # to install yarn dependencies
pod install --project-directory ios/ # to keep it tested

?

[cortinico]

Can we create a job in CI that tests that use case?

Yes please. We broke this too often that it's really worth to just add a small test in one of our template tests.

[tido64]

On the joke, I think it is very spot on, actually.

Can we create a job in CI that tests that use case? I think is important that we make sure we don't regress. PErhaps we can move the discussion in another place, but what would be a good example/way to test it? Would be enough to do something like:

npx react-native init NewApp --skip-install
cd NewApp # to move into the NewApp folder
yarn install # to install yarn dependencies
pod install --project-directory ios/ # to keep it tested

?

This is what we do in react-native-test-app and it catches most cases but it's unfortunately not enough. There are community repos (AsyncStorage and Webview come to mind) that have a structure like:

root
├── example
│ ├── android
│ │ └── build.gradle
│ ├── ios
│ │ └── Podfile
│ └── App.js
├── package.json
└── src

Where you run pod install --project-directory=example/ios. This is how I found the breakage. I don't have a good suggestion for an alternative solution other than replicating the folder structure somehow.

[cortinico]

Where you run pod install --project-directory=example/ios. This is how I found the breakage. I don't have a good suggestion for an alternative solution other than replicating the folder structure somehow.

I think we should add a template job that tests a commit of RN against create-react-native-library (which has the same structure you suggested) and use --project-directory=

That would help us cover a lot of scenarios that are library specific and that we broke in the past.

[kelset]

I think we should add a template job that tests a commit of RN against create-react-native-library (which has the same structure you suggested) and use --project-directory=

That would help us cover a lot of scenarios that are library specific and that we broke in the past

@cipolleschi@cortinico please remember that this PR is against 0.71 to address the specific problem, if we want to do what you propose it should be handled as a separate work in main branch

[cortinico]

@cipolleschi@cortinico please remember that this PR is against 0.71 to address the specific problem, if we want to do what you propose it should be handled as a separate work in main branch

Yup I was just brainstorming ideas to prevent this from happening again, if that could help @tido64

[tido64]

I think we should add a template job that tests a commit of RN against create-react-native-library (which has the same structure you suggested) and use --project-directory=

That would help us cover a lot of scenarios that are library specific and that we broke in the past.

I think this is a good suggestion. It will catch most scenarios like you said.