From 4ef270f8afbd3c51d9e3a8260f253253caa18a35 Mon Sep 17 00:00:00 2001 From: Dimitris Koutsogiorgas Date: Fri, 29 Sep 2017 18:16:36 -0700 Subject: [PATCH] Extend script phase DSL to support execution position --- lib/cocoapods-core/podfile/dsl.rb | 4 +++ .../podfile/target_definition.rb | 9 +++++-- lib/cocoapods-core/specification.rb | 11 ++++++-- lib/cocoapods-core/specification/consumer.rb | 10 +++++++- lib/cocoapods-core/specification/dsl.rb | 8 +++++- lib/cocoapods-core/specification/linter.rb | 4 +++ spec/podfile/target_definition_spec.rb | 23 +++++++++++++---- spec/specification/consumer_spec.rb | 25 +++++++++++++------ spec/specification/json_spec.rb | 17 ++++++++++--- spec/specification/linter_spec.rb | 8 +++++- spec/specification_spec.rb | 17 +++++++++++++ 11 files changed, 113 insertions(+), 23 deletions(-) diff --git a/lib/cocoapods-core/podfile/dsl.rb b/lib/cocoapods-core/podfile/dsl.rb index 652baa0a9..9bd8e4355 100644 --- a/lib/cocoapods-core/podfile/dsl.rb +++ b/lib/cocoapods-core/podfile/dsl.rb @@ -368,6 +368,10 @@ def target(name, options = nil) # @option options [Boolean] :show_env_vars_in_log # whether this script phase should output the environment variables during execution. # + # @option options [Symbol] :execution_position + # specifies the position of which this script phase should be executed. The currently supported values are: + # `:before_compile`, `:after_compile` and `:any` which is the default. + # # @return [void] # def script_phase(options) diff --git a/lib/cocoapods-core/podfile/target_definition.rb b/lib/cocoapods-core/podfile/target_definition.rb index c2bb8c210..30e9d3049 100644 --- a/lib/cocoapods-core/podfile/target_definition.rb +++ b/lib/cocoapods-core/podfile/target_definition.rb @@ -590,7 +590,7 @@ def store_podspec(options = nil) # @param [Hash] options # The options to use for this script phase. The required keys # are: `:name`, `:script`, while the optional keys are: - # `:shell_path`, `:input_files`, `:output_files` and `:show_env_vars_in_log`. + # `:shell_path`, `:input_files`, `:output_files`, `:show_env_vars_in_log` and `:execution_position`. # # @return [void] # @@ -598,7 +598,7 @@ def store_script_phase(options) option_keys = options.keys unrecognized_keys = option_keys - Specification::ALL_SCRIPT_PHASE_KEYS unless unrecognized_keys.empty? - raise StandardError, "Unrecognized options `#{unrecognized_keys}` in shell script `#{options}` within `#{name}` target. " \ + raise StandardError, "Unrecognized options `#{unrecognized_keys}` in shell script `#{options[:name]}` within `#{name}` target. " \ "Available options are `#{Specification::ALL_SCRIPT_PHASE_KEYS}`." end missing_required_keys = Specification::SCRIPT_PHASE_REQUIRED_KEYS - option_keys @@ -609,6 +609,11 @@ def store_script_phase(options) if script_phases_hash.map { |script_phase_options| script_phase_options[:name] }.include?(options[:name]) raise StandardError, "Script phase with name `#{options[:name]}` name already present for target `#{name}`." end + options[:execution_position] = :any unless options.key?(:execution_position) + unless Specification::EXECUTION_POSITION_KEYS.include?(options[:execution_position]) + raise StandardError, "Invalid execution position value `#{options[:execution_position]}` in shell script `#{options[:name]}` within `#{name}` target. " \ + "Available options are `#{Specification::EXECUTION_POSITION_KEYS}`." + end script_phases_hash << options end diff --git a/lib/cocoapods-core/specification.rb b/lib/cocoapods-core/specification.rb index 54b65fa27..f838adcf2 100644 --- a/lib/cocoapods-core/specification.rb +++ b/lib/cocoapods-core/specification.rb @@ -355,8 +355,15 @@ def prefix_header_file # @return [ArrayString}>] The script_phases value. # def script_phases - attributes_hash['script_phases'].map do |script_phase| - Specification.convert_keys_to_symbol(script_phase) + script_phases = attributes_hash['script_phases'] || [] + script_phases.map do |script_phase| + phase = Specification.convert_keys_to_symbol(script_phase) + phase[:execution_position] = if phase.key?(:execution_position) + phase[:execution_position].to_sym + else + :any + end + phase end end diff --git a/lib/cocoapods-core/specification/consumer.rb b/lib/cocoapods-core/specification/consumer.rb index 9b0907df3..c465af75f 100644 --- a/lib/cocoapods-core/specification/consumer.rb +++ b/lib/cocoapods-core/specification/consumer.rb @@ -381,7 +381,15 @@ def _prepare_prefix_header_contents(value) def _prepare_script_phases(value) if value value.map do |script_phase| - Specification.convert_keys_to_symbol(script_phase) if script_phase.is_a?(Hash) + if script_phase.is_a?(Hash) + phase = Specification.convert_keys_to_symbol(script_phase) + phase[:execution_position] = if phase.key?(:execution_position) + phase[:execution_position].to_sym + else + :any + end + phase + end end.compact end end diff --git a/lib/cocoapods-core/specification/dsl.rb b/lib/cocoapods-core/specification/dsl.rb index f5d418c4a..2d4a34183 100644 --- a/lib/cocoapods-core/specification/dsl.rb +++ b/lib/cocoapods-core/specification/dsl.rb @@ -1276,7 +1276,9 @@ def dependency(*args) SCRIPT_PHASE_REQUIRED_KEYS = [:name, :script].freeze - SCRIPT_PHASE_OPTIONAL_KEYS = [:shell_path, :input_files, :output_files, :show_env_vars_in_log].freeze + SCRIPT_PHASE_OPTIONAL_KEYS = [:shell_path, :input_files, :output_files, :show_env_vars_in_log, :execution_position].freeze + + EXECUTION_POSITION_KEYS = [:before_compile, :after_compile, :any].freeze ALL_SCRIPT_PHASE_KEYS = (SCRIPT_PHASE_REQUIRED_KEYS + SCRIPT_PHASE_OPTIONAL_KEYS).freeze @@ -1295,6 +1297,10 @@ def dependency(*args) # # @example # + # spec.script_phase = { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :before_compile } + # + # @example + # # spec.script_phase = { :name => 'Hello World', :script => 'puts "Hello World"', :shell_path => '/usr/bin/ruby' } } # # @example diff --git a/lib/cocoapods-core/specification/linter.rb b/lib/cocoapods-core/specification/linter.rb index 063828891..1259b6668 100644 --- a/lib/cocoapods-core/specification/linter.rb +++ b/lib/cocoapods-core/specification/linter.rb @@ -408,6 +408,10 @@ def _validate_script_phases(s) unless missing_required_keys.empty? results.add_error('script_phases', "Missing required shell script phase options `#{missing_required_keys.join(', ')}` in script phase `#{script_phase[:name]}`.") end + unless Specification::EXECUTION_POSITION_KEYS.include?(script_phase[:execution_position]) + results.add_error('script_phases', "Invalid execution position value `#{script_phase[:execution_position]}` in shell script `#{script_phase[:name]}`. " \ + "Available options are `#{Specification::EXECUTION_POSITION_KEYS}`.") + end end end diff --git a/spec/podfile/target_definition_spec.rb b/spec/podfile/target_definition_spec.rb index 6f7a2a5bf..1cfcd55ab 100644 --- a/spec/podfile/target_definition_spec.rb +++ b/spec/podfile/target_definition_spec.rb @@ -332,8 +332,14 @@ module Pod it 'raises if script phase includes an unrecognized key' do e = lambda { @parent.store_script_phase(:name => 'PhaseName', :unknown => 'Unknown') }.should.raise Podfile::StandardError - e.message.should == 'Unrecognized options `[:unknown]` in shell script `{:name=>"PhaseName", :unknown=>"Unknown"}` within `MyApp` target. ' \ - 'Available options are `[:name, :script, :shell_path, :input_files, :output_files, :show_env_vars_in_log]`.' + e.message.should == 'Unrecognized options `[:unknown]` in shell script `PhaseName` within `MyApp` target. ' \ + 'Available options are `[:name, :script, :shell_path, :input_files, :output_files, :show_env_vars_in_log, :execution_position]`.' + end + + it 'raises if script phase includes an invalid execution position key' do + e = lambda { @parent.store_script_phase(:name => 'PhaseName', :script => 'echo "Hello World"', :execution_position => :unknown) }.should.raise Podfile::StandardError + e.message.should == 'Invalid execution position value `unknown` in shell script `PhaseName` within `MyApp` target. ' \ + 'Available options are `[:before_compile, :after_compile, :any]`.' end it 'raises if the same script phase name already exists' do @@ -347,14 +353,21 @@ module Pod it 'stores a script phase if requirements are provided' do @parent.store_script_phase(:name => 'PhaseName', :script => 'echo "Hello World"') @parent.script_phases.should == [ - { :name => 'PhaseName', :script => 'echo "Hello World"' }, + { :name => 'PhaseName', :script => 'echo "Hello World"', :execution_position => :any }, ] end it 'stores a script phase with requirements and optional keys' do - @parent.store_script_phase(:name => 'PhaseName', :script => 'echo "Hello World"', :shell_path => :'/usr/bin/ruby') + @parent.store_script_phase(:name => 'PhaseName', :script => 'echo "Hello World"', :shell_path => '/usr/bin/ruby') + @parent.script_phases.should == [ + { :name => 'PhaseName', :script => 'echo "Hello World"', :shell_path => '/usr/bin/ruby', :execution_position => :any }, + ] + end + + it 'stores a script phase with a specified execution position value' do + @parent.store_script_phase(:name => 'PhaseName', :script => 'echo "Hello World"', :shell_path => '/usr/bin/ruby', :execution_position => :before_compile) @parent.script_phases.should == [ - { :name => 'PhaseName', :script => 'echo "Hello World"', :shell_path => :'/usr/bin/ruby' }, + { :name => 'PhaseName', :script => 'echo "Hello World"', :shell_path => '/usr/bin/ruby', :execution_position => :before_compile }, ] end diff --git a/spec/specification/consumer_spec.rb b/spec/specification/consumer_spec.rb index 226eb7f7c..15853ae85 100644 --- a/spec/specification/consumer_spec.rb +++ b/spec/specification/consumer_spec.rb @@ -284,12 +284,12 @@ module Pod it 'returns the script phases in correct format' do @spec.script_phases = { :name => 'Hello World', :script => 'echo "Hello World"' } - @consumer.script_phases.should == [{ :name => 'Hello World', :script => 'echo "Hello World"' }] + @consumer.script_phases.should == [{ :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :any }] end it 'returns the script phases in correct format with optional options' do @spec.script_phases = { :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby' } - @consumer.script_phases.should == [{ :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby' }] + @consumer.script_phases.should == [{ :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby', :execution_position => :any }] end it 'returns the script phases in correct format for multiple script phases' do @@ -298,22 +298,31 @@ module Pod { :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby' }, ] @consumer.script_phases.should == [ - { :name => 'Hello World', :script => 'echo "Hello World"' }, - { :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby' }, + { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :any }, + { :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby', :execution_position => :any }, ] end it 'handles multi-platform script phases' do @spec.ios.script_phases = { :name => 'Hello World iOS', :script => 'echo "Hello World iOS"' } - @consumer.script_phases.should == [{ :name => 'Hello World iOS', :script => 'echo "Hello World iOS"' }] + @consumer.script_phases.should == [{ :name => 'Hello World iOS', :script => 'echo "Hello World iOS"', :execution_position => :any }] end it 'returns both global and multi platform script phases' do @spec.script_phases = { :name => 'Hello World', :script => 'echo "Hello World"' } - @spec.ios.script_phases = { :name => 'Hello World iOS', :script => 'echo "Hello World iOS"' } + @spec.ios.script_phases = { :name => 'Hello World iOS', :script => 'echo "Hello World iOS"', :execution_position => :any } @consumer.script_phases.should == [ - { :name => 'Hello World', :script => 'echo "Hello World"' }, - { :name => 'Hello World iOS', :script => 'echo "Hello World iOS"' }, + { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :any }, + { :name => 'Hello World iOS', :script => 'echo "Hello World iOS"', :execution_position => :any }, + ] + end + + it 'retains the value set for execution position' do + @spec.script_phases = { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :before_compile } + @spec.ios.script_phases = { :name => 'Hello World iOS', :script => 'echo "Hello World iOS"', :execution_position => :after_compile } + @consumer.script_phases.should == [ + { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :before_compile }, + { :name => 'Hello World iOS', :script => 'echo "Hello World iOS"', :execution_position => :after_compile }, ] end diff --git a/spec/specification/json_spec.rb b/spec/specification/json_spec.rb index 7942f7617..fea78115a 100644 --- a/spec/specification/json_spec.rb +++ b/spec/specification/json_spec.rb @@ -118,11 +118,13 @@ module Pod it 'writes script phases' do @spec.script_phases = [ { :name => 'Hello World', :script => 'echo "Hello World"' }, + { :name => 'Hello World 2', :script => 'echo "Hello World 2"', :execution_position => :after_compile }, { :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby' }, ] hash = @spec.to_hash hash['script_phases'].should == [ { :name => 'Hello World', :script => 'echo "Hello World"' }, + { :name => 'Hello World 2', :script => 'echo "Hello World 2"', :execution_position => :after_compile }, { :name => 'Hello Ruby World', :script => 'puts "Hello Ruby World"', :shell_path => 'usr/bin/ruby' }, ] end @@ -182,8 +184,8 @@ module Pod result = Specification.from_hash(hash) result.script_phases.count.should.equal 2 result.script_phases.should == [ - { :name => 'Hello World', :script => 'echo "Hello World"' }, - { :name => 'Hello Ruby World', :script => 'puts "Hello World"', :shell_path => '/usr/bin/ruby' }, + { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :any }, + { :name => 'Hello Ruby World', :script => 'puts "Hello World"', :shell_path => '/usr/bin/ruby', :execution_position => :any }, ] end @@ -221,7 +223,16 @@ module Pod result = Specification.from_json(json) result.script_phases.count.should.equal 1 result.script_phases.should == [ - { :name => 'Hello World', :script => 'echo "Hello World"' }, + { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :any }, + ] + end + + it 'loads script phase execution position from json' do + json = '{"script_phases": [{"name": "Hello World", "script": "echo \"Hello World\"", "execution_position": "before_compile"}]}' + result = Specification.from_json(json) + result.script_phases.count.should.equal 1 + result.script_phases.should == [ + { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :before_compile }, ] end diff --git a/spec/specification/linter_spec.rb b/spec/specification/linter_spec.rb index e2654777e..658a95eb8 100644 --- a/spec/specification/linter_spec.rb +++ b/spec/specification/linter_spec.rb @@ -465,7 +465,13 @@ def result_should_include(*values) it 'checks script phases include the required keys' do @spec.script_phases = { :name => 'Hello World', :script => 'echo "Hello World"', :unknown => 'unknown' } result_should_include('script_phases', 'Unrecognized options `[:unknown]` in script phase `Hello World`. ' \ - 'Available options are `[:name, :script, :shell_path, :input_files, :output_files, :show_env_vars_in_log]`.') + 'Available options are `[:name, :script, :shell_path, :input_files, :output_files, :show_env_vars_in_log, :execution_position]`.') + end + + it 'checks script phases include a valid execution position value' do + @spec.script_phases = { :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :unknown } + result_should_include('script_phases', 'Invalid execution position value `unknown` in shell script `Hello World`. ' \ + 'Available options are `[:before_compile, :after_compile, :any]`.') end #------------------# diff --git a/spec/specification_spec.rb b/spec/specification_spec.rb index 458ca97a2..4e5db77dc 100644 --- a/spec/specification_spec.rb +++ b/spec/specification_spec.rb @@ -122,6 +122,23 @@ module Pod spec_1.to_s.should == 'No-name' end + it 'returns any empty array without any script phases' do + spec = @spec.dup + spec.script_phases.should == [] + end + + it 'returns the script phases with keys converted to symbols' do + spec = @spec.dup + spec.script_phase = [{ 'name' => 'Hello World', 'script' => 'echo "Hello World"', 'execution_position' => :before_compile }] + spec.script_phases.should == [{ :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :before_compile }] + end + + it 'returns the script phases with default execution position' do + spec = @spec.dup + spec.script_phase = [{ 'name' => 'Hello World', 'script' => 'echo "Hello World"' }] + spec.script_phases.should == [{ :name => 'Hello World', :script => 'echo "Hello World"', :execution_position => :any }] + end + describe '#validate_cocoapods_version' do it 'passes when none is specified' do spec_1 = Specification.new