Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mamba 2.0 SPM Support #146

Merged
merged 4 commits into from
Nov 13, 2024
Merged

Mamba 2.0 SPM Support #146

merged 4 commits into from
Nov 13, 2024

Conversation

rmigneco
Copy link
Collaborator

@rmigneco rmigneco commented Nov 4, 2024

Description

This PR adds support for Swift Package Manager (SPM)

Change Notes

  • Adds Package.swift file
  • Updates Project Dir Structure to support multiple targets for SPM to manage C and Swift sources with interdependencies
  • Updates Xcode project file to reflect moved files and ensures Framework builds and tests run
  • Fix import statements where necessary

Outstanding items

  • Handle project version for SPM (possibly a version.txt file as a resource)
  • Ensure CocoaPods integration works
  • Ensure Carthage Integration works
  • Update README

Pre-submission Checklist

  • I ran the unit tests locally before checking in.
  • I made sure there were no compiler warnings before checking in.
  • I have written useful documentation for all public code.
  • I have written unit tests for this new feature.

Attachments

  • example project demonstrating SPM integration
  • example project demonstrating Carthage integration

MambaCarthageTest.zip
MambaSPMTest.zip

@rmigneco rmigneco added the mamba2.0 A marker for features/bug fixes for a new version of mamba label Nov 4, 2024
@rmigneco rmigneco requested a review from theRealRobG November 4, 2024 22:14
@rmigneco
Copy link
Collaborator Author

rmigneco commented Nov 9, 2024

@theRealRobG Confirmed installation with Carthage and SPM both work with attached demo projects. It's been a while since I've done CocaPods so might need an assist there.

Copy link
Collaborator

@theRealRobG theRealRobG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff! This will finally fix #118 and fix #81 in both supported versions! 🥳

I've left a nit-pick comment, but really out of curiosity rather than asking for any change.

I'll see if I can get a valid test for the CocoaPod approach and after that I'll approve 👍

mamba.xcodeproj/project.pbxproj Show resolved Hide resolved
theRealRobG
theRealRobG previously approved these changes Nov 10, 2024
Copy link
Collaborator

@theRealRobG theRealRobG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested using the following project and all looked good:
TestMambaCocoaPods.zip

Some notes on how I tested:

  • Used brew install cocoapods to install CocoaPods.
  • Had to chmod 600 ~/.netrc for the pod install to work.
  • Had to follow this SO answer to compile the project: https://stackoverflow.com/a/76626156
  • The app imports mamba to parse and count a downloaded playlist that the user chooses via text input of a URL.

@rmigneco
Copy link
Collaborator Author

I tested using the following project and all looked good: TestMambaCocoaPods.zip

Some notes on how I tested:

  • Used brew install cocoapods to install CocoaPods.
  • Had to chmod 600 ~/.netrc for the pod install to work.
  • Had to follow this SO answer to compile the project: https://stackoverflow.com/a/76626156
  • The app imports mamba to parse and count a downloaded playlist that the user chooses via text input of a URL.

thank you for verifying this!

@rmigneco
Copy link
Collaborator Author

@theRealRobG two things left here (Unless you prefer to resolve in separate PRs)

  1. Providing versioning info for the SPM. We could add a text file as we do in the develop_.x` branch https://github.com/Comcast/mamba/blob/develop_1.x/mambaSharedFramework/Resources/version.txt
    I'm happy to add this but I'm not sure if there's another preferred way.

  2. Anything you'd like to add to the README?

@theRealRobG
Copy link
Collaborator

Ah, good point about the version.txt file @rmigneco, I think that's a good enough way of providing version information. That then needs to get updated in FrameworkInfo.swift similar to how it was done in develop_1.x (from memory, the version is used by mamba on writing the playlist, as it includes a comment with a Comcast copyright message as well as the library version). Without the version.txt, the block looking for the CFBundleShortVersionString fails, as with SPM the Bundle(for: PlaylistParser.self) doesn't exist.

- Updated Package.swift to process resources
- Updated FrameworkInfo to include the SPM path for extracting the version
@rmigneco rmigneco marked this pull request as ready for review November 11, 2024 15:15
@rmigneco
Copy link
Collaborator Author

Ah, good point about the version.txt file @rmigneco, I think that's a good enough way of providing version information. That then needs to get updated in FrameworkInfo.swift similar to how it was done in develop_1.x (from memory, the version is used by mamba on writing the playlist, as it includes a comment with a Comcast copyright message as well as the library version). Without the version.txt, the block looking for the CFBundleShortVersionString fails, as with SPM the Bundle(for: PlaylistParser.self) doesn't exist.

@theRealRobG this has been addressed and I think we are all set

@rmigneco rmigneco merged commit 263f3c7 into develop Nov 13, 2024
4 checks passed
@rmigneco rmigneco deleted the add-spm-support branch November 13, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mamba2.0 A marker for features/bug fixes for a new version of mamba
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants