Skip to content

Commit 96bab74

Browse files
coadofacebook-github-bot
coado
authored and
facebook-github-bot
committed
Fix dynamic-framework header resolution for stable C++ umbrella headers (#58679)
Summary: Fix dynamic-framework header resolution for stable C++ umbrella headers. Header paths are now added only to targets that depend on the owning framework, including transitively, without exposing a framework’s built headers to its own compilation. ## Changelog: [Internal] Test Plan: - Focused CocoaPods tests: 16 tests, 27 assertions, all passing. - Dynamic-framework CocoaPods generation: passing. - Verified React-RuntimeApple → React-RCTFabric → React-Fabric receives the React-Fabric umbrella path. - Verified React-Fabric does not receive its own built-framework path. Differential Revision: D121777587 Pulled By: coado
1 parent 361bc24 commit 96bab74

3 files changed

Lines changed: 147 additions & 10 deletions

File tree

‎packages/react-native/ReactCommon/React-Fabric.podspec‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ Pod::Spec.new do |s|
141141

142142
s.subspec "components" do |ss|
143143
ss.subspec "root" do |sss|
144+
sss.dependency "React-Fabric/coreUmbrella"
145+
sss.dependency "React-Fabric/components/viewUmbrella"
144146
sss.source_files = podspec_sources("react/renderer/components/root/**/*.{m,mm,cpp,h}", "react/renderer/components/root/**/*.{h}")
145147
sss.exclude_files = "react/renderer/components/root/tests"
146148
sss.header_dir = "react/renderer/components/root"

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

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,12 @@ def test_updateSearchPaths_whenUseFrameworks_addsSearchPaths
732732
second_target
733733
]
734734
)
735-
pods_projects_mock = PodsProjectMock.new([third_target], {"hermes-engine" => {}})
735+
pods_projects_mock = PodsProjectMock.new(
736+
[third_target],
737+
{"hermes-engine" => {}},
738+
"test/path-pod.xcodeproj",
739+
[prepare_config("Debug"), prepare_config("Release")]
740+
)
736741
installer = InstallerMock.new(pods_projects_mock, [
737742
AggregatedProjectMock.new(user_project_mock)
738743
])
@@ -743,10 +748,15 @@ def test_updateSearchPaths_whenUseFrameworks_addsSearchPaths
743748
# Assert
744749
user_project_mock.build_configurations.each do |config|
745750
received_search_path = config.build_settings["HEADER_SEARCH_PATHS"]
746-
expected_search_path = "$(inherited) ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core ${PODS_CONFIGURATION_BUILD_DIR}/React-runtimeexecutor/React_runtimeexecutor.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-runtimeexecutor/React_runtimeexecutor.framework/Headers/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/components/view/platform/cxx ${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-rendererdebug/React_rendererdebug.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-timing/React_timing.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-NativeModulesApple/React_NativeModulesApple.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/React-featureflags/React_featureflags.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-renderercss/React_renderercss.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-cxxstableapi/React_cxxstableapi.framework/Headers"
751+
expected_search_path = "$(inherited) ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core ${PODS_CONFIGURATION_BUILD_DIR}/React-runtimeexecutor/React_runtimeexecutor.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-runtimeexecutor/React_runtimeexecutor.framework/Headers/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/components/view/platform/cxx ${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/components/scrollview/platform/cxx ${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/components/scrollview/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-rendererdebug/React_rendererdebug.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-timing/React_timing.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-FabricComponents/React_FabricComponents.framework/Headers/react/renderer/textlayoutmanager/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/React-FabricComponents/React_FabricComponents.framework/Headers/react/renderer/components/text/platform/cxx ${PODS_CONFIGURATION_BUILD_DIR}/React-FabricComponents/React_FabricComponents.framework/Headers/react/renderer/components/textinput/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/React-FabricComponents/React_FabricComponents.framework/Headers/react/renderer/components/switch/iosswitch ${PODS_CONFIGURATION_BUILD_DIR}/React-NativeModulesApple/React_NativeModulesApple.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-bridging/React_bridging.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios ${PODS_CONFIGURATION_BUILD_DIR}/React-featureflags/React_featureflags.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-renderercss/React_renderercss.framework/Headers ${PODS_CONFIGURATION_BUILD_DIR}/React-cxxstableapi/React_cxxstableapi.framework/Headers"
747752
assert_equal(expected_search_path, received_search_path)
748753
end
749754

