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

Remove the line from .gitattributes that treats pbxproj files as binary #1561

Closed
wants to merge 1 commit into from

Conversation

jawwad
Copy link
Contributor

@jawwad jawwad commented Nov 5, 2020

Summary:
The "*.pbxproj binary merge=union" line was added in a prior commit (D18655686 (c54b37a)) as a way to automatically resolve merge conflicts in Xcode project files based on this article: https://thoughtbot.com/blog/xcode-and-git-bridging-the-gap

The article doesn't specify why the pbxproj file should be treated as a binary, however one side effect is that the git diff command will no longer show changes in the project file.

For example if a project file is changed it will just show the following:

Binary files a/FBSDKGamingServicesKit/FBSDKGamingServicesKit.xcodeproj/project.pbxproj and b/FBSDKGamingServicesKit/FBSDKGamingServicesKit.xcodeproj/project.pbxproj differ

It doesn't seem like the binary attribute is necessary to use the merge=union strategy as the following article
https://roadfiresoftware.com/2015/09/automatically-resolving-git-merge-conflicts-in-xcodes-project-pbxproj/

However I'm not convinced that we need the merge=union strategy since resolving conflicts in project files is fairly simple and mostly only occurs when two files have been added in the same location in the project. Even then the conflict is easy to resolve.

Furthermore it doesn't look like this strategy works reliably. This StackOverflow answer indicates that the user tried this strategy and ended up removing it since it didn't work in about 1 of 4 cases: https://stackoverflow.com/a/18275082. (When it doesn't work it will prevent the project from being opened at all and would have to fixed manually anyway).

And finally, if there is a conflict between two similarly named files that have been added to the location in the project file, resolving the conflict manually allows the alphabetical order of the files to be corrected if needed.

Reviewed By: joesus

Differential Revision: D24762186

Summary:
The "*.pbxproj binary merge=union" line was added in a prior commit (D18655686 (facebook@c54b37a)) as a way to automatically resolve merge conflicts in Xcode project files based on this article: https://thoughtbot.com/blog/xcode-and-git-bridging-the-gap

The article doesn't specify why the pbxproj file should be treated as a binary, however one side effect is that the `git diff` command will no longer show changes in the project file.

For example if a project file is changed it will just show the following:
```
Binary files a/FBSDKGamingServicesKit/FBSDKGamingServicesKit.xcodeproj/project.pbxproj and b/FBSDKGamingServicesKit/FBSDKGamingServicesKit.xcodeproj/project.pbxproj differ
```

It doesn't seem like the binary attribute is necessary to use the merge=union strategy as the following article
https://roadfiresoftware.com/2015/09/automatically-resolving-git-merge-conflicts-in-xcodes-project-pbxproj/

However I'm not convinced that we need the merge=union strategy since resolving conflicts in project files is fairly simple and mostly only occurs when two files have been added in the same location in the project. Even then the conflict is easy to resolve.

Furthermore it doesn't look like this strategy works reliably. This StackOverflow answer indicates that the user tried this strategy and ended up removing it since it didn't work in about 1 of 4 cases: https://stackoverflow.com/a/18275082. (When it doesn't work it will prevent the project from being opened at all and would have to fixed manually anyway).

And finally, if there is a conflict between two similarly named files that have been added to the location in the project file, resolving the conflict manually allows the alphabetical order of the files to be corrected if needed.

Reviewed By: joesus

Differential Revision: D24762186

fbshipit-source-id: fc07d94bd00aa5e7219cd9bd4885464671ba5265
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24762186

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 508983d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants