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: use real_path to find plist files is update_ats Cocoapod util #42242

Closed

Conversation

chriszs
Copy link

@chriszs chriszs commented Jan 11, 2024

Summary:

This PR fixes a pod install failure detailed in #42239 which can occur when the update_ats_in_plist utility method in a Cocoapod script fails to find an Info.plist in the target folder. It does so by calling the Cocoapods real_path method instead of just path on the file reference, which correctly adds the target folder to the path.

Changelog:

[iOS] [Fixed] - Fix pod install error when update_ats unable to find Info.plist in targets

Test Plan:

Updated Ruby unit test:

ruby packages/react-native/scripts/cocoapods/__tests__/utils-test.rb

Also tested using patch-package against my reproduction branch in: chriszs/reproducer-react-native#2

To test, clone the branch, cd ReproducerApp, yarn, npx pod-install. CI fails with "error Cannot start server in new window because no terminal app was specified" on the build step, but I believe that's unrelated.

@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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 11, 2024
@chriszs
Copy link
Author

chriszs commented Jan 11, 2024

This does add the following to each Info.plist in the project, which overwrites a warning in the default app target Info.plist and causes a diff change to the Info.plist in the "Tests" target.

	<key>NSAppTransportSecurity</key>
	<dict>
		<key>NSAllowsArbitraryLoads</key>
		<false/>
		<key>NSAllowsLocalNetworking</key>
		<true/>
	</dict>
Screenshot 2024-01-11 at 6 39 27 PM

Have to consider how to deal with that, as well as Evan Bacon's point about not modifying targets that aren't running a React Native dev server.

@chriszs chriszs force-pushed the fix/use-full-path-in-update-ats branch from 4e87215 to 8a761ba Compare January 11, 2024 11:35
@chriszs chriszs changed the title fix: use full_path to find plist files is update_ats Cocoapod util fix: use real_path to find plist files is update_ats Cocoapod util Jan 11, 2024
@chriszs
Copy link
Author

chriszs commented Jan 11, 2024

Updated to use real_path instead, which is described as "The absolute path of the object resolving the source tree."


def initialize(name)
@name = name
@path = name
@real_path = name
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this accurate? Should we add the path to it also?

@chriszs chriszs force-pushed the fix/use-full-path-in-update-ats branch from 8a761ba to e7caf78 Compare January 11, 2024 11:59
@chriszs
Copy link
Author

chriszs commented Jan 11, 2024

Closing until I can better work out some of the issues here.

@chriszs chriszs closed this Jan 11, 2024
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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants