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

release/v1.6.2-dylib

Open neekolas opened this issue 1 month ago β€’ 3 comments

release/v1.6.2-dylib

Revert "Remove dylib"

This reverts commit ff88ff53bf5ffbd2225099b100fb353f89ec8c2f.

neekolas avatar Nov 26 '25 01:11 neekolas

[!WARNING] This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

neekolas avatar Nov 26 '25 01:11 neekolas

Claude finished @neekolas's task β€”β€” View job


PR Review: release/v1.6.2-dylib

This PR reverts the removal of dylib support and configures the Swift package to use dynamic linking. Here's my analysis:


βœ… Code Quality & Best Practices

Good:

  • Clean revert pattern with proper commit message referencing the original commit
  • Consistent naming convention change from LibXMTPSwiftFFI β†’ LibXMTPSwiftFFIDynamic across all files
  • Version naming follows semantic versioning with -dylib suffix for clear differentiation

Observations:

  • The workflow file comments still reference LibXMTPSwiftFFI.xcframework but the code uses LibXMTPSwiftFFIDynamic.xcframework (Package.swift:57-67 and workflow:57-66). Consider updating the comment on line 57 of the workflow file for clarity.

πŸ› Potential Issues

Critical:

  1. Inconsistent xcframework naming: The workflow extracts and bundles LibXMTPSwiftFFIDynamic.xcframework but the podspec still references LibXMTPSwiftFFI.xcframework at XMTP.podspec:28:

    spec.vendored_frameworks = 'LibXMTPSwiftFFI.xcframework'
    

    This mismatch will likely cause CocoaPods installation failures. The podspec should reference LibXMTPSwiftFFIDynamic.xcframework instead.

  2. Binary target URL validation: The new URL points to LibXMTPSwiftFFIDynamic.zip with a different checksum. Verify this artifact actually exists at the specified URL (Package.swift:29-30).

Medium: 3. Package.swift library type: The product is now explicitly set to .dynamic (Package.swift:12). This is intentional for this PR, but ensure downstream consumers are aware this changes the linking behavior and may affect app size/startup time.


⚑ Performance Considerations

Dynamic linking implications:

  • Startup time: Dynamic libraries are loaded at runtime, which may slightly increase app launch time compared to static linking
  • Memory: Dynamic libraries can be shared between processes, potentially reducing memory footprint if multiple processes use XMTP
  • Binary size: The main app binary will be smaller, but the total distribution size remains similar (dylib is separate)

For an SDK like XMTP, dynamic linking is reasonable if:

  • You expect the library to be updated independently
  • You're shipping via frameworks/XCFrameworks where dynamic is standard
  • You want to reduce main binary size

πŸ”’ Security Concerns

Low risk, but verify:

  • The checksum change (Package.swift:30) is expected when switching binary variants, but verify the new checksum matches the actual artifact to prevent supply chain attacks
  • Ensure the libxmtp release at swift-bindings-1.6.3.ab0df09 includes both static and dynamic variants
  • The workflow uses --skip-import-validation (line 84) which bypasses some CocoaPods checks - ensure this is intentional

πŸ§ͺ Test Coverage

Missing validation:

  • No automated tests verify the dynamic linking configuration
  • Consider adding a test that validates the library type in Package.swift matches expectations
  • The workflow should ideally test the built archive before publishing to CocoaPods

Recommendation: Add a validation step in the workflow before the CocoaPods deployment:

- name: Validate archive structure
  run: |
    unzip -l XMTP-${{ env.UPDATED_VERSION }}.zip
    # Verify LibXMTPSwiftFFIDynamic.xcframework exists

🎯 Summary & Action Items

Must fix before merge:

  1. ⚠️ Update XMTP.podspec:28 to reference LibXMTPSwiftFFIDynamic.xcframework instead of LibXMTPSwiftFFI.xcframework

Recommended improvements: 2. Verify the binary artifact exists at the URL specified in Package.swift:29 3. Update workflow comment on line 57 to reflect LibXMTPSwiftFFIDynamic 4. Add archive validation step before CocoaPods deployment 5. Document the dynamic linking change in release notes for consumers

Questions to consider:

  • Is the switch to dynamic linking intended for all consumers or just specific use cases?
  • Have you tested CocoaPods installation with the -dylib version suffix?
  • Are there any integration tests that verify the dynamic framework loads correctly?

The changes are well-structured and the revert is clean, but the xcframework name mismatch in the podspec is a blocking issue that will break CocoaPods installations.


claude[bot] avatar Nov 26 '25 01:11 claude[bot]

Configure SwiftPM product as dynamic and switch binary target to LibXMTPSwiftFFIDynamic for release/v1.6.2-dylib in Package.swift

Set product type: .dynamic, rename the binary target to LibXMTPSwiftFFIDynamic, and update its url and checksum in Package.swift; bump spec.version to 4.6.2-rc1-dylib in XMTP.podspec.

πŸ“Where to Start

Start with the product and binary target configuration in Package.swift, then verify the version bump in XMTP.podspec.


Macroscope summarized ec4b34d.

macroscopeapp[bot] avatar Nov 26 '25 01:11 macroscopeapp[bot]