Merge pull request #7631 from CocoaPods/segiddins/build-settings-refactor

Refactor build settings generation to be memoizable

CHANGELOG.md

@@ -18,6 +18,9 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
 * Improve performance of Pods project generator by skipping native targets for which dependent targets have already been added.
   [Jacek Suliga](https://github.com/jmkk)
 
+* Refactor build settings generation to perform much better on large projects.  
+  [Samuel Giddins](https://github.com/segiddins)
+
 ##### Bug Fixes
 
 * Remove [system] declaration attribute from generated module maps  
@@ -56,6 +59,12 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
   [Maxime Le Moine](https://github.com/MaximeLM)
   [#7590](https://github.com/CocoaPods/CocoaPods/issues/7590)
 
+* When integrating a vendored framework while building pods as static 
+  libraries, public headers will be found via `FRAMEWORK_SEARCH_PATHS` 
+  instead of via the sandbox headers store.  
+  [Samuel Giddins](https://github.com/segiddins)
+
+
 ## 1.5.0 (2018-04-04)
 
 ##### Enhancements

lib/cocoapods.rb

@@ -66,7 +66,6 @@ module Pod
     autoload :ModuleMap,               'cocoapods/generator/module_map'
     autoload :PrefixHeader,            'cocoapods/generator/prefix_header'
     autoload :UmbrellaHeader,          'cocoapods/generator/umbrella_header'
-    autoload :XCConfig,                'cocoapods/generator/xcconfig'
     autoload :AppTargetHelper,         'cocoapods/generator/app_target_helper'
   end
 

lib/cocoapods/generator/xcconfig.rb

-module Pod
-  module Generator
-    # Generates Xcode configuration files. A configuration file is generated
-    # for each Pod and for each Pod target definition. The aggregates the
-    # configurations of the Pods and define target specific settings.
-    #
-    module XCConfig
-      autoload :AggregateXCConfig,  'cocoapods/generator/xcconfig/aggregate_xcconfig'
-      autoload :PodXCConfig,        'cocoapods/generator/xcconfig/pod_xcconfig'
-      autoload :XCConfigHelper,     'cocoapods/generator/xcconfig/xcconfig_helper'
-    end
-  end
-end

lib/cocoapods/generator/xcconfig/pod_xcconfig.rb

-module Pod
-  module Generator
-    module XCConfig
-      # Generates the private xcconfigs for the pod targets.
-      #
-      # The xcconfig file for a Pod target merges the pod target
-      # configuration values with the default configuration values
-      # required by CocoaPods.
-      #
-      class PodXCConfig
-        # @return [Target] the target represented by this xcconfig.
-        #
-        attr_reader :target
-
-        # @return [Boolean] whether this xcconfig is for a test target.
-        #
-        attr_reader :test_xcconfig
-        alias test_xcconfig? test_xcconfig
-
-        # Initialize a new instance
-        #
-        # @param  [Target] target @see #target
-        #
-        # @param  [Boolean] test_xcconfig
-        #         whether this is an xcconfig for a test native target.
-        #
-        def initialize(target, test_xcconfig = false)
-          @target = target
-          @test_xcconfig = test_xcconfig
-        end
-
-        # Generates and saves the xcconfig to the given path.
-        #
-        # @param  [Pathname] path
-        #         the path where the xcconfig should be stored.
-        #
-        # @return [Xcodeproj::Config]
-        #
-        def save_as(path)
-          result = generate
-          result.save_as(path)
-          result
-        end
-
-        # Generates the xcconfig.
-        #
-        # @return [Xcodeproj::Config]
-        #
-        def generate
-          config = {
-            'FRAMEWORK_SEARCH_PATHS' => '$(inherited) ',
-            'GCC_PREPROCESSOR_DEFINITIONS' => '$(inherited) COCOAPODS=1',
-            'HEADER_SEARCH_PATHS' => '$(inherited) ' + XCConfigHelper.quote(target.header_search_paths(test_xcconfig?)),
-            'LIBRARY_SEARCH_PATHS' => '$(inherited) ',
-            'OTHER_CFLAGS' => '$(inherited) ',
-            'OTHER_LDFLAGS' => XCConfigHelper.default_ld_flags(target, test_xcconfig?),
-            'OTHER_SWIFT_FLAGS' => '$(inherited) ',
-            'PODS_ROOT' => '${SRCROOT}',
-            'PODS_TARGET_SRCROOT' => target.pod_target_srcroot,
-            'PRODUCT_BUNDLE_IDENTIFIER' => 'org.cocoapods.${PRODUCT_NAME:rfc1034identifier}',
-            'SKIP_INSTALL' => 'YES',
-            'SWIFT_ACTIVE_COMPILATION_CONDITIONS' => '$(inherited) ',
-            'SWIFT_INCLUDE_PATHS' => '$(inherited) ',
-          }
-
-          xcconfig = Xcodeproj::Config.new(config)
-
-          XCConfigHelper.add_settings_for_file_accessors_of_target(nil, target, xcconfig, true, test_xcconfig?)
-          target.file_accessors.each do |file_accessor|
-            xcconfig.merge!(file_accessor.spec_consumer.pod_target_xcconfig) if test_xcconfig? == file_accessor.spec.test_specification?
-          end
-          XCConfigHelper.add_target_specific_settings(target, xcconfig)
-          recursive_dependent_targets = target.recursive_dependent_targets
-          xcconfig.merge! XCConfigHelper.search_paths_for_dependent_targets(target, recursive_dependent_targets, test_xcconfig?)
-          XCConfigHelper.generate_vendored_build_settings(target, recursive_dependent_targets, xcconfig, false, test_xcconfig?)
-          if test_xcconfig?
-            test_dependent_targets = [target, *target.recursive_test_dependent_targets].uniq
-            xcconfig.merge! XCConfigHelper.search_paths_for_dependent_targets(target, test_dependent_targets - recursive_dependent_targets, test_xcconfig?)
-            XCConfigHelper.generate_vendored_build_settings(nil, target.all_dependent_targets, xcconfig, true, test_xcconfig?)
-            XCConfigHelper.generate_other_ld_flags(nil, target.all_dependent_targets, xcconfig)
-            XCConfigHelper.generate_ld_runpath_search_paths(target, false, true, xcconfig)
-          end
-          xcconfig
-        end
-
-        #-----------------------------------------------------------------------#
-      end
-    end
-  end
-end

lib/cocoapods/installer/analyzer.rb

@@ -441,7 +441,7 @@ module Pod
           user_project = nil
           client_root = config.installation_root.realpath
           user_target_uuids = []
-          user_build_configurations = target_definition.build_configurations || { 'Release' => :release, 'Debug' => :debug }
+          user_build_configurations = target_definition.build_configurations || Target::DEFAULT_BUILD_CONFIGURATIONS
           archs = []
           if target_definition.platform && target_definition.platform.name == :osx
             archs = ['$(ARCHS_STANDARD_64_BIT)']

lib/cocoapods/installer/xcode/pods_project_generator/aggregate_target_installer.rb

@@ -96,9 +96,9 @@ module Pod
           def create_xcconfig_file(native_target)
             native_target.build_configurations.each do |configuration|
               path = target.xcconfig_path(configuration.name)
-              gen = Generator::XCConfig::AggregateXCConfig.new(target, configuration.name)
-              xcconfig = update_changed_file(gen, path)
-              target.xcconfigs[configuration.name] = xcconfig
+              build_settings = target.build_settings(configuration.name)
+              update_changed_file(build_settings, path)
+              target.xcconfigs[configuration.name] = build_settings.xcconfig
               xcconfig_file_ref = add_file_to_support_group(path)
               configuration.base_configuration_reference = xcconfig_file_ref
             end

lib/cocoapods/installer/xcode/pods_project_generator/file_references_installer.rb

@@ -158,13 +158,6 @@ module Pod
                     added_public_headers = true
                     sandbox.public_headers.add_files(namespaced_path, files)
                   end
-
-                  unless pod_target.requires_frameworks?
-                    vendored_frameworks_header_mappings(headers_sandbox, file_accessor).each do |namespaced_path, files|
-                      added_public_headers = true
-                      sandbox.public_headers.add_files(namespaced_path, files)
-                    end
-                  end
                 end
 
                 pod_target.build_headers.add_search_path(headers_sandbox, pod_target.platform) if added_build_headers
@@ -332,36 +325,6 @@ module Pod
             mappings
           end
 
-          # Computes the destination sub-directory in the sandbox for headers
-          # from inside vendored frameworks.
-          #
-          # @param  [Pathname] headers_sandbox
-          #         The sandbox where the header links should be stored for this
-          #         Pod.
-          #
-          # @param  [Sandbox::FileAccessor] file_accessor
-          #         The consumer file accessor for which the headers need to be
-          #         linked.
-          #
-          def vendored_frameworks_header_mappings(headers_sandbox, file_accessor)
-            mappings = {}
-            file_accessor.vendored_frameworks.each do |framework|
-              headers_dir = Sandbox::FileAccessor.vendored_frameworks_headers_dir(framework)
-              headers = Sandbox::FileAccessor.vendored_frameworks_headers(framework)
-              framework_name = framework.basename(framework.extname)
-              dir = headers_sandbox + framework_name
-              headers.each do |header|
-                # the relative path of framework headers should be kept,
-                # not flattened like is done for most public headers.
-                relative_path = header.relative_path_from(headers_dir)
-                sub_dir = dir + relative_path.dirname
-                mappings[sub_dir] ||= []
-                mappings[sub_dir] << header
-              end
-            end
-            mappings
-          end
-
           #-----------------------------------------------------------------------#
         end
       end

lib/cocoapods/installer/xcode/pods_project_generator/pod_target_installer.rb

@@ -402,8 +402,7 @@ module Pod
           #
           def create_xcconfig_file(native_target, resource_bundle_targets)
             path = target.xcconfig_path
-            xcconfig_gen = Generator::XCConfig::PodXCConfig.new(target)
-            update_changed_file(xcconfig_gen, path)
+            update_changed_file(target.build_settings, path)
             xcconfig_file_ref = add_file_to_support_group(path)
 
             native_target.build_configurations.each do |c|
@@ -427,8 +426,7 @@ module Pod
           def create_test_xcconfig_files(test_native_targets, test_resource_bundle_targets)
             target.supported_test_types.each do |test_type|
               path = target.xcconfig_path(test_type.to_s)
-              xcconfig_gen = Generator::XCConfig::PodXCConfig.new(target, true)
-              update_changed_file(xcconfig_gen, path)
+              update_changed_file(Target::BuildSettings::PodTargetSettings.new(target, true), path)
               xcconfig_file_ref = add_file_to_support_group(path)
 
               test_native_targets.each do |test_target|

lib/cocoapods/sandbox/headers_store.rb

@@ -63,7 +63,7 @@ module Pod
           paths << "${PODS_ROOT}/#{headers_dir}/#{@relative_path}" if !use_modular_headers || @visibility_scope == :public
           paths << "${PODS_ROOT}/#{headers_dir}/#{entry[:path]}" if !use_modular_headers || @visibility_scope == :private
           paths
-        end.uniq
+        end.tap(&:uniq!).freeze
       end
 
       # Removes the directory as it is regenerated from scratch during each

lib/cocoapods/target.rb

+require 'cocoapods/target/build_settings'
+
 module Pod
   # Model class which describes a Pods target.
   #
@@ -8,6 +10,7 @@ module Pod
   class Target
     DEFAULT_VERSION = '1.0.0'.freeze
     DEFAULT_NAME = 'Default'.freeze
+    DEFAULT_BUILD_CONFIGURATIONS = { 'Release' => :release, 'Debug' => :debug }.freeze
 
     # @return [Sandbox] The sandbox where the Pods should be installed.
     #
@@ -33,6 +36,10 @@ module Pod
     #
     attr_reader :platform
 
+    # @return [BuildSettings] the build settings for this target.
+    #
+    attr_reader :build_settings
+
     # Initialize a new target
     #
     # @param [Sandbox] sandbox @see #sandbox
@@ -47,6 +54,8 @@ module Pod
       @user_build_configurations = user_build_configurations
       @archs = archs
       @platform = platform
+
+      @build_settings = create_build_settings
     end
 
     # @return [String] the name of the library.
@@ -228,5 +237,9 @@ module Pod
     def c99ext_identifier(name)
       name.gsub(/^([0-9])/, '_\1').gsub(/[^a-zA-Z0-9_]/, '_')
     end
+
+    def create_build_settings
+      BuildSettings.new(self)
+    end
   end
 end

lib/cocoapods/target/aggregate_target.rb

@@ -77,6 +77,16 @@ module Pod
       @xcconfigs = {}
     end
 
+    def build_settings(configuration_name = nil)
+      if configuration_name
+        @build_settings[configuration_name] ||
+          raise(ArgumentError, "#{self} does not contain a build setting for the #{configuration_name.inspect} configuration, only #{@build_settings.keys.inspect}")
+      else
+        @build_settings.each_value.first ||
+          raise(ArgumentError, "#{self} does not contain any build settings")
+      end
+    end
+
     # @return [Boolean] True if the user_target refers to a
     #         library (framework, static or dynamic lib).
     #
@@ -153,10 +163,6 @@ module Pod
       end
     end
 
-    def pod_targets_to_link
-      @pod_targets_to_link ||= pod_targets.to_set - search_paths_aggregate_targets.flat_map(&:pod_targets)
-    end
-
     # @return [Array<Specification>] The specifications used by this aggregate target.
     #
     def specs
@@ -309,5 +315,15 @@ module Pod
     def relative_to_srcroot(path)
       path.relative_path_from(client_root).to_s
     end
+
+    def create_build_settings
+      settings = {}
+
+      user_build_configurations.each_key do |configuration_name|
+        settings[configuration_name] = BuildSettings::AggregateTargetSettings.new(self, configuration_name)
+      end
+
+      settings
+    end
   end
 end

lib/cocoapods/target/pod_target.rb

@@ -275,7 +275,7 @@ module Pod
           accessor.resources.flat_map { |res| "${PODS_ROOT}/#{res.relative_path_from(sandbox.project.path.dirname)}" }
         end
         resource_bundles = accessors.flat_map do |accessor|
-          prefix = Generator::XCConfig::XCConfigHelper::CONFIGURATION_BUILD_DIR_VARIABLE
+          prefix = BuildSettings::CONFIGURATION_BUILD_DIR_VARIABLE
           prefix = configuration_build_dir unless accessor.spec.test_specification?
           accessor.resource_bundles.keys.map { |name| "#{prefix}/#{name.shellescape}.bundle" }
         end
@@ -535,7 +535,7 @@ module Pod
     #
     # @return [String] The absolute path to the configuration build dir
     #
-    def configuration_build_dir(dir = Generator::XCConfig::XCConfigHelper::CONFIGURATION_BUILD_DIR_VARIABLE)
+    def configuration_build_dir(dir = BuildSettings::CONFIGURATION_BUILD_DIR_VARIABLE)
       "#{dir}/#{label}"
     end
 
@@ -544,7 +544,7 @@ module Pod
     #
     # @return [String] The absolute path to the build product
     #
-    def build_product_path(dir = Generator::XCConfig::XCConfigHelper::CONFIGURATION_BUILD_DIR_VARIABLE)
+    def build_product_path(dir = BuildSettings::CONFIGURATION_BUILD_DIR_VARIABLE)
       "#{configuration_build_dir(dir)}/#{product_name}"
     end
 
@@ -610,5 +610,9 @@ module Pod
         Specification.root_name(dependency.name) == pod_name
       end
     end
+
+    def create_build_settings
+      BuildSettings::PodTargetSettings.new(self, false)
+    end
   end
 end

cocoapods-integration-specs @ 2faa5dd5

-Subproject commit f85066239cc102e9386b2417e7638512d2b696eb
+Subproject commit 2faa5dd550afc3158b119ebc1fe90eea5fa15825

spec/spec_helper.rb

@@ -146,7 +146,7 @@ def fixture_pod_target_with_specs(specs, host_requires_frameworks = false, user_
                      target_definitions, file_accessors, scope_suffix)
 end
 
-def fixture_aggregate_target(pod_targets = [], host_requires_frameworks = false, user_build_configurations = {},
+def fixture_aggregate_target(pod_targets = [], host_requires_frameworks = false, user_build_configurations = Pod::Target::DEFAULT_BUILD_CONFIGURATIONS,
                              archs = [], platform = Pod::Platform.new(:ios, '6.0'), target_definition = nil)
   target_definition ||= pod_targets.flat_map(&:target_definitions).first || fixture_target_definition
   Pod::AggregateTarget.new(config.sandbox, host_requires_frameworks, user_build_configurations, archs, platform,

spec/unit/generator/xcconfig_spec.rb

-require File.expand_path('../../../spec_helper', __FILE__)
-
-module Pod
-  describe Generator::XCConfig do
-  end
-end

spec/unit/installer/xcode/pods_project_generator/file_references_installer_spec.rb

@@ -90,12 +90,8 @@ module Pod
               framework_subdir_header = headers_root + 'BananaLib/Bananalib/SubDir/SubBananalib.h'
               public_headers.each { |public_header| public_header.should.exist }
               private_header.should.not.exist
-              framework_header.should.exist
-              framework_subdir_header.should.exist
-              config.sandbox.public_headers.search_paths(@pod_target.platform).should == %w(
-                ${PODS_ROOT}/Headers/Public
-                ${PODS_ROOT}/Headers/Public/BananaLib
-              )
+              framework_header.should.not.exist
+              framework_subdir_header.should.not.exist
             end
 
             it 'links the public headers meant for the user for a vendored framework' do

spec/unit/installer/xcode/pods_project_generator_spec.rb

@@ -322,8 +322,9 @@ module Pod
             user_target = stub('SampleApp-iOS-User-Target', :symbol_type => :application)
             user_target.expects(:common_resolved_build_setting).with('APPLICATION_EXTENSION_API_ONLY').returns('NO')
 
-            target = AggregateTarget.new(config.sandbox, false, {}, [],
-                                         Platform.new(:ios, '6.0'), fixture_target_definition,
+            target = AggregateTarget.new(config.sandbox, false,
+                                         { 'App Store' => :release, 'Debug' => :debug, 'Release' => :release, 'Test' => :debug },
+                                         [], Platform.new(:ios, '6.0'), fixture_target_definition,
                                          config.sandbox.root.dirname, proj, nil, [])
 
             target.stubs(:user_targets).returns([user_target])

spec/unit/target/build_settings_spec.rb

+require File.expand_path('../../../spec_helper', __FILE__)
+
+module Pod
+  class Target
+    describe BuildSettings do
+      def pod(pod_target, test_xcconfig = false)
+        BuildSettings::PodTargetSettings.new(pod_target, test_xcconfig)
+      end
+
+      def aggregate(aggregate_target, configuration_name = 'Release')
+        BuildSettings::AggregateTargetSettings.new(aggregate_target, configuration_name)
+      end
+
+      describe 'memoization' do
+        it 'memoizes methods when requested' do
+          cls = Class.new(BuildSettings) do
+            define_build_settings_method :foobar, :memoized => true do
+              Object.new
+            end
+          end
+
+          settings = cls.new(stub('Target'))
+
+          object = settings.foobar
+          object.should.be.frozen
+          object.should.equal?(settings.foobar)
+        end
+
+        it 'memoizes array methods when requested' do
+          cls = Class.new(BuildSettings) do
+            define_build_settings_method :foobar, :memoized => true, :sorted => true, :uniqued => true do
+              %w(b a c a)
+            end
+          end
+
+          settings = cls.new(stub('Target'))
+
+          object = settings.foobar
+          object.should.be.frozen
+          object.should == %w(a b c)
+          object.should.equal?(settings.foobar)
+        end
+      end
+
+      #---------------------------------------------------------------------#
+
+      describe '::add_developers_frameworks_if_needed' do
+        it 'adds the developer frameworks search paths to the xcconfig if SenTestingKit has been detected' do
+          xcconfig = BuildSettings.new(stub('Target'))
+          xcconfig.stubs(:frameworks => %w(SenTestingKit))
+          frameworks_search_paths = xcconfig.framework_search_paths
+          frameworks_search_paths.should == %w($(PLATFORM_DIR)/Developer/Library/Frameworks)
+        end
+
+        it 'adds the developer frameworks search paths to the xcconfig if XCTest has been detected' do
+          xcconfig = BuildSettings.new(stub('Target'))
+          xcconfig.stubs(:frameworks => %w(XCTest))
+          frameworks_search_paths = xcconfig.framework_search_paths
+          frameworks_search_paths.should == %w($(PLATFORM_DIR)/Developer/Library/Frameworks)
+        end
+      end
+
+      #---------------------------------------------------------------------#
+
+      describe '::add_language_specific_settings' do
+        it 'does not add OTHER_SWIFT_FLAGS to the xcconfig if the target does not use swift' do
+          target = fixture_pod_target('integration/Reachability/Reachability.podspec')
+          build_settings = pod(target)
+          other_swift_flags = build_settings.xcconfig.to_hash['OTHER_SWIFT_FLAGS']
+          other_swift_flags.should.be.nil
+        end
+
+        it 'does not add the -suppress-warnings flag to the xcconfig if the target uses swift, but does not inhibit warnings' do
+          target = fixture_pod_target('integration/Reachability/Reachability.podspec')
+          target.stubs(:uses_swift? => true, :inhibit_warnings? => false)
+          build_settings = pod(target)
+          other_swift_flags = build_settings.xcconfig.to_hash['OTHER_SWIFT_FLAGS']
+          other_swift_flags.should.not.include '-suppress-warnings'
+        end
+
+        it 'adds the -suppress-warnings flag to the xcconfig if the target uses swift and inhibits warnings' do
+          target = fixture_pod_target('integration/Reachability/Reachability.podspec')
+          target.stubs(:uses_swift? => true, :inhibit_warnings? => true)
+          build_settings = pod(target)
+          other_swift_flags = build_settings.xcconfig.to_hash['OTHER_SWIFT_FLAGS']
+          other_swift_flags.should.include '-suppress-warnings'
+        end
+      end
+
+      #---------------------------------------------------------------------#
+
+      describe 'concerning settings for file accessors' do
+        it 'does not propagate framework or libraries from a test specification to an aggregate target' do
+          target_definition = stub('target_definition', :inheritance => 'complete', :abstract? => false, :podfile => Podfile.new)
+          spec = stub('spec', :test_specification? => true)
+          consumer = stub('consumer',
+                          :libraries => ['xml2'],
+                          :frameworks => ['XCTest'],
+                          :weak_frameworks => [],
+                          :spec => spec,
+                         )
+          file_accessor = stub('file_accessor',
+                               :spec => spec,
+                               :spec_consumer => consumer,
+                               :vendored_static_frameworks => [config.sandbox.root + 'StaticFramework.framework'],
+                               :vendored_static_libraries => [config.sandbox.root + 'StaticLibrary.a'],
+                               :vendored_dynamic_frameworks => [config.sandbox.root + 'VendoredFramework.framework'],
+                               :vendored_dynamic_libraries => [config.sandbox.root + 'VendoredDyld.dyld'],
+                              )
+          pod_target = stub('pod_target',
+                            :file_accessors => [file_accessor],
+                            :requires_frameworks? => true,
+                            :dependent_targets => [],
+                            :recursive_dependent_targets => [],
+                            :sandbox => config.sandbox,
+                            :include_in_build_config? => true,
+                            :should_build? => false,
+                            :spec_consumers => [consumer],
+                            :static_framework? => false,
+                            :product_basename => 'PodTarget',
+                            :target_definitions => [target_definition],
+                           )
+          pod_target.stubs(:build_settings => pod(pod_target))
+          aggregate_target = fixture_aggregate_target([pod_target])
+          aggregate(aggregate_target).other_ldflags.should.not.include '-framework'
+        end
+      end
+
+      #---------------------------------------------------------------------#
+    end
+  end
+end