755+
pods_projects_mock.build_configurations.each do |config|
756+
received_search_paths = Shellwords.shellsplit(config.build_settings["HEADER_SEARCH_PATHS"])
757+
assert_false(received_search_paths.include?("${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers"))
758+
end
759+
750760
installer.target_installation_results.pod_target_installation_results.each do |pod_name, target_installation_result|
751761
if pod_name == "SecondTarget"
752762
target_installation_result.native_target.build_configurations.each do |config|
@@ -808,6 +818,20 @@ def test_add_search_path_if_not_included_does_not_add_existing_path_to_string
808818
assert_equal("/path/to/headers /another/path", result)
809819
end
810820

821+
def test_add_search_path_if_not_included_adds_path_that_is_prefix_of_existing_path
822+
current_paths = "/path/to/headers/platform/ios"
823+
new_path = "/path/to/headers"
824+
result = ReactNativePodsUtils.add_search_path_if_not_included(current_paths, new_path)
825+
assert_equal("/path/to/headers/platform/ios /path/to/headers", result)
826+
end
827+
828+
def test_add_search_path_if_not_included_does_not_duplicate_quoted_path_with_spaces
829+
current_paths = '"/path/with spaces/headers" /another/path'
830+
new_path = '"/path/with spaces/headers"'
831+
result = ReactNativePodsUtils.add_search_path_if_not_included(current_paths, new_path)
832+
assert_equal(current_paths, result)
833+
end
834+
811835
def test_add_search_path_if_not_included_does_not_add_existing_path_with_leading_space_to_string
812836
current_paths = " /path/with/leading/space /another/path"
813837
new_path = "/path/with/leading/space"
@@ -943,6 +967,68 @@ def test_createHeaderSearchPathsForStableUmbrellas_whenUsingPrebuiltCore_addsNot
943967
assert_equal([], result)
944968
end
945969

970+
def test_setStableUmbrellaSearchPaths_addsOwningFrameworkPathToDependentTarget
971+
target = prepare_target("Consumer", nil, [DependencyMock.new("React-Fabric")])
972+
target_installation_result = TargetInstallationResultMock.new(target, target)
973+
974+
ReactNativePodsUtils.set_stable_umbrella_search_paths(target_installation_result)
975+
976+
target.build_configurations.each do |config|
977+
assert_equal(
978+
'$(inherited) "${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers"',
979+
config.build_settings["HEADER_SEARCH_PATHS"]
980+
)
981+
end
982+
end
983+
984+
def test_setStableUmbrellaSearchPaths_addsOwningFrameworkPathToTransitivelyDependentTarget
985+
dependency = Struct.new(:name, :target)
986+
fabric_target = prepare_target("React-Fabric")
987+
intermediate_target = prepare_target(
988+
"Intermediate",
989+
nil,
990+
[dependency.new("React-Fabric", fabric_target)]
991+
)
992+
consumer_target = prepare_target(
993+
"Consumer",
994+
nil,
995+
[dependency.new("Intermediate", intermediate_target)]
996+
)
997+
target_installation_result = TargetInstallationResultMock.new(consumer_target, consumer_target)
998+
999+
ReactNativePodsUtils.set_stable_umbrella_search_paths(target_installation_result)
1000+
1001+
consumer_target.build_configurations.each do |config|
1002+
assert_equal(
1003+
'$(inherited) "${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers"',
1004+
config.build_settings["HEADER_SEARCH_PATHS"]
1005+
)
1006+
end
1007+
end
1008+
1009+
def test_setStableUmbrellaSearchPaths_doesNotAddOwningFrameworkPathToUnrelatedTarget
1010+
target = prepare_target("React-Fabric")
1011+
target_installation_result = TargetInstallationResultMock.new(target, target)
1012+
1013+
ReactNativePodsUtils.set_stable_umbrella_search_paths(target_installation_result)
1014+
1015+
target.build_configurations.each do |config|
1016+
assert_nil(config.build_settings["HEADER_SEARCH_PATHS"])
1017+
end
1018+
end
1019+
1020+
def test_setStableUmbrellaSearchPaths_whenUsingPrebuiltCore_addsNothing
1021+
ReactNativeCoreUtils.class_variable_set(:@@build_from_source, false)
1022+
target = prepare_target("Consumer", nil, [DependencyMock.new("React-Fabric")])
1023+
target_installation_result = TargetInstallationResultMock.new(target, target)
1024+
1025+
ReactNativePodsUtils.set_stable_umbrella_search_paths(target_installation_result)
1026+
1027+
target.build_configurations.each do |config|
1028+
assert_nil(config.build_settings["HEADER_SEARCH_PATHS"])
1029+
end
1030+
end
1031+
9461032
# ================================= #
9471033
# TEST - Add RN_BUILDING definition #
9481034
# ================================= #

