Skip to content

Commit e752c63

Browse files
authored
[Infra] Add workflow for detecting unlinked Firestore symbols (#10901)
* [Infra] Add workflow for detecting unlinked Firestore symbols * Exit if xcodebuild fails * Add more documentation for script * Enable script to work on current state of master * Style * Feedback pt. 1 * Feedback pt. 2 * Pipe to xcpretty to shorten logs * Debug CI error code * Attempt to fix CI error code * Fix CI issue * Fix pipe issue
1 parent 1f3befa commit e752c63

File tree

3 files changed

+227
-0
lines changed

3 files changed

+227
-0
lines changed

.github/workflows/zip.yml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,37 @@ jobs:
438438
name: quickstart_artifacts_firestore
439439
path: quickstart-ios/
440440

441+
check_framework_firestore_symbols:
442+
# Don't run on private repo.
443+
if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch'
444+
needs: package-head
445+
env:
446+
FRAMEWORK_DIR: "Firebase-actions-dir"
447+
runs-on: macos-12
448+
steps:
449+
- name: Xcode 13.3.1
450+
run: sudo xcode-select -s /Applications/Xcode_13.3.1.app/Contents/Developer
451+
- uses: actions/checkout@v3
452+
- name: Get framework dir
453+
uses: actions/download-artifact@v1
454+
with:
455+
name: Firebase-actions-dir
456+
- uses: ruby/setup-ruby@v1
457+
- name: Setup Bundler
458+
run: ./scripts/setup_bundler.sh
459+
- name: Install xcpretty
460+
run: gem install xcpretty
461+
- name: Move frameworks
462+
run: |
463+
mkdir -p "${HOME}"/ios_frameworks/
464+
find "${GITHUB_WORKSPACE}/${FRAMEWORK_DIR}" -name "Firebase*latest.zip" -exec unzip -d "${HOME}"/ios_frameworks/ {} +
465+
- uses: actions/checkout@v3
466+
- name: Check linked Firestore.xcframework for unlinked symbols.
467+
run: |
468+
scripts/check_firestore_symbols.sh \
469+
$(pwd) \
470+
"${HOME}"/ios_frameworks/Firebase/FirebaseFirestore/FirebaseFirestore.xcframework
471+
441472
quickstart_framework_inappmessaging:
442473
# Don't run on private repo.
443474
if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch'

Package.swift

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,3 +1355,36 @@ if ProcessInfo.processInfo.environment["FIREBASECI_USE_LATEST_GOOGLEAPPMEASUREME
13551355
)
13561356
}
13571357
}
1358+
1359+
// This is set when running `scripts/check_firestore_symbols.sh`.
1360+
if ProcessInfo.processInfo.environment["FIREBASECI_USE_LOCAL_FIRESTORE_ZIP"] != nil {
1361+
if let firestoreIndex = package.targets
1362+
.firstIndex(where: { $0.name == "FirebaseFirestore" }) {
1363+
package.targets[firestoreIndex] = .binaryTarget(
1364+
name: "FirebaseFirestore",
1365+
// The `xcframework` should be moved to the root of the repo.
1366+
path: "FirebaseFirestore.xcframework"
1367+
)
1368+
}
1369+
1370+
// TODO(ncooke3): Below re-defining is not needed when original
1371+
// FirebaseFirestoreTarget definition matches below definition.
1372+
if let firestoreTargetIndex = package.targets
1373+
.firstIndex(where: { $0.name == "FirebaseFirestoreTarget" }) {
1374+
package.targets[firestoreTargetIndex] = .target(
1375+
name: "FirebaseFirestoreTarget",
1376+
dependencies: [
1377+
.target(
1378+
name: "FirebaseFirestore",
1379+
condition: .when(platforms: [.iOS, .tvOS, .macOS])
1380+
),
1381+
.product(name: "abseil", package: "abseil"),
1382+
.product(name: "gRPC-cpp", package: "gRPC"),
1383+
.product(name: "nanopb", package: "nanopb"),
1384+
"FirebaseCore",
1385+
"leveldb",
1386+
],
1387+
path: "SwiftPM-PlatformExclude/FirebaseFirestoreWrap"
1388+
)
1389+
}
1390+
}

