-
-
Notifications
You must be signed in to change notification settings - Fork 132
Build from Git submodule dependencies instead of from CocoaPods #94
Conversation
This would fix #84. |
@dstarke can you address the issues in the CI? |
I'm looking at it. The error isn't in the logs, but I think what's going on is the build is trying to set up the CocoaPods environment, which should no longer be necessary, so I'll remove those from the automated build script. |
Investigating on my side the failures with CP, I'll probably PR on yours |
Progress, but the CocoaPods test is still failing. This is because the .podspec lint check can only check against official releases, and the last tagged release of SocketRocket was a long time ago and is not consistent with the version in the Git submodule (or the one specified in the Cartfile). If you were building an app with a pod file, you could specify where to get code from, but there doesn't appear to be any way to specify this in the .podspec. The SocketRocket project has open issues for making a new release (for related reasons) dating from July, September, and November, and it's unclear what their official release schedule will be. A milestone for a 0.6.0 release appears to be 100% complete, but hasn't been updated for two months. I'm not sure what the best way is to handle this. Regardless, I think anyone who uses this library through CocoaPods would need to specify an appropriate version of the SocketRocket library in their pod file, which isn't ideal. |
Originally, we used the 0.5.0 version of SR, it seems that your PR bumps to master for carthage which is not ideal. Checking if we put 0.5.1, latest published pod, would work correctly. |
I didn't change the Carthage settings, I just used what was there. I didn't notice the inconsistency, but I did fix a couple of build issues that I now realize must have been there because the code in the Git submodule was more recent than the code in the older library version. |
Now I recall why the discrepancy. SocketRocket doesn't build correctly as a carthage dependency (0.5.0, 0.5.1), this is why I pinned it to 'master' in the Cartfile with the submodules checked in. Now, the issue is that CP require a published dependency to be able to lint the podspec. To add to the problem, the API between 0.5 and 'master' changed, so that won't probably fit our bill, unless SocketRocket published 0.6, with proper Carthage compatibility it seems we reach a dead end on that side, breaking CP compatibility in favour of Carthage is not something I wanna move forward with. Last resort option would be to temporarily publish a compatible version of SR. |
I don't have a lot of experience with CP, but it seems like temporarily publishing a compatible version is the only way to proceed when you depend on a library that isn't being maintained regularly with official releases. My understanding is that it is still possible to produce a working app with this by including the git reference to the appropriate SR version directly in the pod file, but there's no way to get it to pass the lint check. On the Carthage side, the current situation is dangerous because the current configuration tells users of the library that it is ok to link with the master of SR, which can break in two unpredictable ways: this project could stop building correctly because of changes in SR (apparently, this has already happened, since I had to fix some issues to get the project to build), or a user could attempt to link using an older binary of this project with a newer build of SR, with unpredictable results at runtime. At the very least, we should pick a commit of SR to use for the Carthage configuration, so it isn't linking against a moving target. That should be the same commit as used to generate the temporary version for CP. |
I completely overlooked Carthage compatibility, when initially publishing it, I probably should rollback those commits and make sure CP builds are stable. |
Well, this version seems to be working for us in a project that uses Carthage (since Carthage allows you to point at anything in a Git repository, we've pointed our dependency at the branch this PR comes from). The API changes to get it to build were pretty minor. Were there other API changes in SR that we should be aware of that would affect the functionality of this library? |
Closing as superseded by #95 |
This pull request restructures the build to remove CocoaPods from the build process. Instead, the dependencies are pulled and built directly from the Git submodules which were already included in the repository. This allows the build to work immediately on a fresh checkout, and makes the framework compatible with tools like Carthage which may need to rebuild the project.
Some additional work may be required to get the example projects to work correctly.