‎packages/react-native/scripts/cocoapods/utils.rb‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,15 +358,19 @@ def self.create_header_search_path_for_frameworks(base_folder, pod_name, framewo
358358
def self.create_header_search_paths_for_stable_umbrellas(base_folder)
359359
return [] unless ReactNativeCoreUtils.build_rncore_from_source()
360360

361+
self.stable_umbrella_frameworks.flat_map { |pod_name, framework_name|
362+
self.create_header_search_path_for_frameworks(base_folder, pod_name, framework_name, [])
363+
}
364+
end
365+
366+
def self.stable_umbrella_frameworks
361367
[
362368
["React-Fabric", "React_Fabric"],
363369
["React-debug", "React_debug"],
364370
["React-rendererdebug", "React_rendererdebug"],
365371
["React-timing", "React_timing"],
366372
["React-utils", "React_utils"],
367-
].flat_map { |pod_name, framework_name|
368-
self.create_header_search_path_for_frameworks(base_folder, pod_name, framework_name, [])
369-
}
373+
]
370374
end
371375

372376
# Add a new dependency to an existing spec, configuring also the headers search paths
@@ -399,6 +403,9 @@ def self.update_search_paths(installer)
399403
projects = self.extract_projects(installer)
400404

401405
projects.each do |project|
406+
stable_umbrella_search_paths = project == installer.pods_project ?
407+
[] : ReactNativePodsUtils.create_header_search_paths_for_stable_umbrellas("PODS_CONFIGURATION_BUILD_DIR")
408+
402409
project.build_configurations.each do |config|
403410

