[Installer] Add support for source provider plugin hook #3792

amorde · 2015-07-07T22:13:26Z

This PR provides an initial/example implementation for the source provider plugin api described in #3190

I very much welcome suggestions for improvements - still new to both Ruby and the CocoaPods code base so it is very likely I am missing something.

The API would allow plugins to supply their own sources, whether they be un-versioned or using an unsupported VCS such as CVS, Hg, SVN, etc.

Example:

# in a Podfile
plugin 'cocoapods-repo-svn', {
    :sources => [
        'https://svn.myrepository.com'
    ]
}

# Now in the plugin's cocoapods_plugin.rb file:
Pod::HooksManager.register('cocoapods-repo-svn', :source_provider) do |context, options|
    # options contains the hash defined in the Podfile
    source_names = options['sources']
    source_names.each do |name|
        source = MyCustomCreateSourceFunction(name)
        # Add any number of Pod::Source objects to the context
        context.add_source(source)
    end
end

segiddins · 2015-07-08T03:17:32Z

lib/cocoapods/installer/source_provider_hooks_context.rb

+    class SourceProviderHooksContext
+      # @return [Podfile] The Podfile for the project.
+      #
+      attr_accessor :podfile


I don't see why the sources would need access to the podfile?

You are totally correct, I only included it because before I was using the Podfile as a way to insert my sources manually. With this plugin, it isn't needed.

segiddins · 2015-07-08T03:19:07Z

I'd really love to see a test for this that actually uses a plugin source

amorde · 2015-07-08T15:32:03Z

Hey @segiddins, thanks for the feedback! I have tested my own plugin which supports "local" sources (ex. "private" for ~/.cocoapods/repos/private) but I am unable to publish that code at the moment. That might change soon.

I'll work on the changes you suggested

amorde · 2015-07-09T18:02:31Z

I apologize for the numerous commits, I cannot get rubocop to run on my machine due to some weird issues with gemfiles so I don't know about violations until the travis build tells me. If anyone who has access would like to cancel those extra builds please do so!

amorde · 2015-07-09T18:15:27Z

Ping @segiddins

segiddins · 2015-07-09T18:36:03Z

bundle exec rake spec should run all of the specs, including rubocop

amorde · 2015-07-09T19:00:40Z

Yes, I have tried that and I get the following:

Bundler could not find compatible versions for gem "cocoapods-core":
  In snapshot (Gemfile.lock):
    cocoapods-core (= 0.38.0.beta.1)

  In Gemfile:
    cocoapods (>= 0) ruby depends on
      cocoapods-core (= 0.38.0.beta.2) ruby

    cocoapods-core (>= 0) ruby

Running `bundle update` will rebuild your snapshot from scratch, using only
the gems in your Gemfile, which may resolve the conflict.

I have tried running bundle update but it never fixes the issue. I have even tried recreating the Rainforest repo from scratch and I get either this or some version conflict with xcodeproj...

Anyways, any comments on my changes? I was able to get a test in using the plugin API albeit a bit verbose as far as tests go

amorde · 2015-07-10T17:05:43Z

ping @orta

orta · 2015-07-10T17:15:30Z

Hello! So, from my perspective this looks all good. It also seems a small enough change that I'd prefer it getting in with 0.38. Given that 0.38 already comes with plugin API changes. I'm going to leave it to either @kylef / @neonichu / @segiddins to do the actual merging however, as they understand the codebase significantly better than I do.

amorde · 2015-07-10T17:19:43Z

I'd be more than happy to make a new PR will all of those extra commits squashed into one if that makes it easier / cleaner, I did a terrible job of passing the rubocop tests heh..

orta · 2015-07-10T17:37:22Z

Sure, do it, feel free to close this one - or you can squash the commits, and do a force push 👍

amorde · 2015-07-10T17:47:36Z

Chose the second option!

orta · 2015-07-10T17:53:13Z

Great, consider yourself accomplished in your task. Someone will finish up the merge job.

segiddins · 2015-07-13T02:36:02Z

lib/cocoapods/installer.rb

@@ -33,6 +33,7 @@ class Installer
    autoload :FileReferencesInstaller,  'cocoapods/installer/file_references_installer'
    autoload :PostInstallHooksContext,  'cocoapods/installer/post_install_hooks_context'
    autoload :PreInstallHooksContext,   'cocoapods/installer/pre_install_hooks_context'
+    autoload :SourceProviderHooksContext, 'cocoapods/installer/source_provider_hooks_context'


please fix the alignment here

Even without a space they will not be aligned.. Do you want me to rename it so it is shorter?

Nope, probably the best thing to do is add a space to all the others so they're aligned again :)

