-
Notifications
You must be signed in to change notification settings - Fork 243
Emt 1964 Automate code signing and dSym binaries generation #1497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Emt 1964 Automate code signing and dSym binaries generation #1497
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds automated code signing and debug symbol generation for the BranchSDK XCFramework. The implementation looks solid, but I have a few suggestions to improve error handling and maintainability.
Skipped files
.github/workflows/release.yml
: Skipped file pattern
XCFRAMEWORK_PATH="./build/BranchSDK.xcframework" | ||
XCFRAMEWORK_PATH_SIGNED="./build/signedFramework/" | ||
XCFRAMEWORK_PATH_dSYM="./build/dSymFramework/BranchSDK.xcframework" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Code Refactor
Issue: The certificate name is hardcoded, which makes it less maintainable and harder to change in the future.
Fix: Extract the certificate name to a variable at the top of the script.
Impact: Improves maintainability and makes it easier to update the certificate name in the future.
XCFRAMEWORK_PATH="./build/BranchSDK.xcframework" | |
XCFRAMEWORK_PATH_SIGNED="./build/signedFramework/" | |
XCFRAMEWORK_PATH_dSYM="./build/dSymFramework/BranchSDK.xcframework" | |
XCFRAMEWORK_PATH=\"./build/BranchSDK.xcframework\" | |
XCFRAMEWORK_PATH_SIGNED=\"./build/signedFramework/\" | |
XCFRAMEWORK_PATH_dSYM=\"./build/dSymFramework/BranchSDK.xcframework\" | |
CODESIGN_IDENTITY=\"Apple Distribution: Branch Metrics, Inc. (R63EM248DP)\" | |
# delete previous build |
# create signed binary | ||
mkdir -p "${XCFRAMEWORK_PATH_SIGNED}" | ||
cp -rf "${XCFRAMEWORK_PATH}" "${XCFRAMEWORK_PATH_SIGNED}" | ||
codesign --deep --timestamp -s "Apple Distribution: Branch Metrics, Inc. (R63EM248DP)" "${XCFRAMEWORK_PATH_SIGNED}/BranchSDK.xcframework" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Code Refactor
Issue: The codesign command doesn't have error handling, which could lead to silent failures.
Fix: Add error handling to check if the codesign command succeeded.
Impact: Prevents silent failures and makes debugging easier.
# create signed binary | |
mkdir -p "${XCFRAMEWORK_PATH_SIGNED}" | |
cp -rf "${XCFRAMEWORK_PATH}" "${XCFRAMEWORK_PATH_SIGNED}" | |
codesign --deep --timestamp -s "Apple Distribution: Branch Metrics, Inc. (R63EM248DP)" "${XCFRAMEWORK_PATH_SIGNED}/BranchSDK.xcframework" | |
# create signed binary | |
mkdir -p \"${XCFRAMEWORK_PATH_SIGNED}\" | |
cp -rf \"${XCFRAMEWORK_PATH}\" \"${XCFRAMEWORK_PATH_SIGNED}\" | |
if ! codesign --deep --timestamp -s \"${CODESIGN_IDENTITY}\" \"${XCFRAMEWORK_PATH_SIGNED}/BranchSDK.xcframework\"; then | |
echo \"Error: Code signing failed\" | |
exit 1 | |
fi |
scripts/build_xcframework_noidfa.sh
Outdated
# create signed binary | ||
mkdir -p "${XCFRAMEWORK_PATH_SIGNED}" | ||
cp -rf "${XCFRAMEWORK_PATH}" "${XCFRAMEWORK_PATH_SIGNED}" | ||
codesign --timestamp -s "Apple Distribution: Branch Metrics, Inc. (R63EM248DP)" "${XCFRAMEWORK_PATH_SIGNED}/BranchSDK.xcframework" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚡️ Performance Improvement
Issue: The codesign command for the no-IDFA version doesn't use the --deep flag, unlike the regular version.
Fix: Add the --deep flag for consistency and to ensure all nested components are signed.
Impact: Ensures thorough code signing of all components in the framework.
# create signed binary | |
mkdir -p "${XCFRAMEWORK_PATH_SIGNED}" | |
cp -rf "${XCFRAMEWORK_PATH}" "${XCFRAMEWORK_PATH_SIGNED}" | |
codesign --timestamp -s "Apple Distribution: Branch Metrics, Inc. (R63EM248DP)" "${XCFRAMEWORK_PATH_SIGNED}/BranchSDK.xcframework" | |
# create signed binary | |
mkdir -p \"${XCFRAMEWORK_PATH_SIGNED}\" | |
cp -rf \"${XCFRAMEWORK_PATH}\" \"${XCFRAMEWORK_PATH_SIGNED}\" | |
codesign --deep --timestamp -s \"Apple Distribution: Branch Metrics, Inc. (R63EM248DP)\" \"${XCFRAMEWORK_PATH_SIGNED}/BranchSDK.xcframework\"``` |
|
||
echo "Packaging signed BranchSDK.xcframework" | ||
zip -rqy $zip_file_signed ./signedFramework/BranchSDK.xcframework/ | ||
shasum $zip_file_signed >> $checksum_file_signed | ||
mv $zip_file_signed .. | ||
mv $checksum_file_signed .. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Code Refactor
Issue: The script doesn't check if the signed framework directory exists before attempting to package it.
Fix: Add a check to verify the directory exists before packaging.
Impact: Prevents errors when the signed framework doesn't exist and provides clearer error messages.
echo "Packaging signed BranchSDK.xcframework" | |
zip -rqy $zip_file_signed ./signedFramework/BranchSDK.xcframework/ | |
shasum $zip_file_signed >> $checksum_file_signed | |
mv $zip_file_signed .. | |
mv $checksum_file_signed .. | |
echo \"Packaging signed BranchSDK.xcframework\" | |
if [ ! -d \"./signedFramework/BranchSDK.xcframework/\" ]; then | |
echo \"Error: Signed framework directory not found\" | |
exit 1 | |
fi | |
zip -rqy $zip_file_signed ./signedFramework/BranchSDK.xcframework/ | |
shasum $zip_file_signed >> $checksum_file_signed | |
mv $zip_file_signed .. | |
mv $checksum_file_signed .. |
|
||
echo "Packaging debug BranchSDK.xcframework with dSyms" | ||
zip -rqy $zip_file_WithdSym ./dSymFramework/BranchSDK.xcframework/ | ||
shasum $zip_file_WithdSym >> $checksum_file_WithdSym | ||
mv $zip_file_WithdSym .. | ||
mv $checksum_file_WithdSym .. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Code Refactor
Issue: Similar to the signed framework, there's no check if the dSym framework directory exists before packaging.
Fix: Add a check to verify the directory exists before packaging.
Impact: Prevents errors when the dSym framework doesn't exist and provides clearer error messages.
echo "Packaging debug BranchSDK.xcframework with dSyms" | |
zip -rqy $zip_file_WithdSym ./dSymFramework/BranchSDK.xcframework/ | |
shasum $zip_file_WithdSym >> $checksum_file_WithdSym | |
mv $zip_file_WithdSym .. | |
mv $checksum_file_WithdSym .. | |
echo \"Packaging debug BranchSDK.xcframework with dSyms\" | |
if [ ! -d \"./dSymFramework/BranchSDK.xcframework/\" ]; then | |
echo \"Error: dSym framework directory not found\" | |
exit 1 | |
fi | |
zip -rqy $zip_file_WithdSym ./dSymFramework/BranchSDK.xcframework/ | |
shasum $zip_file_WithdSym >> $checksum_file_WithdSym | |
mv $zip_file_WithdSym .. | |
mv $checksum_file_WithdSym .. |
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
Reference
EMT-1964 - Automate Code Signing and Debug Symbol Generation for iOS SDK Released Binaries
https://branch.atlassian.net/browse/EMT-1964
Summary
This PR has updated build scripts and updated GHA for release process to generate signed binaries and binaries with dSym
Type Of Change
Testing Instructions
Not yet tested as this will create a new release in Github page and this action will run only after its merged into master.
cc @BranchMetrics/saas-sdk-devs for visibility.