From 61065603e5df4623c4c457921f7f4f9556e4cac6 Mon Sep 17 00:00:00 2001 From: Ryan Zulkoski Date: Thu, 10 Sep 2026 15:24:00 -0500 Subject: [PATCH] Support build files that reference a Swift package product A PBXBuildFile references either a file (fileRef) or a Swift package product (productRef, an XCSwiftPackageProductDependency). add_build_file assumed the former: it warned "Trying to add a build file without any file reference" and returned whenever fileRef was nil, silently dropping any build file that links a package product (e.g. a package framework in a Frameworks build phase). Warn and return only when both fileRef and productRef are absent, and dedup productRef build files by their full product reference (two products from different packages may share a product name). A package product dependency is shared within a target by its packageProductDependencies entry and the productRef of the build file that links it, so reuse an equivalent dependency already present in the same target rather than adding a duplicate (scoped per target, since Xcode keeps a separate dependency object per target). Co-Authored-By: Claude Opus 4.8 --- lib/kintsugi/apply_change_to_project.rb | 63 ++++++++- spec/kintsugi_apply_change_to_project_spec.rb | 132 ++++++++++++++++++ 2 files changed, 190 insertions(+), 5 deletions(-) diff --git a/lib/kintsugi/apply_change_to_project.rb b/lib/kintsugi/apply_change_to_project.rb index 0e925b2..27f0df8 100644 --- a/lib/kintsugi/apply_change_to_project.rb +++ b/lib/kintsugi/apply_change_to_project.rb @@ -744,10 +744,24 @@ def add_remote_swift_package_reference(containing_component, change, change_path end def add_swift_package_product_dependency(containing_component, change, change_path) + project = containing_component.project swift_package_product_dependency = - containing_component.project.new(Xcodeproj::Project::XCSwiftPackageProductDependency) + project.new(Xcodeproj::Project::XCSwiftPackageProductDependency) add_attributes_to_component(swift_package_product_dependency, change, change_path) + # Within a single target, the target's `packageProductDependencies` entry and the `productRef` + # of the build file that links the product are the same object. The diff adds it from both + # places, so reuse an equivalent dependency already present in the SAME target rather than + # adding a duplicate. The reuse is scoped to the target because Xcode keeps a separate + # dependency object per target. + target = owning_native_target(containing_component) + existing_dependency = + existing_package_product_dependency(target, swift_package_product_dependency) + unless existing_dependency.nil? + swift_package_product_dependency.remove_from_project + swift_package_product_dependency = existing_dependency + end + case containing_component when Xcodeproj::Project::PBXBuildFile containing_component.product_ref = swift_package_product_dependency @@ -759,6 +773,35 @@ def add_swift_package_product_dependency(containing_component, change, change_pa end end + # The native target that owns `component`: the target itself, or -- for a build file -- the + # target whose build phases contain it. Returns nil if none (e.g. the build phase is not yet + # linked to a target), in which case the caller adds a fresh dependency. + def owning_native_target(component) + return component if component.is_a?(Xcodeproj::Project::PBXNativeTarget) + return nil unless component.is_a?(Xcodeproj::Project::PBXBuildFile) + + component.project.native_targets.find do |target| + # Match by object identity, not `include?` (which compares by value): a freshly-created + # build file is still attribute-less here, and `PBXBuildFile#==` would treat it as equal to + # any other attribute-less build file, wrongly attributing it to an earlier target. + target.build_phases.any? do |build_phase| + build_phase.files.any? { |file| file.equal?(component) } + end + end + end + + # An existing package product dependency of `target` (either in its `packageProductDependencies` + # or referenced by one of its build files) equivalent to `dependency`, or nil if there is none. + def existing_package_product_dependency(target, dependency) + return nil if target.nil? + + candidates = target.package_product_dependencies.to_a + + target.build_phases.flat_map(&:files).map(&:product_ref).compact + candidates.uniq.find do |candidate| + !candidate.equal?(dependency) && candidate.to_tree_hash == dependency.to_tree_hash + end + end + def add_reference_proxy(containing_component, change, change_path) case containing_component when Xcodeproj::Project::PBXBuildFile @@ -1084,14 +1127,24 @@ def add_build_configuration(configuration_list, change, change_path) end def add_build_file(build_phase, change, change_path) - if change["fileRef"].nil? - puts "Warning: Trying to add a build file without any file reference to build phase " \ - "'#{build_phase}'" + # A build file references either a file (`fileRef`) or a Swift package product (`productRef`). + if change["fileRef"].nil? && change["productRef"].nil? + puts "Warning: Trying to add a build file without any file or product reference to build " \ + "phase '#{build_phase}'" return end existing_build_file = build_phase.files.find do |build_file| - build_file.file_ref && build_file.file_ref.path == change["fileRef"]["path"] + if change["fileRef"] + build_file.file_ref && build_file.file_ref.path == change["fileRef"]["path"] + else + # Compare the whole product reference, not just its name: two different Swift package + # products (from different packages) may share a product name, so a name-only match would + # wrongly drop a distinct build file. Comparing the full tree hash also distinguishes + # nameless products by their package. + build_file.product_ref && + build_file.product_ref.to_tree_hash == change["productRef"] + end end return if !Settings.allow_duplicates && !existing_build_file.nil? diff --git a/spec/kintsugi_apply_change_to_project_spec.rb b/spec/kintsugi_apply_change_to_project_spec.rb index a387acf..612be17 100644 --- a/spec/kintsugi_apply_change_to_project_spec.rb +++ b/spec/kintsugi_apply_change_to_project_spec.rb @@ -644,6 +644,138 @@ expect(base_project).to be_equivalent_to_project(theirs_project) end + it "adds a build file that references a swift package product" do + theirs_project = create_copy_of_project(base_project, "theirs") + build_file = theirs_project.new(Xcodeproj::Project::PBXBuildFile) + build_file.product_ref = create_swift_package_product_dependency(theirs_project) + theirs_project.targets[0].frameworks_build_phase.files << build_file + + changes_to_apply = get_diff(theirs_project, base_project) + + described_class.apply_change_to_project(base_project, changes_to_apply, theirs_project) + + expect(base_project).to be_equivalent_to_project(theirs_project) + end + + it "links a target and its build file to the same package product dependency" do + theirs_project = create_copy_of_project(base_project, "theirs") + dependency = create_swift_package_product_dependency(theirs_project) + theirs_project.targets[0].package_product_dependencies << dependency + build_file = theirs_project.new(Xcodeproj::Project::PBXBuildFile) + build_file.product_ref = dependency + theirs_project.targets[0].frameworks_build_phase.files << build_file + + changes_to_apply = get_diff(theirs_project, base_project) + + described_class.apply_change_to_project(base_project, changes_to_apply, theirs_project) + + expect(base_project).to be_equivalent_to_project(theirs_project) + + # The dependency is shared, as Xcode writes it: the build file's product_ref is the very same + # object as the target's package product dependency, not a duplicate. + dependencies = base_project.objects.select { |o| o.isa == "XCSwiftPackageProductDependency" } + expect(dependencies.count).to eq(1) + product_build_file = + base_project.targets[0].frameworks_build_phase.files.find(&:product_ref) + expect(product_build_file.product_ref) + .to equal(base_project.targets[0].package_product_dependencies.first) + end + + it "keeps a separate package product dependency per target for the same product" do + base_project.new_target("com.apple.product-type.library.static", "bar", :ios) + + theirs_project = create_copy_of_project(base_project, "theirs") + theirs_project.targets.each do |target| + dependency = create_swift_package_product_dependency(theirs_project) + target.package_product_dependencies << dependency + build_file = theirs_project.new(Xcodeproj::Project::PBXBuildFile) + build_file.product_ref = dependency + target.frameworks_build_phase.files << build_file + end + + changes_to_apply = get_diff(theirs_project, base_project) + + described_class.apply_change_to_project(base_project, changes_to_apply, theirs_project) + + expect(base_project).to be_equivalent_to_project(theirs_project) + + # Each target keeps its own dependency object -- Xcode does not share one across targets -- but + # within a target the build file's product_ref is that same object. + dependencies = base_project.objects.select { |o| o.isa == "XCSwiftPackageProductDependency" } + expect(dependencies.count).to eq(2) + base_project.targets.each do |target| + product_build_file = target.frameworks_build_phase.files.find(&:product_ref) + expect(product_build_file.product_ref).to equal(target.package_product_dependencies.first) + end + end + + it "keeps two products that share a name but come from different packages" do + base_dependency = create_swift_package_product_dependency(base_project) + base_project.targets[0].package_product_dependencies << base_dependency + base_build_file = base_project.new(Xcodeproj::Project::PBXBuildFile) + base_build_file.product_ref = base_dependency + base_project.targets[0].frameworks_build_phase.files << base_build_file + + # theirs links a second product with the SAME product name but from a different package. + theirs_project = create_copy_of_project(base_project, "theirs") + other_package = theirs_project.new(Xcodeproj::Project::XCRemoteSwiftPackageReference) + other_package.repositoryURL = "http://other" + other_dependency = theirs_project.new(Xcodeproj::Project::XCSwiftPackageProductDependency) + other_dependency.product_name = "foo" + other_dependency.package = other_package + theirs_project.targets[0].package_product_dependencies << other_dependency + other_build_file = theirs_project.new(Xcodeproj::Project::PBXBuildFile) + other_build_file.product_ref = other_dependency + theirs_project.targets[0].frameworks_build_phase.files << other_build_file + + changes_to_apply = get_diff(theirs_project, base_project) + + described_class.apply_change_to_project(base_project, changes_to_apply, theirs_project) + + expect(base_project).to be_equivalent_to_project(theirs_project) + + # Both same-named products are distinct (different packages), so both survive, each linked by + # its own build file -- a name-only dedup would have dropped the second build file. + dependencies = base_project.objects.select { |o| o.isa == "XCSwiftPackageProductDependency" } + expect(dependencies.count).to eq(2) + product_build_files = + base_project.targets[0].frameworks_build_phase.files.select(&:product_ref) + expect(product_build_files.count).to eq(2) + end + + it "attributes an added product build file to its own target despite a bare build file elsewhere" do + base_project.new_target("com.apple.product-type.library.static", "bar", :ios) + foo = base_project.targets.find { |target| target.display_name == "foo" } + # foo already links the product, and also carries a bare (reference-less) build file, which is + # value-equal to any freshly-created build file. + foo.package_product_dependencies << create_swift_package_product_dependency(base_project) + foo_build_file = base_project.new(Xcodeproj::Project::PBXBuildFile) + foo_build_file.product_ref = foo.package_product_dependencies.first + foo.frameworks_build_phase.files << foo_build_file + foo.frameworks_build_phase.files << base_project.new(Xcodeproj::Project::PBXBuildFile) + + # theirs links the same product in bar. + theirs_project = create_copy_of_project(base_project, "theirs") + theirs_bar = theirs_project.targets.find { |target| target.display_name == "bar" } + theirs_bar.package_product_dependencies << create_swift_package_product_dependency(theirs_project) + bar_build_file = theirs_project.new(Xcodeproj::Project::PBXBuildFile) + bar_build_file.product_ref = theirs_bar.package_product_dependencies.first + theirs_bar.frameworks_build_phase.files << bar_build_file + + changes_to_apply = get_diff(theirs_project, base_project) + + described_class.apply_change_to_project(base_project, changes_to_apply, theirs_project) + + expect(base_project).to be_equivalent_to_project(theirs_project) + + # bar's build file must link bar's own dependency, not foo's (a value-based target lookup would + # attribute it to foo via foo's bare build file, collapsing the two targets' dependencies). + merged_bar = base_project.targets.find { |target| target.display_name == "bar" } + bar_reference = merged_bar.frameworks_build_phase.files.map(&:product_ref).compact.first + expect(merged_bar.package_product_dependencies).to include(bar_reference) + expect(base_project.objects.count { |o| o.isa == "XCSwiftPackageProductDependency" }).to eq(2) + end + it "changes framework from reference proxy to file reference" do framework_filename = "baz"