Oops, I'm an idiot, that's a way better idea. Just pushed that in

segiddins · 2015-07-13T02:38:06Z

This seems good, but would like to also get @kylef to take a look at this.

👍

kylef · 2015-07-13T17:39:32Z

Code look fine, can you add a CHANGELOG entry @amorde?

The main concern I have with this implementation is that this API doesn't allow consumers to decide the order of sources with plugins. The ordering of sources is important as it can determine which source a Pod may be used from.

source 'https://github.com/CocoaPods/Specs'
source 'svn://foo'
source 'cvs://bar'

Is different to:

source 'svn://foo'
source 'https://github.com/CocoaPods/Specs'
source 'cvs://bar'

amorde · 2015-07-13T17:55:51Z

Sure, just pushed up the changelog now.

Regarding the order of sources, unless I am mistaken I believe changing the order of where you put the call to plugin would give you the desired behavior:

# This SVN source will be evaluated before the master repo
plugin 'cocoapods-repo-svn', {
    :sources => [
        'https://svn.myserver.com'
    ]
}
source 'https://github.com/CocoaPods/Specs.git

This should work as expected, because the installer does a push onto the sources of the analyzer. The analyzer's source accessor will instantiate the array with podfile.sources first as seen here

Edit: I'm wrong, ignore what I said about the ordering here

segiddins · 2015-07-13T17:59:10Z

I don't believe the order of plugin invocations will actually provide any ordering, though

amorde · 2015-07-13T17:59:33Z

Yeah, I just had that realization as well. I'm not sure how to fix that to be honest

kylef · 2015-07-13T17:59:50Z

@amorde I don't think that's the case, because the hooks won't be ran until run_source_provider_hooks, in your example the custom source (https://svn.myserver.com) will be after the https://github.com/CocoaPods/Specs.git source,

amorde · 2015-07-13T18:02:25Z

@segiddins @kylef You are both correct. If there is a way we can support ordering I would be more than willing to add it but I'm not sure how to do that as of right now. I feel that this would still be a valuable feature and we could add the ordering later, but I understand if you do not want to include it until it supports ordering

segiddins · 2015-07-13T18:22:38Z

It might make sense to make the invocation of plugins ordered, but that still wouldn't solve the issue of ordering between native sources and plugin sources

amorde · 2015-07-13T18:34:03Z

We could do something like this:

# in `Installer.resolve_dependencies`
context = run_source_provider_hooks
analyzer.sources.insert(0, *context.sources_before_builtin)
analyzer.sources.push(*context.sources_after_builtin)

Then let the plugin decide the order, possibly allowing the user to provide a flag as to whether to put it before or after the builtin repos

amorde · 2015-07-14T19:05:22Z

@kylef Any thoughts? I can implement some sort of source ordering, but I think this pull request has become a large enough discussion that it would be a good idea to discuss ordering elsewhere / in a separate issue

orta · 2015-07-14T19:14:05Z

It might just be acceptable enough to say that if you care enough to be using a plugin for a source provider, they should be of the highest priority so that providers / users don't have to add a weird index in their pod files / APIs.

amorde · 2015-07-14T19:20:43Z

Yeah, I agree. If using a private source that requires a plugin to support, you probably want your source to come first.

I will change the push to an insert, if that sounds reasonable

segiddins · 2015-07-14T20:19:50Z

Sounds reasonable to me

amorde · 2015-07-23T22:27:38Z

Just wondering if there's any info on this getting merged. We are looking to use this along with the cocoapods-repo-svn plugin at my workplace to manage our dependencies, but are waiting to switch our builds over until this is available

segiddins · 2015-07-23T22:28:14Z

@amorde just waiting for @kylef to review :)

kylef · 2015-07-24T07:01:13Z

What about executing the source_provider plugin hook right away when the plugin is used, along with supporting it's use as a source multiple times?

Example:

plugin 'cocoapods-svn', :source => 'svn://foo'
source 'https://github.com/CocoaPods/Specs'
plugin 'cocoapods-cvs', :source => 'cvs://bar'
plugin 'cocoapods-svn', :source => 'svn://zzz'

This way the user would get full control of ordering.

amorde · 2015-07-24T07:45:09Z

That sounds perfect, but I don't see how that would be implemented. Right now any call to plugin just adds entries to a hash. Like you mentioned earlier, they won't be executed until later.

It seems that to implement that would require massive changes to the way the podfile is evaluated

amorde · 2015-07-24T07:54:39Z

I think something that might be easier to implement would be to simply specify the master repo in the plugin's source, and let the plugin handle it (eg. just pass it off to the cocoapods implementation)

