Adyen / adyen-ios

Adyen iOS Drop-in and Components
https://docs.adyen.com/checkout/ios
MIT License
150 stars 119 forks source link

Support library evolution #1354

Open ffittschen opened 1 year ago

ffittschen commented 1 year ago

Describe the bug As we are using Adyen alongside an internal binary framework, we need to compile it with library evolution enabled (BUILD_LIBRARY_FOR_DISTRIBUTION=YES), so that binary compatibility between our binary framework and its Adyen dependencies is guaranteed.

Starting with this PR, Adyen uses the underscored attribute @_spi to mark declarations as System Programming Interface (SPI). These @_spi annotations cause build errors when compiling Adyen with the BUILD_LIBRARY_FOR_DISTRIBUTION=YES build setting to enable library evolution.

To Reproduce Steps to reproduce the behavior:

  1. Check out this repository
  2. Run the following command
xcodebuild -project Adyen.xcodeproj -scheme Adyen -destination 'generic/platform=iOS' -archivePath "archives/Adyen-iphoneos.xcarchive" SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES clean archive

Expected behavior I'd expect no build errors when using the BUILD_LIBRARY_FOR_DISTRIBUTION=YES build setting.

Screenshots If applicable, add screenshots to help explain your problem.

Environment

Additional context This problem was first brought up in this issue: https://github.com/Adyen/adyen-ios/issues/1130

descorp commented 1 year ago

Hey @ffittschen

Thank you for your feedback! We have noticed this behavior lately but didn't have any research on this topic yet. We will keep you posted.

descorp commented 11 months ago

Hey @ffittschen

We would like to have a better understanding of your case. The "internal binary framework", who uses it?

descorp commented 10 months ago

Prospect PR #1413

goergisn commented 9 months ago

Hi @ffittschen,

We merged a fix for the described issue to develop yesterday 🤝 Feel free to check it out for confirmation for now.

Best, Alex

goergisn commented 9 months ago

Hi @ffittschen,

we just released 5.5.0 that contains support for library evolution.

Please let us know if you are running into any issues. And feel free to re-open the issue, if so. 👍

Best, Alex

ffittschen commented 7 months ago

Hi @goergisn, while trying to update to 5.5.0, we are still facing an issue with Adyen's @_spi usage. Compiling the individual Adyen schemes with library evolution enabled and creating an xcframework out of them works fine now, but when importing them in our own code, we get the following error.

Cannot use conformance of 'AbstractPersonalinformationComponent' to 'Component' here; the conformance is declared as SPI

Adyen_library_evolution

Attached below you can find a minimal sample project that reproduces the issue. For the example, I compiled Adyen and its dependency AdyenNetworking and created XCFrameworks using the following script:

archive.sh ```bash #!/usr/bin/env bash set -e WORKING_DIR=$(pwd) BUILD_PRODUCTS_DIRECTORY="archives" PROJECT_NAME="Adyen.xcodeproj" SCHEME_NAME="$1" FRAMEWORK_PATH="${WORKING_DIR}/${BUILD_PRODUCTS_DIRECTORY}/${SCHEME_NAME}.xcframework" SIMULATOR_ARCHIVE_PATH="${WORKING_DIR}/${BUILD_PRODUCTS_DIRECTORY}/${SCHEME_NAME}-iphonesimulator.xcarchive" IOS_DEVICE_ARCHIVE_PATH="${WORKING_DIR}/${BUILD_PRODUCTS_DIRECTORY}/${SCHEME_NAME}-iphoneos.xcarchive" DERIVED_DATA_PATH="${WORKING_DIR}/DerivedData" mkdir -p "${BUILD_PRODUCTS_DIRECTORY}" echo "Created ${BUILD_PRODUCTS_DIRECTORY}" echo "Archiving ${SCHEME_NAME}" set -o pipefail && xcodebuild -project "${PROJECT_NAME}" -scheme "${SCHEME_NAME}" -destination 'generic/platform=iOS' -archivePath "${IOS_DEVICE_ARCHIVE_PATH}" -derivedDataPath "${DERIVED_DATA_PATH}" SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED="NO" CODE_SIGNING_ALLOWED="NO" clean archive | tee "${WORKING_DIR}/${BUILD_PRODUCTS_DIRECTORY}/${SCHEME_NAME}-iphoneos.log" | xcbeautify set -o pipefail && xcodebuild -project "${PROJECT_NAME}" -scheme "${SCHEME_NAME}" -destination 'generic/platform=iOS Simulator' -archivePath "${SIMULATOR_ARCHIVE_PATH}" -derivedDataPath "${DERIVED_DATA_PATH}" SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED="NO" CODE_SIGNING_ALLOWED="NO" clean archive | tee "${WORKING_DIR}/${BUILD_PRODUCTS_DIRECTORY}/${SCHEME_NAME}-iphonesimulator.log" | xcbeautify set -o pipefail && xcodebuild -create-xcframework \ -framework "${SIMULATOR_ARCHIVE_PATH}/Products/Library/Frameworks/${SCHEME_NAME}.framework" \ -debug-symbols "${SIMULATOR_ARCHIVE_PATH}/dSYMs/${SCHEME_NAME}.framework.dSYM" \ -framework "${IOS_DEVICE_ARCHIVE_PATH}/Products/Library/Frameworks/${SCHEME_NAME}.framework" \ -debug-symbols "${IOS_DEVICE_ARCHIVE_PATH}/dSYMs/${SCHEME_NAME}.framework.dSYM" \ -output "${FRAMEWORK_PATH}" ```

After creating the XCFrameworks, I generated the project using Tuist (which should not be relevant for the example, but I included the Project.swift for completeness) and built the LibraryEvolutionTest scheme, which results in the same error as in the screenshot above.

Sample project

LibraryEvolutionTest.zip

ffittschen commented 7 months ago

Please correct me if I'm wrong, but looking at the original PR that introduced the @_spi usage, it looks to me like it is only used to hide types from the DocC documentation. Since Swift 5.8, there is a @_documentation(visibility:) annotation, which was introduced specifically for this purpose.

Maybe it would make sense to migrate from @_spi to @_documentation(visibility: internal)? That would most likely also eliminate the need to provide default implementations the compiler complained about when compiling @_spi annotated code with library evolution.

goergisn commented 7 months ago

Hi @ffittschen Thank you for sharing + providing a sample project.

@_spi is used in our case to have visibility/accessibility of the specific objects within the SDK parts (Core, Session, Card, Components, ...) but not exposing them to the public interface. The reason for that is to not imply stability of these parts of the code (meaning not having to introduce breaking changes if any of these interfaces change).

I'm taking a look at it and keep you posted.

ffittschen commented 7 months ago

Ahh I see, maybe in that scenario the package access modifier introduced in Swift 5.9 would help to explicitly remove those SDK parts from the public interface instead of hiding them through SPI groups, while still being able to access them within the SPM package?

goergisn commented 7 months ago

Yes, we're currently exploring the scope of switching to the package access level as we also use @_spi on protocol properties/methods which don't allow individual access control and might require some more thought/work.