404411
header_search_paths = config.build_settings["HEADER_SEARCH_PATHS"] ||= "$(inherited)"
@@ -411,7 +418,7 @@ def self.update_search_paths(installer)
411418
"react/renderer/components/scrollview/platform/cxx",
412419
"react/renderer/components/scrollview/platform/ios",
413420
], false))
414-
.concat(ReactNativePodsUtils.create_header_search_paths_for_stable_umbrellas("PODS_CONFIGURATION_BUILD_DIR"))
421+
.concat(stable_umbrella_search_paths)
415422
.concat(ReactNativePodsUtils.create_header_search_path_for_frameworks("PODS_CONFIGURATION_BUILD_DIR", "React-FabricComponents", "React_FabricComponents", [
416423
"react/renderer/textlayoutmanager/platform/ios",
417424
"react/renderer/components/text/platform/cxx",
@@ -435,6 +442,8 @@ def self.update_search_paths(installer)
435442
end
436443

437444
installer.target_installation_results.pod_target_installation_results.each do |pod_name, target_installation_result|
445+
self.set_stable_umbrella_search_paths(target_installation_result)
446+
438447
if self.react_native_pods.include?(pod_name) || pod_name.include?("Pod") || pod_name.include?("Tests")
439448
next
440449
end
@@ -648,7 +657,9 @@ def self.add_search_path_if_not_included(current_search_paths, new_search_path)
648657

649658
if current_search_paths.is_a?(String)
650659
current_search_paths = current_search_paths.strip
651-
return "#{current_search_paths} #{new_search_path}" unless current_search_paths.include?(new_search_path)
660+
current_paths = Shellwords.shellsplit(current_search_paths)
661+
normalized_new_path = Shellwords.shellsplit(new_search_path).first
662+
return "#{current_search_paths} #{new_search_path}" unless current_paths.include?(normalized_new_path)
652663
end
653664

654665
if current_search_paths.is_a?(Array)
@@ -659,17 +670,55 @@ def self.add_search_path_if_not_included(current_search_paths, new_search_path)
659670
current_search_paths
660671
end
661672

662-
def self.update_header_paths_if_depends_on(target_installation_result, dependency_name, header_paths)
663-
depends_on_framework = target_installation_result.native_target.dependencies.any? { |d| d.name == dependency_name }
673+
def self.target_depends_on?(target, dependency_name, visited_targets = {})
674+
return false if target == nil
675+
676+
target_identifier = target.respond_to?(:uuid) ? target.uuid : target.object_id
677+
target_identifier ||= target.object_id
678+
return false if visited_targets[target_identifier]
679+
680+
visited_targets[target_identifier] = true
681+
target.dependencies.any? do |dependency|
682+
dependency.name == dependency_name ||
683+
(dependency.respond_to?(:target) && self.target_depends_on?(dependency.target, dependency_name, visited_targets))
684+
end
685+
end
686+
687+
def self.update_header_paths_if_depends_on(target_installation_result, dependency_name, header_paths, include_transitive_dependencies: false)
688+
native_target = target_installation_result.native_target
689+
depends_on_framework = if include_transitive_dependencies
690+
self.target_depends_on?(native_target, dependency_name)
691+
else
692+
native_target.dependencies.any? { |d| d.name == dependency_name }
693+
end
664694
if depends_on_framework
665-
target_installation_result.native_target.build_configurations.each do |config|
695+
native_target.build_configurations.each do |config|
666696
header_search_path = config.build_settings["HEADER_SEARCH_PATHS"] != nil ? config.build_settings["HEADER_SEARCH_PATHS"] : "$(inherited)"
667697
header_paths.each { |header| header_search_path = ReactNativePodsUtils.add_search_path_if_not_included(header_search_path, header) }
668698
config.build_settings["HEADER_SEARCH_PATHS"] = header_search_path
669699
end
670700
end
671701
end
672702

703+
def self.set_stable_umbrella_search_paths(target_installation_result)
704+
return unless ReactNativeCoreUtils.build_rncore_from_source()
705+
706+
ReactNativePodsUtils.stable_umbrella_frameworks.each do |pod_name, framework_name|
707+
header_search_paths = ReactNativePodsUtils.create_header_search_path_for_frameworks(
708+
"PODS_CONFIGURATION_BUILD_DIR",
709+
pod_name,
710+
framework_name,
711+
[]
712+
).map { |search_path| "\"#{search_path}\"" }
713+
ReactNativePodsUtils.update_header_paths_if_depends_on(
714+
target_installation_result,
715+
pod_name,
716+
header_search_paths,
717+
include_transitive_dependencies: true
718+
)
719+
end
720+
end
721+
673722
def self.set_rctfolly_search_paths(target_installation_result)
674723
ReactNativePodsUtils.update_header_paths_if_depends_on(target_installation_result, "RCT-Folly", [
675724
"\"$(PODS_ROOT)/RCT-Folly\"",

0 commit comments

Comments
 (0)