amplitude / analytics-connector-ios

Analytics connector library for seamless integration between Amplitude Analytics and Experiment SDKs
MIT License
1 stars 13 forks source link

Add support for BUILD_LIBRARY_FOR_DISTRIBUTION=YES #4

Open mikehouse opened 6 months ago

mikehouse commented 6 months ago

Hi. We've built the library with one version of swift compiler (Xcode 15.1/15.2) using BUILD_LIBRARY_FOR_DISTRIBUTION=YES and linked the library to a project that uses Xcode 15.3. After adding the library the project cannot be compiled anymore. The reason is names collision because of Swift compiler bug https://github.com/apple/swift/issues/56573. We fixed it by modifying private.swiftinterface file after it being built. Xcode shows such error when open the project analytics-connector-ios

Public class 'AnalyticsConnector.AnalyticsConnector' shadows module 'AnalyticsConnector', which may cause failures when importing 'AnalyticsConnector' or its clients in some configurations; please rename either the class 'AnalyticsConnector.AnalyticsConnector' or the module 'AnalyticsConnector', or see https://github.com/apple/swift/issues/56573 for workarounds

bgiori commented 6 months ago

Hi @mikehouse thanks for submitting this issue.

Not sure exactly what the right approach to solve this is. Either we need to rename the package (breaking) or we need to rename the Experiment class (breaking). So either way a major version upgrade would be required.

The swift issue you linked proposes:

If that isn't possible, you may be able to work around this bug by adding the special compiler flags -Xfrontend -module-interface-preserve-types-as-written to OTHER_SWIFT_FLAGS. This solution is not 100% reliable; in particular, you may have to manually implement conformances to protocols like Hashable to get them to print correctly. It is also something your module's clients may need to adopt if their interfaces refer to types from the affected module.

However, I am concerned about the reliability. Since this issue is only present when building the library using BUILD_LIBRARY_FOR_DISTRIBUTION=YES I am hesitant to potentially break the class conformances for a non-standard use case.

Are you aware of any other approaches that might work around this issue without renaming either the module or the class, or using the -Xfrontend -module-interface-preserve-types-as-written swift flags?

mikehouse commented 6 months ago

Hi @bgiori. Yes, we found that just fixing private.swiftinterface is enough. We run such script after Carthage finished building libraries

if [[ -d ./Carthage/Build/AnalyticsConnector.xcframework ]]; then
    for swiftinterface in $(find ./Carthage/Build/AnalyticsConnector.xcframework -type f -name "*private.swiftinterface"); do
        sed -i '' 's/AnalyticsConnector\.//' "${swiftinterface}"
    done
fi

if [[ -d ./Carthage/Build/Experiment.xcframework ]]; then
    for swiftinterface in $(find ./Carthage/Build/Experiment.xcframework -type f -name "*private.swiftinterface"); do
        sed -i '' 's/Experiment\.//' "${swiftinterface}"
        sed -i '' 's/Experiment\.//' "${swiftinterface}"
        sed -i '' 's/Experiment\.//' "${swiftinterface}"
    done
fi

Thank you.

crleona commented 3 weeks ago

While it doesn't quite resolve the issue if building from source, we have begun to distribute a precompiled version of this framework in our releases. Carthage should automatically default to this precompiled version.

Thanks to @mikehouse for script to fix the conflicting module names.