adyen-ios icon indicating copy to clipboard operation
adyen-ios copied to clipboard

Support library evolution

Open ffittschen opened this issue 1 year ago • 10 comments

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

  • Device: n/a
  • iOS Version: n/a
  • SDK Version: n/a
  • Adyen API Version: n/a
  • Package management system: n/a
  • Xcode version: n/a
  • Mac OS type: n/a

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

ffittschen avatar Sep 01 '23 15:09 ffittschen

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 avatar Sep 04 '23 08:09 descorp

Hey @ffittschen

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

descorp avatar Sep 28 '23 15:09 descorp

Prospect PR #1413

descorp avatar Oct 16 '23 10:10 descorp

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 avatar Dec 06 '23 09:12 goergisn

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

goergisn avatar Dec 11 '23 10:12 goergisn

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
#!/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 avatar Jan 25 '24 16:01 ffittschen

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.

ffittschen avatar Jan 31 '24 07:01 ffittschen

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.

goergisn avatar Jan 31 '24 10:01 goergisn

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?

ffittschen avatar Feb 01 '24 15:02 ffittschen

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.

goergisn avatar Feb 02 '24 09:02 goergisn