Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the iOS and tvOS dependency management from CocoaPods to Swift Package Manager (SPM). It removes the old ios_pod configuration, Podfile, and CocoaPods setup scripts, replacing them with an ios_spm directory containing Package.swift and a new CMake integration (setup_spm_headers and symlink_spm_headers). All Firebase modules, helper scripts, and documentation have been updated to support this transition. Feedback on the changes highlights a critical NameError in scripts/update_android_ios_dependencies.py due to an un-renamed variable, a potential failure in file(CREATE_LINK) in ios_spm/CMakeLists.txt if symlinks already exist, and a redundant include path in firestore/CMakeLists.txt that should be removed.
| modify_project_file_spm(project_file, latest_pod_versions_map, args.dryrun, | ||
| args.ignore_ios_versions) |
There was a problem hiding this comment.
The variable latest_pod_versions_map is not defined in this scope because it was renamed to latest_ios_versions_map on line 771. This will cause a NameError at runtime when running the dependency update script. Please update this reference to use latest_ios_versions_map.
| modify_project_file_spm(project_file, latest_pod_versions_map, args.dryrun, | |
| args.ignore_ios_versions) | |
| modify_project_file_spm(project_file, latest_ios_versions_map, args.dryrun, | |
| args.ignore_ios_versions) |
| if(fw_headers MATCHES "${preferred_slice}" OR NOT EXISTS "${symlink_base_dir}/${fw_name}") | ||
| file(MAKE_DIRECTORY "${symlink_base_dir}") | ||
| file(CREATE_LINK "${fw_headers}" "${symlink_base_dir}/${fw_name}" SYMBOLIC COPY_ON_ERROR) |
There was a problem hiding this comment.
If a symlink already exists at ${symlink_base_dir}/${fw_name} (for example, from a previous CMake run or a different architecture slice), file(CREATE_LINK) will fail because it does not automatically overwrite existing links. To make this robust, we should remove the existing symlink before creating a new one.
if(fw_headers MATCHES "${preferred_slice}" OR NOT EXISTS "${symlink_base_dir}/${fw_name}")
file(MAKE_DIRECTORY "${symlink_base_dir}")
if(EXISTS "${symlink_base_dir}/${fw_name}")
file(REMOVE "${symlink_base_dir}/${fw_name}")
endif()
file(CREATE_LINK "${fw_headers}" "${symlink_base_dir}/${fw_name}" SYMBOLIC COPY_ON_ERROR)
| ${FIREBASE_SPM_CHECKOUTS_DIR}/firebase-ios-sdk | ||
| ${FIREBASE_SPM_CHECKOUTS_DIR}/firebase-ios-sdk/Firestore | ||
| ${FIREBASE_SPM_CHECKOUTS_DIR}/firebase-ios-sdk/FirebaseFirestoreInternal | ||
| ) |
There was a problem hiding this comment.
FirebaseFirestoreInternal is a target name in the Swift Package, but it is not an actual directory in the firebase-ios-sdk repository checkout (the Firestore source is entirely under Firestore/). Therefore, this include path is non-existent and redundant. It should be removed.
${FIREBASE_SPM_CHECKOUTS_DIR}/firebase-ios-sdk
${FIREBASE_SPM_CHECKOUTS_DIR}/firebase-ios-sdk/Firestore
)
Description
Since Cocoapods is being turned down, migrate the iOS builds to Swift Packages instead. This includes both the SDK build logic, converting ios_pod to ios_spm and getting the necessary dependencies, and the scripts we have to update the version numbers.
This should not affect end users of the built SDKs, only the build process.
Testing
Running scripts locally.
Type of Change
Place an
xthe applicable box:Notes
Release Notessection ofrelease_build_files/readme.md.