Skip to content

Support PBXFileSystemSynchronizedRootGroup merges - #139

Open
ivoidcat wants to merge 1 commit into
Lightricks:mainfrom
ivoidcat:main
Open

Support PBXFileSystemSynchronizedRootGroup merges#139
ivoidcat wants to merge 1 commit into
Lightricks:mainfrom
ivoidcat:main

Conversation

@ivoidcat

@ivoidcat ivoidcat commented Mar 5, 2026

Copy link
Copy Markdown

Summary

  • add handling for PBXFileSystemSynchronizedRootGroup in add_child_to_component
  • support attaching synchronized root groups under PBXNativeTarget.file_system_synchronized_groups
  • allow group handling paths to accept PBXFileSystemSynchronizedRootGroup where appropriate

Test plan

  • Build and install patched gem locally
  • Reproduce merge on a project containing fileSystemSynchronizedGroups
  • Verify conflict resolution no longer fails with unsupported component errors

Handle additions under PBXNativeTarget and group-like containers so Xcode 16 synchronized folder changes no longer fail as unsupported components.
@1880akshay

Akshay Jain (1880akshay) commented Jul 30, 2026

Copy link
Copy Markdown

Ben Yohay (@byohay) Barak Weiss (@barakwei) Yonathan Oren (@oryonatan) are we planning to merge this?

@noamfreeman

Copy link
Copy Markdown
Member

Thanks for picking this up!

can you share the pbxproj (or the diff) that motivated it? I want to
confirm whether the PBXFileSystemSynchronizedRootGroup is also referenced as a child of
mainGroup, because in the case I ran where it is, apply_group_additions drops it (it filters on
PBXGroup/PBXVariantGroup) and the folder ends up attached to the target but absent from the group
tree.

Relatedly, the PBXNativeTarget branch always calls project.new(...), whereas every other add path
here resolves the existing object (add_file_reference via hierarchy_path, add_reference_proxy
via find_reference_proxy) — if the group arrives from the group tree too, doesn't this end up with
two root groups for one folder? It's also missing the Settings.allow_duplicates guard the sibling
paths have.

Finally, could you add specs (every other supported isa has them), and drop the
respond_to?(:file_system_synchronized_groups) fallback plus the PBXGroup/sync-group no-op
branches — only PBXNativeTarget declares that has_many, and a sync root group has no children,
so none of those paths can fire.

@ivoidcat

Copy link
Copy Markdown
Author

Thanks for picking this up!

can you share the pbxproj (or the diff) that motivated it? I want to confirm whether the PBXFileSystemSynchronizedRootGroup is also referenced as a child of mainGroup, because in the case I ran where it is, apply_group_additions drops it (it filters on PBXGroup/PBXVariantGroup) and the folder ends up attached to the target but absent from the group tree.

Relatedly, the PBXNativeTarget branch always calls project.new(...), whereas every other add path here resolves the existing object (add_file_reference via hierarchy_path, add_reference_proxy via find_reference_proxy) — if the group arrives from the group tree too, doesn't this end up with two root groups for one folder? It's also missing the Settings.allow_duplicates guard the sibling paths have.

Finally, could you add specs (every other supported isa has them), and drop the respond_to?(:file_system_synchronized_groups) fallback plus the PBXGroup/sync-group no-op branches — only PBXNativeTarget declares that has_many, and a sync root group has no children, so none of those paths can fire.

Thanks for the detailed review.

Unfortunately, quite a bit of time has passed and I no longer have the original .pbxproj or diff that reproduced the issue, so I cannot confirm the exact object graph from that project.

As far as I remember, the project had been created or modified with Xcode 16 and contained a PBXFileSystemSynchronizedRootGroup. When Kintsugi applied the merge diff, that object appeared under PBXNativeTarget.fileSystemSynchronizedGroups, but add_child_to_component did not handle its isa, so the merge failed with an unsupported component error.

This patch was a narrow fix for that failure. It added dispatching for PBXFileSystemSynchronizedRootGroup and created and attached the object to PBXNativeTarget#file_system_synchronized_groups. That was sufficient to make the affected project merge successfully.

Looking at the implementation again, I agree that it may be incomplete if the same synchronized root group is also referenced from mainGroup. In that case, apply_group_additions currently filters it out, while the PBXNativeTarget path creates a new object instead of resolving an existing one, which could result in a missing group-tree entry or duplicate objects. I also agree that the respond_to? fallback and the group no-op branches are unnecessary.

I no longer have the original fixture to verify whether the synchronized root group was present in both locations, so this should be treated as a project-specific workaround rather than a complete implementation. A proper fix should handle the group in the mainGroup addition flow, reuse the same project object when attaching it to the target, respect Settings.allow_duplicates, and include specs for both references.

@noamfreeman

Copy link
Copy Markdown
Member

fair. it was a long time. is your project still using anything but buildable folders? if so, i'l try to add a proper fix.

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.

3 participants