Merge pull request #2830 from CocoaPods/seg-hooks-plugin-name

[HooksManager] Show the name of the source for each hook that is run in verbose mode

CHANGELOG.md

@@ -4,6 +4,24 @@ To install or update CocoaPods see this [guide](http://docs.cocoapods.org/guides
 
 To install release candidates run `[sudo] gem install cocoapods --pre`
 
+## Master
+
+##### Breaking
+
+* Only the hooks specified by usage of the `plugin` directive of the `Podfile`
+  will be run. Additionally, plugins that depend on hooks will have to update to
+  specify their 'plugin name' when registering the hooks in order to make it
+  possible for those hooks to be run.  
+  [Samuel Giddins](https://github.com/segiddins)
+  [#2640](https://github.com/CocoaPods/CocoaPods/issues/2640)
+
+##### Enhancements
+
+* Show the name of the source for each hook that is run in verbose mode.  
+  [Samuel Giddins](https://github.com/segiddins)
+  [#2639](https://github.com/CocoaPods/CocoaPods/issues/2639)
+
+
 ## 0.35.0
 
 [CocoaPods](https://github.com/CocoaPods/CocoaPods/compare/0.34.4...0.35.0)

Gemfile

@@ -14,8 +14,8 @@ gemspec
 gem 'json', '1.7.7'
 
 group :development do
-  #cp_gem 'claide',               'CLAide'
-  cp_gem 'cocoapods-core',       'Core'
+  cp_gem 'claide',               'CLAide'
+  cp_gem 'cocoapods-core',       'Core', 'seg-podfile-plugins'
   cp_gem 'cocoapods-downloader', 'cocoapods-downloader'
   cp_gem 'cocoapods-plugins',    'cocoapods-plugins'
   cp_gem 'cocoapods-trunk',      'cocoapods-trunk'

Gemfile.lock

 GIT
-  remote: https://github.com/CocoaPods/Core.git
-  revision: 8d0a5072f64a780dc3f7ef919a66bf82dcfc524d
+  remote: https://github.com/CocoaPods/CLAide.git
+  revision: b1eccc296b68b1a432e42ecf0c4b799d8551c75c
   branch: master
+  specs:
+    claide (0.7.0)
+
+GIT
+  remote: https://github.com/CocoaPods/Core.git
+  revision: e3ab1ae538f10500d5cb0035f458c71d308e68ad
+  branch: seg-podfile-plugins
   specs:
     cocoapods-core (0.35.0)
       activesupport (>= 3.2.15)
@@ -88,7 +95,6 @@ GEM
       parser (>= 2.2.0.pre.3, < 3.0)
     awesome_print (1.2.0)
     bacon (1.2.0)
-    claide (0.7.0)
     clintegracon (0.6.0)
       colored (~> 1.2)
       diffy
@@ -162,6 +168,7 @@ DEPENDENCIES
   awesome_print
   bacon
   bundler (~> 1.3)
+  claide!
   clintegracon
   cocoapods!
   cocoapods-core!

lib/cocoapods/command.rb

@@ -29,7 +29,7 @@ module Pod
     self.command = 'pod'
     self.version = VERSION
     self.description = 'CocoaPods, the Objective-C library package manager.'
-    self.plugin_prefix = 'cocoapods'
+    self.plugin_prefixes = %w(claide cocoapods)
 
     [Install, Update, Outdated, IPC::Podfile, IPC::Repl].each { |c| c.send(:include, ProjectDirectory) }
 

lib/cocoapods/hooks_manager.rb

+require 'rubygems'
+
 module Pod
   # Provides support for the hook system of CocoaPods. The system is designed
   # especially for plugins. Interested clients can register to notifications by
@@ -14,27 +16,70 @@ module Pod
   # from CocoaPods 1.0).
   #
   module HooksManager
+    # Represents a single registered hook.
+    #
+    class Hook
+      # @return [String]
+      #         The name of the hook's notification.
+      #
+      attr_reader :plugin_name
+
+      # @return [String]
+      #         The name of the plugin the hook came from.
+      #
+      attr_reader :name
+
+      # @return [Proc]
+      #         The block.
+      #
+      attr_reader :block
+
+      # @param  [String] name        @see {#name}.
+      #
+      # @param  [String] plugin_name @see {#plugin_name}.
+      #
+      # @param  [Proc]   block       @see {#block}.
+      #
+      def initialize(name, plugin_name, block)
+        raise ArgumentError, 'Missing name' unless name
+        raise ArgumentError, 'Missing block' unless block
+
+        UI.warn '[Hooks] The use of hooks without specifying a `plugin_name` ' \
+                'has been deprecated.' unless plugin_name
+
+        @name = name
+        @plugin_name = plugin_name
+        @block = block
+      end
+    end
+
     class << self
-      # @return [Hash{Symbol => Proc}] The list of the blocks that are
+      # @return [Hash{Symbol => Array<Proc>}] The list of the blocks that are
       #         registered for each notification name.
       #
       attr_reader :registrations
 
       # Registers a block for the hook with the given name.
       #
-      # @param  [Symbol] name
+      # @param  [String] plugin_name
+      #         The name of the plugin the hook comes from.
+      #
+      # @param  [Symbol] hook_name
       #         The name of the notification.
       #
       # @param  [Proc] block
       #         The block.
       #
-      def register(name, &block)
-        raise ArgumentError, 'Missing name' unless name
-        raise ArgumentError, 'Missing block' unless block
+      def register(plugin_name, hook_name = nil, &block)
+        # TODO: Backwards compatibility with nameless plugins from CP 0.34
+        if hook_name.nil?
+          hook_name = plugin_name
+          plugin_name = nil
+        end
 
         @registrations ||= {}
-        @registrations[name] ||= []
-        @registrations[name] << block
+        @registrations[hook_name] ||= []
+        @registrations[hook_name] << Hook.new(hook_name, plugin_name, block)
       end
 
       # Runs all the registered blocks for the hook with the given name.
@@ -45,15 +90,32 @@ module Pod
       # @param  [Object] context
       #         The context object which should be passed to the blocks.
       #
-      def run(name, context)
+      # @param  [Hash<String, Hash>] whitelisted_plugins
+      #         The plugins that should be run, in the form of a hash keyed by
+      #         plugin name, where the values are the custom options that should
+      #         be passed to the hook's block if it supports taking a second
+      #         argument.
+      #
+      def run(name, context, whitelisted_plugins = nil)
         raise ArgumentError, 'Missing name' unless name
         raise ArgumentError, 'Missing options' unless context
 
-        if @registrations
-          blocks = @registrations[name]
-          if blocks
-            blocks.each do |block|
-              block.call(context)
+        if registrations
+          hooks = registrations[name]
+          if hooks
+            UI.message "- Running #{name.to_s.gsub('_', ' ')} hooks" do
+              hooks.each do |hook|
+                next if whitelisted_plugins && !whitelisted_plugins.key?(hook.plugin_name)
+                UI.message "- #{hook.plugin_name || 'unknown plugin'} from " \
+                           "`#{hook.block.source_location.first}`" do
+                  block = hook.block
+                  if block.arity > 1
+                    block.call(context, whitelisted_plugins[hook.plugin_name])
+                  else
+                    block.call(context)
+                  end
+                end
+              end
             end
           end
         end

lib/cocoapods/installer.rb

@@ -97,6 +97,7 @@ module Pod
     def prepare
       UI.message 'Preparing' do
         sandbox.prepare
+        ensure_plugins_are_installed!
         Migrator.migrate(sandbox)
       end
     end
@@ -323,7 +324,23 @@ module Pod
     #
     def run_plugins_post_install_hooks
       context = HooksContext.generate(sandbox, aggregate_targets)
-      HooksManager.run(:post_install, context)
+      HooksManager.run(:post_install, context, podfile.plugins)
+    end
+
+    # Ensures that all plugins specified in the {#podfile} are loaded.
+    #
+    # @return [void]
+    #
+    def ensure_plugins_are_installed!
+      require 'claide/command/plugin_manager'
+
+      loaded_plugins = Command::PluginManager.specifications.map(&:name)
+
+      podfile.plugins.keys.each do |plugin|
+        unless loaded_plugins.include? plugin
+          raise Informative, "Your Podfile requires that the plugin `#{plugin}` be installed. Please install it and try installation again."
+        end
+      end
     end
 
     # Prints a warning for any pods that are deprecated

lib/cocoapods/user_interface.rb

@@ -294,7 +294,7 @@ module Pod
           string
         else
           first_space = ' ' * indent
-          indented = CLAide::Helper.wrap_with_indent(string, indent, 9999)
+          indented = CLAide::Command::Banner::TextWrapper.wrap_with_indent(string, indent, 9999)
           first_space + indented
         end
       end

lib/cocoapods/user_interface/error_report.rb

@@ -114,7 +114,7 @@ EOS
         end
 
         def installed_plugins
-          CLAide::Command::PluginsHelper.specifications.
+          CLAide::Command::PluginManager.specifications.
             reduce({}) { |hash, s| hash.tap { |h| h[s.name] = s.version.to_s } }
         end
 

cocoapods-integration-specs @ a59b7985

-Subproject commit 20622f4674797be935cbf1cd5d7ca5c96390f407
+Subproject commit a59b7985755a64a86f83d966ef6c8740067ea10f

spec/unit/hooks_manager_spec.rb

@@ -4,13 +4,17 @@ module Pod
   describe HooksManager do
     before do
       @hooks_manager = Pod::HooksManager
+      @hooks_manager.instance_variable_set(:@registrations, nil)
     end
 
     describe 'register' do
       it 'allows to register a block for a notification with a given name' do
-        @hooks_manager.register(:post_install) {}
+        @hooks_manager.register('plugin', :post_install) {}
         @hooks_manager.registrations[:post_install].count.should == 1
-        @hooks_manager.registrations[:post_install].first.class.should == Proc
+        hook = @hooks_manager.registrations[:post_install].first
+        hook.class.should == HooksManager::Hook
+        hook.name.should == :post_install
+        hook.plugin_name.should == 'plugin'
       end
 
       it 'raises if no name is given' do
@@ -24,6 +28,11 @@ module Pod
           @hooks_manager.register(:post_install)
         end
       end
+
+      it 'warns if no plugin name is given' do
+        @hooks_manager.register(:post_install) {}
+        UI.warnings.should.match /hooks without.*deprecated/
+      end
     end
 
     describe 'run' do
@@ -49,6 +58,26 @@ module Pod
         end
       end
 
+      it 'only runs hooks from the allowed plugins' do
+        @hooks_manager.register('plugin', :post_install) do |_options|
+          raise 'Should not be called'
+        end
+
+        should.not.raise do
+          @hooks_manager.run(:post_install, Object.new,  'plugin2' => {})
+        end
+      end
+
+      it 'passed along user-specified options when the hook block has arity 2' do
+        @hooks_manager.register('plugin', :post_install) do |_options, user_options|
+          user_options['key'].should == 'value'
+        end
+
+        should.not.raise do
+          @hooks_manager.run(:post_install, Object.new,  'plugin' => {'key' => 'value'})
+        end
+      end
+
       it 'raises if no name is given' do
         should.raise ArgumentError do
           @hooks_manager.run(nil, Object.new) {}
@@ -60,6 +89,20 @@ module Pod
           @hooks_manager.run(:post_install, nil)
         end
       end
+
+      it 'prints a message in verbose mode when any hooks are run' do
+        config.verbose = true
+        @hooks_manager.register(:post_install) {}
+        @hooks_manager.run(:post_install, Object.new)
+        UI.output.should.match /- Running post install hooks/
+      end
+
+      it 'prints a message in verbose mode for each hook run' do
+        config.verbose = true
+        @hooks_manager.register('plugin', :post_install) {}
+        @hooks_manager.run(:post_install, Object.new)
+        UI.output.should.match %r{- plugin from `spec/unit/hooks_manager_spec.rb`}
+      end
     end
   end
 end

spec/unit/installer_spec.rb

@@ -53,6 +53,7 @@ module Pod
         @installer.stubs(:generate_pods_project)
         @installer.stubs(:integrate_user_project)
         @installer.stubs(:run_plugins_post_install_hooks)
+        @installer.stubs(:ensure_plugins_are_installed!)
         @installer.stubs(:perform_post_install_actions)
       end
 
@@ -501,9 +502,36 @@ module Pod
       it 'runs plugins post install hook' do
         context = stub
         Installer::HooksContext.expects(:generate).returns(context)
-        HooksManager.expects(:run).with(:post_install, context)
+        HooksManager.expects(:run).with(:post_install, context, {})
         @installer.send(:run_plugins_post_install_hooks)
       end
+
+      it 'only runs the podfile-specified post-install hooks' do
+        context = stub
+        Installer::HooksContext.expects(:generate).returns(context)
+        plugins_hash = {'cocoapods-keys' => {'keyring' => 'Eidolon'}}
+        @installer.podfile.stubs(:plugins).returns(plugins_hash)
+        HooksManager.expects(:run).with(:post_install, context, plugins_hash)
+        @installer.send(:run_plugins_post_install_hooks)
+      end
+
+      it 'raises if a podfile-specified plugin is not loaded' do
+        @installer.podfile.stubs(:plugins).returns('cocoapods-keys' => {})
+        Command::PluginManager.stubs(:specifications).returns([])
+        should.raise Informative do
+          @installer.send(:ensure_plugins_are_installed!)
+        end.message.should.match /require.*plugin.*`cocoapods-keys`/
+      end
+
+      it 'does not raise if all podfile-specified plugins are loaded' do
+        @installer.podfile.stubs(:plugins).returns('cocoapods-keys' => {})
+        spec = stub
+        spec.stubs(:name).returns('cocoapods-keys')
+        Command::PluginManager.stubs(:specifications).returns([spec])
+        should.not.raise do
+          @installer.send(:ensure_plugins_are_installed!)
+        end
+      end
     end
 
     #-------------------------------------------------------------------------#