Flipper: Refactor Sonar Podspec to include all of its dependencies

Created on 20 Jun 2018  路  19Comments  路  Source: facebook/flipper

We should make sure to declare all the Sonar dependencies within Sonar's podspec, and make sure they point to the versions they depend on(we could also use the ~> for floating versions).

It is very hard to maintain an application consuming Sonar since its Podfile has to manually define all Sonar's dependencies and the paths to the each one of the podspecs. What happens if we update Sonar? Then we also might have to manually update the dependencies Sonar is depending on.

There are two ways to prevent this from happening:

  • Upload all the Sonar's dependencies podspecs to the cocoapods public master repo along with Sonar podspec and make sure we define those dependencies in the Sonar's podspec.
  • Create a private repo and push the Sonar's podspec and its podspec dependencies to it. Then communicate to consumers of Sonar that they have to include the source of the private repo within the Podfile.

Sonar's consumers would go from this:

platform :ios, '8.0'
swift_version = '4.1'

target 'MyApp' do

  pod 'RSocket', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/third-party-podspecs/RSocket.podspec'
  pod 'DoubleConversion', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/third-party-podspecs/DoubleConversion.podspec'
  pod 'glog', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/third-party-podspecs/glog.podspec'
  pod 'Folly', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/third-party-podspecs/Folly.podspec'
  pod 'PeerTalk', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/third-party-podspecs/PeerTalk.podspec'
  pod 'Yoga','~>1.8.1', :modular_headers => true
  pod 'Sonar', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/xplat/Sonar/Sonar.podspec'
  pod 'SonarKit', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/SonarKit.podspec'
  pod 'SonarKit/SonarKitLayoutComponentKitSupport', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/SonarKit.podspec'
  pod 'SonarKit/SKIOSNetworkPlugin', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/SonarKit.podspec'
  pod 'ComponentKit', :podspec => 'https://raw.githubusercontent.com/facebook/Sonar/master/iOS/third-party-podspecs/ComponentKit.podspec'
  post_install do |installer|
        installer.pods_project.targets.each do |target|
            if ['YogaKit'].include? target.name
                target.build_configurations.each do |config|
                    config.build_settings['SWIFT_VERSION'] = swift_version
                end
            end
        end
    end
end

To this:

platform :ios, '8.0'
swift_version = '4.1'
# The path to the private repo source if we decide pod specs need to be hosted in a private repo.
source 'https://path/to/private/repo'
target 'MyApp' do
   pod 'Sonar', '~>x.y'
end

If you guys are OK with this approach I can make the changes, create the private repo and submit the merge request.

iOS

Most helpful comment