plugin 'cocoapods-repo-svn', :sources => [
       'svn://foo.com',
       'https://github.com/CocoaPods/Specs',
       'svn://bar.com'
    ]

Then the plugin could do something like

master = Pod::SourcesManager.master.first
if master.url == source_url
   context.add_source(master)
else
   # plugin source made here
end

amorde · 2015-07-28T18:12:37Z

@kylef @segiddins I believe what I described above would allow the ordering of sources as long as plugins support the master repo, which is fairly easy. So if you're Podfile contained this:

plugin 'cocoapods-repo-svn', :sources => [
       'svn://foo.com',
       'https://github.com/CocoaPods/Specs',
       'svn://bar.com'
    ]

It would behave as if you did this:

source 'svn://foo.com',
source 'https://github.com/CocoaPods/Specs',
source 'svn://bar.com'
# There will be a duplicate master at the end, which should not matter
source 'https://github.com/CocoaPods/Specs',

The time I am able to dedicate to this is wearing thin, so if a consensus cannot be reached I will have to abandon this, unfortunately. If there is anything I can do to make this work for you guys please let me know

dustywusty · 2015-07-29T22:34:56Z

@kylef i agree with @amorde that supporting true ordering will likely require some pretty deep changes. i could maybe help with this if this is how we decide we need to do it.

but i'm kinda in the same camp as @orta and feel that if you are far enough off the beaten path to use a source plugin then we can probably just resolve the source plugins in order before our regular source.

and thanks so much @amorde for the time and effort you're putting into this! this is great stuff!

eseilnacht · 2015-08-03T21:29:13Z

+1

orta · 2015-08-04T00:15:49Z

I feel like this is OK as is.

segiddins · 2015-08-04T01:03:11Z

I'm waiting for @kylef to review before merging, since he stated he had objections before

Whirlwind · 2015-08-05T15:17:50Z

It sounds good!

kylef · 2015-08-05T15:29:49Z

CHANGELOG.md

