Skip to content

Commit 71c6bfb

Browse files
Kudometa-codesync[bot]
authored andcommitted
- Keep the quotes on header search paths (#57981)
Summary: a regression from a8156ac and breaks react-native nightly build at expo: https://github.com/expo/expo/actions/runs/31990917142/job/95274214849 the `shellsplit` will stripe quotes and break paths with spaces like `Swift Compatibility Header`. this pr tries to re-quote. here's small demo script ```ruby require 'shellwords' # What main does today: shellsplit the existing value, append, write it back. def main_behaviour(existing, add) paths = existing || [] paths = Shellwords.shellsplit(paths) if paths.is_a?(String) (paths + add).uniq end # What this PR does: quote only the paths we add, never touch what is there. def this_pr(existing, add) quoted = add.map { |path| "\"#{path}\"" } case existing when nil quoted when Array existing + quoted.reject { |path| existing.include?(path) } else ([existing] + quoted.reject { |path| existing.include?(path) }).join(" ") end end # Xcode joins an Array setting with spaces, then splits it on whitespace while # honouring quotes. This is what the compiler ends up with. def as_xcode_reads_it(value) Shellwords.shellsplit(value.is_a?(Array) ? value.join(" ") : value) end ADD = ['$(PODS_ROOT)/ReactNativeDependencies/Headers'] # A pod that exports a Swift compatibility header. The directory name has # spaces, so the podspec quotes it. Podspecs write this as a String or as an # Array, and both forms reach add_rn_third_party_dependencies. SWIFT_HEADER = '${PODS_CONFIGURATION_BUILD_DIR}/MyPod/Swift Compatibility Header' CASES = { 'String' => "\"$(PODS_ROOT)/DoubleConversion\" \"#{SWIFT_HEADER}\"", 'Array' => ['"$(PODS_ROOT)/DoubleConversion"', "\"#{SWIFT_HEADER}\""], } CASES.each do |label, existing| puts "#{label} HEADER_SEARCH_PATHS" { 'main' => method(:main_behaviour), 'this PR' => method(:this_pr) }.each do |name, fn| paths = as_xcode_reads_it(fn.call(existing, ADD)) verdict = paths.include?(SWIFT_HEADER) ? 'ok' : 'BROKEN' puts " #{name.ljust(7)} -> #{paths.size} paths, #{verdict}: #{paths.inspect}" end puts end ``` output ``` String HEADER_SEARCH_PATHS main -> 5 paths, BROKEN: ["$(PODS_ROOT)/DoubleConversion", "${PODS_CONFIGURATION_BUILD_DIR}/MyPod/Swift", "Compatibility", "Header", "$(PODS_ROOT)/ReactNativeDependencies/Headers"] this PR -> 3 paths, ok: ["$(PODS_ROOT)/DoubleConversion", "${PODS_CONFIGURATION_BUILD_DIR}/MyPod/Swift Compatibility Header", "$(PODS_ROOT)/ReactNativeDependencies/Headers"] Array HEADER_SEARCH_PATHS main -> 3 paths, ok: ["$(PODS_ROOT)/DoubleConversion", "${PODS_CONFIGURATION_BUILD_DIR}/MyPod/Swift Compatibility Header", "$(PODS_ROOT)/ReactNativeDependencies/Headers"] this PR -> 3 paths, ok: ["$(PODS_ROOT)/DoubleConversion", "${PODS_CONFIGURATION_BUILD_DIR}/MyPod/Swift Compatibility Header", "$(PODS_ROOT)/ReactNativeDependencies/Headers"] ``` ## Changelog: [IOS][FIXED] - Keep the quotes on header search paths containing spaces in `add_rn_third_party_dependencies` Pull Request resolved: #57981 Test Plan: apply the patch on create-expo-nightly and the ios build should pass expo/expo#49023 Reviewed By: javache Differential Revision: D116600347 Pulled By: cipolleschi fbshipit-source-id: 73c3c7560e5c4970b678298c05743d1e2404cc9b
1 parent b03652a commit 71c6bfb

2 files changed

Lines changed: 120 additions & 25 deletions

File tree

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
#
3+
# This source code is licensed under the MIT license found in the
4+
# LICENSE file in the root directory of this source tree.
5+
6+
require "test/unit"
7+
require "shellwords"
8+
require_relative "../rndependencies.rb"
9+
require_relative "./test_utils/SpecMock.rb"
10+
11+
class RNDependenciesTests < Test::Unit::TestCase
12+
13+
# A pod that exports a Swift compatibility header ships this path, and the
14+
# directory name contains spaces.
15+
SWIFT_HEADER = "${PODS_CONFIGURATION_BUILD_DIR}/MyPod/Swift Compatibility Header"
16+
17+
def teardown
18+
ReactNativeDependenciesUtils.class_variable_set(:@@build_from_source, true)
19+
end
20+
21+
# Xcode joins an array setting with spaces, then splits it back on
22+
# whitespace while honouring quotes. This is what the compiler ends up with.
23+
def resolved_paths(xcconfig)
24+
value = xcconfig["HEADER_SEARCH_PATHS"]
25+
Shellwords.shellsplit(value.is_a?(Array) ? value.join(" ") : value)
26+
end
27+
28+
# ================================== #
29+
# TEST - append_header_search_paths #
30+
# ================================== #
31+
32+
def test_appendHeaderSearchPaths_whenUnset_quotesTheAddedPaths
33+
xcconfig = {}
34+
35+
ReactNativeDependenciesUtils.append_header_search_paths(xcconfig, ["$(PODS_ROOT)/glog"])
36+
37+
assert_equal(["\"$(PODS_ROOT)/glog\""], xcconfig["HEADER_SEARCH_PATHS"])
38+
end
39+
40+
def test_appendHeaderSearchPaths_whenStringHasQuotedPathWithSpaces_keepsItIntact
41+
xcconfig = {"HEADER_SEARCH_PATHS" => "\"$(PODS_ROOT)/DoubleConversion\" \"#{SWIFT_HEADER}\""}
42+
43+
ReactNativeDependenciesUtils.append_header_search_paths(xcconfig, ["$(PODS_ROOT)/glog"])
44+
45+
assert_equal(["$(PODS_ROOT)/DoubleConversion", SWIFT_HEADER, "$(PODS_ROOT)/glog"], resolved_paths(xcconfig))
46+
end
47+
48+
def test_appendHeaderSearchPaths_whenArrayHasQuotedPathWithSpaces_keepsItIntact
49+
xcconfig = {"HEADER_SEARCH_PATHS" => ["\"$(PODS_ROOT)/DoubleConversion\"", "\"#{SWIFT_HEADER}\""]}
50+
51+
ReactNativeDependenciesUtils.append_header_search_paths(xcconfig, ["$(PODS_ROOT)/glog"])
52+
53+
assert_equal(["$(PODS_ROOT)/DoubleConversion", SWIFT_HEADER, "$(PODS_ROOT)/glog"], resolved_paths(xcconfig))
54+
end
55+
56+
def test_appendHeaderSearchPaths_whenCalledTwice_doesNotDuplicateEntries
57+
xcconfig = {"HEADER_SEARCH_PATHS" => "\"#{SWIFT_HEADER}\""}
58+
59+
ReactNativeDependenciesUtils.append_header_search_paths(xcconfig, ["$(PODS_ROOT)/glog"])
60+
ReactNativeDependenciesUtils.append_header_search_paths(xcconfig, ["$(PODS_ROOT)/glog"])
61+
62+
assert_equal([SWIFT_HEADER, "$(PODS_ROOT)/glog"], resolved_paths(xcconfig))
63+
end
64+
65+
# ======================================= #
66+
# TEST - add_rn_third_party_dependencies #
67+
# ======================================= #
68+
69+
def test_addRNThirdPartyDependencies_whenBuildingFromSource_keepsQuotedPathWithSpaces
70+
spec = SpecMock.new
71+
spec.pod_target_xcconfig = {"HEADER_SEARCH_PATHS" => "\"#{SWIFT_HEADER}\""}
72+
73+
add_rn_third_party_dependencies(spec)
74+
75+
paths = resolved_paths(spec.pod_target_xcconfig)
76+
assert_equal(SWIFT_HEADER, paths.first)
77+
assert(paths.include?("$(PODS_ROOT)/RCT-Folly"))
78+
end
79+
80+
def test_addRNThirdPartyDependencies_whenUsingPrebuiltDeps_keepsQuotedPathWithSpaces
81+
ReactNativeDependenciesUtils.class_variable_set(:@@build_from_source, false)
82+
spec = SpecMock.new
83+
spec.pod_target_xcconfig = {"HEADER_SEARCH_PATHS" => "\"#{SWIFT_HEADER}\""}
84+
85+
add_rn_third_party_dependencies(spec)
86+
87+
assert_equal([SWIFT_HEADER, "$(PODS_ROOT)/ReactNativeDependencies/Headers"], resolved_paths(spec.pod_target_xcconfig))
88+
end
89+
end

packages/react-native/scripts/cocoapods/rndependencies.rb

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
require "json"
77
require 'net/http'
88
require 'rexml/document'
9-
require 'shellwords'
109

1110
require_relative './utils.rb'
1211

@@ -34,38 +33,26 @@ def add_rn_third_party_dependencies(s)
3433
s.dependency "RCT-Folly/Fabric"
3534
end
3635

37-
header_search_paths = current_pod_target_xcconfig["HEADER_SEARCH_PATHS"] || []
38-
39-
if header_search_paths.is_a?(String)
40-
header_search_paths = Shellwords.shellsplit(header_search_paths)
41-
end
42-
43-
header_search_paths << "$(PODS_ROOT)/glog"
44-
header_search_paths << "$(PODS_ROOT)/boost"
45-
header_search_paths << "$(PODS_ROOT)/DoubleConversion"
46-
header_search_paths << "$(PODS_ROOT)/fast_float/include"
47-
header_search_paths << "$(PODS_ROOT)/fmt/include"
48-
header_search_paths << "$(PODS_ROOT)/SocketRocket"
49-
header_search_paths << "$(PODS_ROOT)/RCT-Folly"
50-
51-
# uniq so a second call on the same spec can't duplicate entries.
52-
current_pod_target_xcconfig["HEADER_SEARCH_PATHS"] = header_search_paths.uniq
36+
ReactNativeDependenciesUtils.append_header_search_paths(current_pod_target_xcconfig, [
37+
"$(PODS_ROOT)/glog",
38+
"$(PODS_ROOT)/boost",
39+
"$(PODS_ROOT)/DoubleConversion",
40+
"$(PODS_ROOT)/fast_float/include",
41+
"$(PODS_ROOT)/fmt/include",
42+
"$(PODS_ROOT)/SocketRocket",
43+
"$(PODS_ROOT)/RCT-Folly",
44+
])
5345
else
5446
# Prebuilt-deps mode: this pod SELF-SERVES the third-party headers from its
5547
# own xcframework (incl. SocketRocket - sole supplier in this mode). See
5648
# scripts/cocoapods/__docs__/prebuilt-deps.md for the full contract.
5749
s.dependency "ReactNativeDependencies"
5850

59-
header_search_paths = current_pod_target_xcconfig["HEADER_SEARCH_PATHS"] || []
60-
if header_search_paths.is_a?(String)
61-
header_search_paths = Shellwords.shellsplit(header_search_paths)
62-
end
6351
# Artifact headers are flattened into the pod-local Headers/ by the podspec
6452
# prepare_command (see __docs__/prebuilt-deps.md).
65-
header_search_paths << "$(PODS_ROOT)/ReactNativeDependencies/Headers"
66-
67-
# uniq so a second call on the same spec can't duplicate entries.
68-
current_pod_target_xcconfig["HEADER_SEARCH_PATHS"] = header_search_paths.uniq
53+
ReactNativeDependenciesUtils.append_header_search_paths(current_pod_target_xcconfig, [
54+
"$(PODS_ROOT)/ReactNativeDependencies/Headers",
55+
])
6956
end
7057

7158
s.pod_target_xcconfig = current_pod_target_xcconfig
@@ -148,6 +135,25 @@ def self.setup_react_native_dependencies(react_native_path, react_native_version
148135
end
149136
end
150137

138+
# Xcode splits HEADER_SEARCH_PATHS on whitespace, so every path we add is
139+
# quoted - PODS_ROOT can expand to a directory with spaces in its name.
140+
# Paths already in the xcconfig are left untouched: they carry the podspec
141+
# author's own quoting, and re-quoting them would break it.
142+
def self.append_header_search_paths(xcconfig, paths)
143+
quoted = paths.map { |path| "\"#{path}\"" }
144+
existing = xcconfig["HEADER_SEARCH_PATHS"]
145+
146+
# reject so a second call on the same spec can't duplicate entries.
147+
case existing
148+
when nil
149+
xcconfig["HEADER_SEARCH_PATHS"] = quoted
150+
when Array
151+
xcconfig["HEADER_SEARCH_PATHS"] = existing + quoted.reject { |path| existing.include?(path) }
152+
else
153+
xcconfig["HEADER_SEARCH_PATHS"] = ([existing] + quoted.reject { |path| existing.include?(path) }).join(" ")
154+
end
155+
end
156+
151157
def self.abort_if_use_local_rndeps_with_no_file()
152158
if !File.exist?(ENV["RCT_USE_LOCAL_RN_DEP"])
153159
abort("RCT_USE_LOCAL_RN_DEP is set to #{ENV["RCT_USE_LOCAL_RN_DEP"]} but the file does not exist!")

0 commit comments

Comments
 (0)