scripts/check_firestore_symbols.sh

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
#!/bin/bash
2+
3+
# Copyright 2023 Google LLC
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
# DESCRIPTION: This script identifies Objective-C symbols within the
18+
# `FirebaseFirestore.xcframework` that are not automatically linked when used
19+
# in a client target. Because the `FirebaseFirestore.xcframework` should
20+
# function without clients needing to pass the `-ObjC` flag, this script
21+
# catches potential regressions that break that requirement.
22+
#
23+
# DEPENDENCIES: This script depends on the given Firebase repo's `Package.swift`
24+
# using the `FIREBASECI_USE_LOCAL_FIRESTORE_ZIP` env var to swap the Firestore
25+
# target definition out to instead reference a *local* binary using the
26+
# `.binaryTarget(path:)` API.
27+
#
28+
# DESIGN: This script creates an executable package that depends on Firestore
29+
# via a local binary SPM target. The package is built twice, once with the
30+
# -ObjC flag and once without. The linked Objective-C symbols are then
31+
# stripped from each build's resulting executable. The symbols are then diffed
32+
# to determine if there exists symbols that were only linked due to the -ObjC
33+
# flag.
34+
#
35+
# USAGE: ./check_firestore_symbols.sh <PATH_TO_FIREBASE_REPO> <PATH_TO_FIRESTORE_XCFRAMEWORK>
36+
37+
if [[ $# -ne 2 ]]; then
38+
echo "Usage: ./check_firestore_symbols.sh <PATH_TO_FIREBASE_REPO> <PATH_TO_FIRESTORE_XCFRAMEWORK>"
39+
exit 1
40+
fi
41+
42+
# Check if the given repo path is valid.
43+
FIREBASE_REPO_PATH=$1
44+
45+
if [[ "$FIREBASE_REPO_PATH" != /* ]]; then
46+
echo "The given path should be an absolute path."
47+
exit 1
48+
fi
49+
50+
if [[ ! -d "$FIREBASE_REPO_PATH" ]]; then
51+
echo "The given repo does not exist: $FIREBASE_REPO_PATH"
52+
exit 1
53+
fi
54+
55+
# Check if the given xcframework path is valid.
56+
FIRESTORE_XCFRAMEWORK_PATH=$2
57+
58+
if [ "$(basename $FIRESTORE_XCFRAMEWORK_PATH)" != 'FirebaseFirestore.xcframework' ]; then
59+
echo "The given xcframework is not a FirebaseFirestore.xcframework."
60+
exit 1
61+
fi
62+
63+
if [[ ! -d "$FIRESTORE_XCFRAMEWORK_PATH" ]]; then
64+
echo "The given xcframework does not exist: $FIRESTORE_XCFRAMEWORK_PATH"
65+
exit 1
66+
fi
67+
68+
# Copy the given Firestore framework to the root of the given Firebase repo.
69+
# This script uses an env var that will alter the repo's `Package.swift` to
70+
# pick up the copied Firestore framework. See
71+
# `FIREBASECI_USE_LOCAL_FIRESTORE_ZIP` in Firebase's `Package.swift` for more.
72+
cp -r "$FIRESTORE_XCFRAMEWORK_PATH" "$FIREBASE_REPO_PATH"
73+
74+
# Create a temporary directory for the test package. The test package defines an
75+
# executable and has the following directory structure:
76+
#
77+
# TestPkg
78+
# ├── Package.swift
79+
# └── Sources
80+
#    └── TestPkg
81+
#     └── main.swift
82+
TEST_PKG_ROOT=$(mktemp -d -t TestPkg)
83+
echo "Test package root: $TEST_PKG_ROOT"
84+
85+
# Create the package's subdirectories.
86+
mkdir -p "$TEST_PKG_ROOT/Sources/TestPkg"
87+
88+
# Generate the package's `Package.swift`.
89+
# TODO(ncooke3): Make package path an argument.
90+
cat > "$TEST_PKG_ROOT/Package.swift" <<- EOM
91+
// swift-tools-version: 5.6
92+
import PackageDescription
93+
94+
let package = Package(
95+
name: "TestPkg",
96+
platforms: [.macOS(.v10_13)],
97+
dependencies: [
98+
.package(path: "${FIREBASE_REPO_PATH}")
99+
],
100+
targets: [
101+
.executableTarget(
102+
name: "TestPkg",
103+
dependencies: [
104+
.product(
105+
name: "FirebaseFirestore",
106+
package: "firebase-ios-sdk"
107+
)
108+
]
109+
)
110+
]
111+
)
112+
EOM
113+
114+
# Generate the package's `main.swift`.
115+
cat > "$TEST_PKG_ROOT/Sources/TestPkg/main.swift" <<- EOM
116+
import FirebaseFirestore
117+
118+
let db = Firestore.firestore()
119+
EOM
120+
121+
# Change to the test package's root directory in order to build the package.
122+
cd "$TEST_PKG_ROOT"
123+
124+
# Build the test package *without* the `-ObjC` linker flag, and dump the
125+
# resulting executable file's Objective-C symbols into a text file.
126+
echo "Building test package without -ObjC linker flag..."
127+
# Invoke a subshell to avoid pipefail affecting the rest of the script.
128+
(
129+
set -eo pipefail && FIREBASECI_USE_LOCAL_FIRESTORE_ZIP=1 \
130+
xcodebuild -scheme 'TestPkg' -destination 'generic/platform=macOS' \
131+
-derivedDataPath "$HOME/Library/Developer/Xcode/DerivedData/TestPkg" \
132+
| xcpretty
133+
)
134+
135+
136+
nm ~/Library/Developer/Xcode/DerivedData/TestPkg/Build/Products/Debug/TestPkg \
137+
| grep -o "[-+]\[.*\]" > objc_symbols_without_linker_flag.txt
138+
139+
# Build the test package *with* the -ObjC linker flag, and dump the
140+
# resulting executable file's Objective-C symbols into a text file.
141+
echo "Building test package with -ObjC linker flag..."
142+
# Invoke a subshell to avoid pipefail affecting the rest of the script.
143+
(
144+
set -eo pipefail && FIREBASECI_USE_LOCAL_FIRESTORE_ZIP=1 \
145+
xcodebuild -scheme 'TestPkg' -destination 'generic/platform=macOS' \
146+
-derivedDataPath "$HOME/Library/Developer/Xcode/DerivedData/TestPkg-ObjC" \
147+
OTHER_LDFLAGS='-ObjC' \
148+
| xcpretty
149+
)
150+
151+
nm ~/Library/Developer/Xcode/DerivedData/TestPkg-ObjC/Build/Products/Debug/TestPkg \
152+
| grep -o "[-+]\[.*\]" > objc_symbols_with_linker_flag.txt
153+
154+
# Compare the two text files to see if the -ObjC linker flag has any effect.
155+
DIFF=$(diff objc_symbols_without_linker_flag.txt objc_symbols_with_linker_flag.txt)
156+
if [[ -n "$DIFF" ]]; then
157+
echo "Failure: Unlinked Objective-C symbols have been detected:"
158+
echo "$DIFF"
159+
exit 1
160+
else
161+
echo "Success: No unlinked Objective-C symbols have been detected."
162+
exit 0
163+
fi

0 commit comments

Comments
 (0)