@@ -22,6 +22,10 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
  is now a default plugin.  
  [Samuel Giddins](https://github.com/segiddins)

+* Added a `:source_provider` hook to allow plugins to provide their own sources


This should be moved to go under master, not beta 2.

kylef · 2015-08-05T15:30:15Z

Other than that one comment about CHANGELOG. This is good to go.

Whirlwind · 2015-08-05T15:41:11Z

source 'svn://foo'
source 'https://github.com/CocoaPods/Specs'
source 'cvs://bar'

I hope the source should be able to auto select the plugin. or we can pass the source type:

source 'svn://foo', :type=>"svn"
source 'https://github.com/CocoaPods/Specs', :type=>"git"
source 'cvs://bar', :type=>"cvs"

I think the plugin is not a good idea. Somebody can use another plugin to pull the same source.

amorde · 2015-08-05T18:17:21Z

I'm having a difficult time including the upstream changes to the changelog.. could anyone help me out? It appears I've introduced a merge conflict

segiddins · 2015-08-05T18:24:52Z

@amorde just reset HEAD to the previous commit, and rebase? If not, I can handle merging this manually tonight.

amorde · 2015-08-05T18:28:08Z

Think I got it!

[Installer] Add support for source provider plugin hook

kylef · 2015-08-05T19:26:38Z

Thanks for your contribution @amorde.

strawnut · 2016-05-13T02:00:31Z

In my case, it is printed the error message.

NoMethodError - undefined method `check_version_information' for #<Pod::Source::Manager:0x007ffb4c04dc70>
/Library/Ruby/Gems/2.0.0/gems/cocoapods-repo-svn-2.0.1/lib/pod/command/repo_svn.rb:43:in `block in run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/user_interface.rb:63:in `section'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-repo-svn-2.0.1/lib/pod/command/repo_svn.rb:36:in `run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-repo-svn-2.0.1/lib/cocoapods_plugin.rb:32:in `update_or_add_source'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-repo-svn-2.0.1/lib/cocoapods_plugin.rb:10:in `block (2 levels) in <top (required)>'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-repo-svn-2.0.1/lib/cocoapods_plugin.rb:8:in `each'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-repo-svn-2.0.1/lib/cocoapods_plugin.rb:8:in `block in <top (required)>'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/hooks_manager.rb:109:in `call'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/hooks_manager.rb:109:in `block (3 levels) in run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/user_interface.rb:144:in `message'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/hooks_manager.rb:103:in `block (2 levels) in run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/hooks_manager.rb:101:in `each'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/hooks_manager.rb:101:in `block in run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/user_interface.rb:144:in `message'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/hooks_manager.rb:100:in `run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/installer.rb:482:in `run_source_provider_hooks'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/installer.rb:143:in `resolve_dependencies'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/installer.rb:114:in `install!'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/command/install.rb:37:in `run'
/Library/Ruby/Gems/2.0.0/gems/claide-1.0.0/lib/claide/command.rb:334:in `run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/lib/cocoapods/command.rb:50:in `run'
/Library/Ruby/Gems/2.0.0/gems/cocoapods-1.0.0/bin/pod:55:in `<top (required)>'
/usr/local/bin/pod:23:in `load'
/usr/local/bin/pod:23:in `<main>'

If I insert below lines then, printed above error messages.

plugin 'cocoapods-repo-svn', :sources => [
    'svn://svn.code.sf.net/p/codeblocks/code/trunk'
]

Dont' mind about svn url. Anything cannot be worked.
I should use svn repo.
Actually, it has being worked using CocoaPods v0.33.1, but I guess CocoaPods does not support svn repo officially. I can't find out official article.

coveralls · 2017-03-30T10:47:43Z

Changes Unknown when pulling fd2773d on amorde:master into ** on CocoaPods:master**.

segiddins reviewed Jul 8, 2015
View reviewed changes

dustywusty mentioned this pull request Jul 8, 2015

"Unable to find specification .." during pod install, but "pod spec which" succeeds dustywusty/cocoapods-repo-svn#7

Closed

amorde mentioned this pull request Jul 9, 2015

Add support for upcoming source_provider plugin hook dustywusty/cocoapods-repo-svn#8

Merged

segiddins reviewed Jul 13, 2015
View reviewed changes

segiddins assigned kylef Jul 29, 2015

segiddins added this to the 0.39 milestone Aug 1, 2015

segiddins mentioned this pull request Aug 5, 2015

Can CP support spec repo of SVN/HG/HTTP? #3974

Closed

kylef reviewed Aug 5, 2015
View reviewed changes

amorde added 4 commits August 5, 2015 11:27

[Installer] Add support for source provider plugin hook

15c2cb7

Give plugin sources precedence over any default sources

a2c8654

Include upstream changes to CHANGELOG

d06fcf7

Update changelog

fd2773d

kylef added a commit that referenced this pull request Aug 5, 2015

Merge pull request #3792 from amorde/master

1134aee

[Installer] Add support for source provider plugin hook

kylef merged commit 1134aee into CocoaPods:master Aug 5, 2015

strawnut unassigned kylef May 13, 2016

[Installer] Add support for source provider plugin hook #3792

[Installer] Add support for source provider plugin hook #3792

Conversation

amorde commented Jul 7, 2015

segiddins Jul 8, 2015

Choose a reason for hiding this comment

amorde Jul 8, 2015

Choose a reason for hiding this comment

segiddins commented Jul 8, 2015

amorde commented Jul 8, 2015

amorde commented Jul 9, 2015

amorde commented Jul 9, 2015

segiddins commented Jul 9, 2015

amorde commented Jul 9, 2015

amorde commented Jul 10, 2015

orta commented Jul 10, 2015

amorde commented Jul 10, 2015

orta commented Jul 10, 2015

amorde commented Jul 10, 2015

orta commented Jul 10, 2015

segiddins Jul 13, 2015

Choose a reason for hiding this comment

amorde Jul 13, 2015

Choose a reason for hiding this comment

segiddins Jul 13, 2015

Choose a reason for hiding this comment

amorde Jul 13, 2015

Choose a reason for hiding this comment

segiddins commented Jul 13, 2015

kylef commented Jul 13, 2015

amorde commented Jul 13, 2015

segiddins commented Jul 13, 2015

amorde commented Jul 13, 2015

kylef commented Jul 13, 2015

amorde commented Jul 13, 2015

segiddins commented Jul 13, 2015

amorde commented Jul 13, 2015

amorde commented Jul 14, 2015

orta commented Jul 14, 2015

amorde commented Jul 14, 2015

segiddins commented Jul 14, 2015

amorde commented Jul 23, 2015

segiddins commented Jul 23, 2015

kylef commented Jul 24, 2015

amorde commented Jul 24, 2015

amorde commented Jul 24, 2015

amorde commented Jul 28, 2015

dustywusty commented Jul 29, 2015

eseilnacht commented Aug 3, 2015

orta commented Aug 4, 2015

segiddins commented Aug 4, 2015

Whirlwind commented Aug 5, 2015

kylef Aug 5, 2015

Choose a reason for hiding this comment

kylef commented Aug 5, 2015

Whirlwind commented Aug 5, 2015

amorde commented Aug 5, 2015

segiddins commented Aug 5, 2015

amorde commented Aug 5, 2015

kylef commented Aug 5, 2015

strawnut commented May 13, 2016

coveralls commented Mar 30, 2017