Dope, nice work!! It sounds like in order to be able to do this without a private repo, the following things need to happen:

  • PR for ComponentKit (already submitted and approved)
  • PR for Folly podspec (looks like there's no podspec currently in the master repo, so I guess we should create a PR to add one)
  • PR for PeerTalk edits
  • PR for Yoga and YogaKit podspecs (this could be done in one since they're in the same repo)

That makes sense right? Yoga/folly/componentkit look to be pretty active so PeerTalk might be the only one we have trouble with - hopefully we can get in contact with the maintainer and they can push a new version to Cocoapods master. If so, we can probably move forward with the Cocoapods master repo approach 馃拑

All 19 comments

+1 The multitude of explicit dependencies is preventing me to use Sonar :(

That seems a good solution. I will put forward this suggestion to our legal team and wait for their input on this.

@pkrmf you can go ahead and implement the above solution.

Hey @pkrmf thanks for the suggestion! as @priteshrnandgaonkar we would be super happy if you wanted to give this a shot. Otherwise we will look into it ourselves as soon as we fix the android build which is our current top priority.

Hey @pkrmf thanks for the suggestion! as @priteshrnandgaonkar we would be super happy if you wanted to give this a shot. Otherwise we will look into it ourselves as soon as we fix the android build which is our current top priority.

@emilsjolander I could implement solution 2 very easily, the only problem is that consumers of Sonar that also consume any of its dependencies would run into conflicts.

Solution 1 is definitely the right approach, where sonar dependencies all exist in cocoapods master repo. I started yesterday working on it, but there is quite work to do, for instance, I had to open an issue in componentKit regarding its podspec due to its Yoga dependency. Also they haven鈥檛 updated the cocoapods master in a long time, current sonarkit depends on componentkit master, but the master cocoapods repo only has until version 0.15.

I will keep identifying the dependencies that are breaking SonarKit.podspec and keep opening issues/pull requests on their repos. Eventually we should have a SonarKit.podspec that is able to lint so we can push it to the master repo.

SonarKit podspec itself has a bunch of issues that need to be addressed as well.

Sent with GitHawk

@emilsjolander I also opened a pull request that fixes the sonar iOS sample application. Now it will be able to pod install compile and run

Sent with GitHawk

@emilsjolander I could implement solution 2 very easily, the only problem is that consumers of Sonar that also consume any of its dependencies would run into conflicts.

Wouldn't different version number help solve the conflict? In such cases I think they can specify in the podfile which pod to refer to, the master or the private one. I think we can go ahead with solution 2 with solution 1 being a step 2 after solution 1.

@emilsjolander I could implement solution 2 very easily, the only problem is that consumers of Sonar that also consume any of its dependencies would run into conflicts. ...

@priteshrnandgaonkar adding version numbers is part of the fix for the current sonarkit podspec along with other issues

Sent with GitHawk

Just a note, when this issue is finished the SonarKit podspec should lint, right? That means we can close #11 when this is done.

@pkrmf when you say "private repo" for option 2, you're referring to a private cocoapods repo correct? Another short-term solution, if my thinking is correct, could be to implement these specs as subspecs of the SonarKit spec, so at least you didn't have to pull in dependencies from githubusercontent.com. I.e. SonarKit/PeerTalk, SonarKit/RSocket

@noahsark769 Yes sir. That is another good approach. If we could get all in cocoapods master it would be ideal.

@emilsjolander and @priteshrnandgaonkar I made very good progress. So far, I am able to lint SonarKit.podspec. Here is what I have done so far to be able to lint it:

  • Modify SonarKit.podspec. Along with several small fixes, I had to create a Core subspec that all the other subspaces depend on.
  • Create a CocoaPods private repository to host the pod specs that are not available in cocoapods master repo. The main reason I did this is to ensure I could eventually lint the podspec since I knew I was gonna be modifying existing dependencies(only podspecs).
  • Update ComponentKit.podspec and push it to the private repo. I also took care of submitting a pull request in ComponentKit repo to fix it. Will try to ask masters to push it to cocoapods master repo.
  • Update Folly.podspec. Applications would not be able to compile unless the ONLY_ACTIVE_ARCH for Folly and the app itself was set to YES. Ideally, we should be pushing the podspec to cocoapods master repo.
  • Update PeerTalk.podspec and source. Unfortunately I wasn't able to compile without making some small code changes in PeerTalk source. I will revisit this later, but we also would want to get the latest PeerTalk updated podspec pushed to cocoapods master.
  • Modified YogaKit to not depend on a patch version(same as ComponentKit) and publish a new version to my cocoapods private repo. This one should be an easy change, since all we need to do is submit a PR in YogaKit to set the dependency on Yoga 1.8 instead of 1.8.1
  • Modified Sonar podspec.
  • Publish a new version of Yoga podspec with a few small changes and then I uploaded it to the private repo. The version changes was basically to avoid conflicts with Yoga in the cocoapods master repo. The Yoga.podspec includes "pod_target_xcconfig": { "DEFINES_MODULE": "YES" }, which allows swift to consume static libraries.
  • Publish some of the other dependencies like RSocket to the cocoapods private repository.

I used the --use-libraries flag to lint the podspec. The reason for that is because we have dependencies that depend on other static libraries, and cocoapods wouldn't allow the podspec to lint.

This is the command I end up running:
pod spec lint SonarKit.podspec --sources=https://github.com/pkrmf/cocoapods-private-repo,master --allow-warnings --use-libraries

In the previous command, I basically tell cocoapods to lint the podspec using both master and my private repository, allow warnings and make sure to use static libraries, otherwise we will get the error I previously mentioned.

I tried other solutions, like making pods with static libraries dependencies as static_frameworks, so I could avoid passing the --use-libraries flag. By using static_frameworks in folly, or Sonar, for example, its consuming libraries/applications wouldn't be able to import them. The header mapping directory (#include <Sonar/someFile.h>, or modular imports(@import) wouldn't work at all.

If someone wants to take a look at the work, bring some new ideas or maybe solutions to the other workarounds, feel free to contribute. If this is the way to go, then we need to get started and upload the new modified podspecs to cocoapods master repo. For that, I believe we will need to get with the owners of those libraries.
We could otherwise leave the work as is, but we would have to require consumers of SonarKit to define my private repo as a source in their Podfile.

Dope, nice work!! It sounds like in order to be able to do this without a private repo, the following things need to happen:

  • PR for ComponentKit (already submitted and approved)
  • PR for Folly podspec (looks like there's no podspec currently in the master repo, so I guess we should create a PR to add one)
  • PR for PeerTalk edits
  • PR for Yoga and YogaKit podspecs (this could be done in one since they're in the same repo)

That makes sense right? Yoga/folly/componentkit look to be pretty active so PeerTalk might be the only one we have trouble with - hopefully we can get in contact with the maintainer and they can push a new version to Cocoapods master. If so, we can probably move forward with the Cocoapods master repo approach 馃拑

@pkrmf Can you also update the sample app in your fork and see if it works. Bytw great effort and awesome work. I would love to publish SonarKit pod in cocoapods master. Again great work. Will wait for your PR.

@noahsark769 That is correct sir!
@priteshrnandgaonkar Pull my fork and pod install the sample app. As you can see, it pod installs and runs the app without any problem. Link to Podfile in my fork

You could also copy the contents of my Podfile in yours and the pod install if you don't want to clone my fork. Should be the same.
I would do a pod cache clean --all, then a pod deintegrate and finally a pod install just in case.

By the way, this time I went ahead and run a pod repo push, which runs a pod spec lint under the covers and it successfully pushed SonarKit.podspec to my private repo.

@pkrmf , we can also have the relevant contents in your private repo in our repo as well, right? The reason for asking this is that it will be easy to maintain, as everything will be at one place.

@pkrmf , we can also have the relevant contents in your private repo in our repo as well, right? The reason for asking this is that it will be easy to maintain, as everything will be at one place.

@priteshrnandgaonkar Yes sir. The private repo con be located anywhere. The idea would be to not have a private repo at all and update all the broken podspecs dependencies like @noahsark769 mentioned. In the meantime I could do a PR of the private repo and the updated SonarKit.podspec to this repo if you prefer

Sent with GitHawk

@priteshrnandgaonkar I think we can close this now, correct?

@noahsark769, thanks for pointing it out. Thanks @pkrmf for fixing the podspec. But lets publish it in cocoapods master repo as a next step. Lets create a separate issue for that.

@priteshrnandgaonkar I will work on creating the issue and next steps.

Was this page helpful?
0 / 5 - 0 ratings