Merge pull request #4408 from timbodeit/2597-variantgroups-localized-resources

Fix handling of localized files in Pods installed as frameworks

CHANGELOG.md

@@ -27,6 +27,10 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
   [Samuel Giddins](https://github.com/segiddins)
   [#4345](https://github.com/CocoaPods/CocoaPods/issues/4345)
 
+* Fix handling of localized files in Pods installed as frameworks.  
+  [Tim Bodeit](https://github.com/timbodeit)
+  [#2597](https://github.com/CocoaPods/CocoaPods/issues/2597)
+
 
 ## 0.39.0 (2015-10-09)
 

lib/cocoapods/project.rb

@@ -20,6 +20,7 @@ module Pod
       super(path, skip_initialization, object_version)
       @support_files_group = new_group('Targets Support Files')
       @refs_by_absolute_path = {}
+      @variant_groups_by_path_and_name = {}
       @pods = new_group('Pods')
       @development_pods = new_group('Development Pods')
       self.symroot = LEGACY_BUILD_ROOT
@@ -179,18 +180,7 @@ module Pod
     #
     def add_file_reference(absolute_path, group, reflect_file_system_structure = false)
       file_path_name = Pathname.new(absolute_path)
-      unless file_path_name.absolute?
-        raise ArgumentError, "Paths must be absolute #{absolute_path}"
-      end
-
-      if reflect_file_system_structure
-        relative_path = file_path_name.relative_path_from(group.real_path)
-        relative_dir = relative_path.dirname
-        relative_dir.each_filename do|name|
-          next if name == '.'
-          group = group[name] || group.new_group(name, name)
-        end
-      end
+      group = group_for_path_in_group(file_path_name, group, reflect_file_system_structure)
 
       if ref = reference_for_path(absolute_path)
         ref
@@ -272,6 +262,62 @@ module Pod
     #
     attr_reader :refs_by_absolute_path
 
+    # @return [Hash{[Pathname, String] => PBXVariantGroup}] The variant groups
+    #         grouped by absolute path of parent dir and name.
+    #
+    attr_reader :variant_groups_by_path_and_name
+
+    # Returns the group for an absolute file path in another group.
+    # Creates subgroups to reflect the file system structure if
+    # reflect_file_system_structure is set to true.
+    # Makes a variant group if the path points to a localized file inside a
+    # *.lproj folder. The same variant group is returned for files with the
+    # same name, even if their file extensions differ.
+    #
+    # @param  [Pathname] absolute_pathname
+    #         The pathname of the file to get the group for.
+    #
+    # @param  [PBXGroup] group
+    #         The parent group used as the base of the relative path.
+    #
+    # @param  [Bool] reflect_file_system_structure
+    #         Whether group structure should reflect the file system structure.
+    #         If yes, where needed, intermediate groups are created, similar to
+    #         how mkdir -p operates.
+    #
+    # @return [PBXGroup] The appropriate group for the filepath.
+    #         Can be PBXVariantGroup, if the file is localized.
+    #
+    def group_for_path_in_group(absolute_pathname, group, reflect_file_system_structure)
+      unless absolute_pathname.absolute?
+        raise ArgumentError, "Paths must be absolute #{absolute_pathname}"
+      end
+
+      relative_pathname = absolute_pathname.relative_path_from(group.real_path)
+      relative_dir = relative_pathname.dirname
+      lproj_regex = /\.lproj/i
+
+      # Add subgroups for folders, but treat .lproj as a file
+      if reflect_file_system_structure
+        relative_dir.each_filename do|name|
+          break if name.to_s =~ lproj_regex
+          next if name == '.'
+          group = group[name] || group.new_group(name, name)
+        end
+      end
+
+      # Turn .lproj into a variant group
+      if relative_dir.basename.to_s =~ lproj_regex
+        filename = absolute_pathname.basename.sub_ext('').to_s
+        lproj_parent_dir = absolute_pathname.dirname.dirname
+        group = @variant_groups_by_path_and_name[[lproj_parent_dir, filename]] ||
+          group.new_variant_group(filename, lproj_parent_dir)
+        @variant_groups_by_path_and_name[[lproj_parent_dir, filename]] ||= group
+      end
+
+      group
+    end
+
     #-------------------------------------------------------------------------#
   end
 end

spec/unit/project_spec.rb

 require File.expand_path('../../spec_helper', __FILE__)
 
 module Pod
+  # Expose to unit test file
+  class Project
+    public :group_for_path_in_group
+  end
+
   describe Project do
     before do
       @project = Project.new(config.sandbox.project_path)
@@ -164,6 +169,7 @@ module Pod
           @project.add_pod_group('BananaLib', config.sandbox.pod_dir('BananaLib'), false)
           @file = config.sandbox.pod_dir('BananaLib') + 'file.m'
           @nested_file = config.sandbox.pod_dir('BananaLib') + 'Dir/SubDir/nested_file.m'
+          @localized_file = config.sandbox.pod_dir('BananaLib') + 'Dir/SubDir/de.lproj/Foo.strings'
           @group = @project.group_for_spec('BananaLib')
         end
 
@@ -177,6 +183,16 @@ module Pod
           ref.hierarchy_path.should == '/Pods/BananaLib/Dir/SubDir/nested_file.m'
         end
 
+        it 'does not add subgroups for a file reference if not requested' do
+          ref = @project.add_file_reference(@nested_file, @group)
+          ref.hierarchy_path.should == '/Pods/BananaLib/nested_file.m'
+        end
+
+        it 'does not add subgroups for a file reference if requested not to' do
+          ref = @project.add_file_reference(@nested_file, @group, false)
+          ref.hierarchy_path.should == '/Pods/BananaLib/nested_file.m'
+        end
+
         it "it doesn't duplicate file references for a single path" do
           ref_1 = @project.add_file_reference(@file, @group)
           ref_2 = @project.add_file_reference(@file, @group)
@@ -184,6 +200,18 @@ module Pod
           @group.children.count.should == 1
         end
 
+        it 'creates variant group for localized file' do
+          ref = @project.add_file_reference(@localized_file, @group)
+          ref.hierarchy_path.should == '/Pods/BananaLib/Foo/Foo.strings'
+          ref.parent.class.should == Xcodeproj::Project::Object::PBXVariantGroup
+        end
+
+        it 'creates variant group for localized file in subgroup' do
+          ref = @project.add_file_reference(@localized_file, @group, true)
+          ref.hierarchy_path.should == '/Pods/BananaLib/Dir/SubDir/Foo/Foo.strings'
+          ref.parent.class.should == Xcodeproj::Project::Object::PBXVariantGroup
+        end
+
         it 'raises if the given path is not absolute' do
           should.raise ArgumentError do
             @project.add_file_reference('relative/path/to/file.m', @group)
@@ -193,6 +221,92 @@ module Pod
 
       #----------------------------------------#
 
+      describe '#group_for_path_in_group' do
+        before do
+          @project.add_pod_group('BananaLib', config.sandbox.pod_dir('BananaLib'), false)
+          poddir = config.sandbox.pod_dir('BananaLib')
+          subdir = poddir + 'Dir/SubDir/'
+          @file = poddir + 'file.m'
+          @nested_file = subdir + 'nested_file.h'
+          @nested_file2 = subdir + 'nested_file.m'
+          @localized_base_foo = subdir + 'Base.lproj/Foo.storyboard'
+          @localized_de_foo = subdir + 'de.lproj/Foo.strings'
+          @localized_de_bar = subdir + 'de.lproj/Bar.strings'
+          @localized_different_foo = poddir + 'Base.lproj/Foo.jpg'
+          @group = @project.group_for_spec('BananaLib')
+        end
+
+        it 'returns parent group when file is in main group directory' do
+          group = @project.group_for_path_in_group(@file, @group, true)
+          group.uuid.should == @group.uuid
+        end
+
+        it 'returns parent group when not localized or reflecting structure' do
+          group = @project.group_for_path_in_group(@nested_file, @group, false)
+          group.uuid.should == @group.uuid
+        end
+
+        it 'adds subgroups if reflecting file system structure' do
+          group = @project.group_for_path_in_group(@nested_file, @group, true)
+          group.hierarchy_path.should == '/Pods/BananaLib/Dir/SubDir'
+        end
+
+        it "doesn't duplicate groups for a single directory path" do
+          group_1 = @project.group_for_path_in_group(@nested_file, @group, true)
+          group_2 = @project.group_for_path_in_group(@nested_file2, @group, true)
+          group_1.uuid.should == group_2.uuid
+        end
+
+        it 'creates variant group for localized file' do
+          group = @project.group_for_path_in_group(@localized_base_foo, @group, false)
+          group.hierarchy_path.should == '/Pods/BananaLib/Foo'
+          group.class.should == Xcodeproj::Project::Object::PBXVariantGroup
+        end
+
+        it 'creates variant group for localized file when adding subgroups' do
+          group = @project.group_for_path_in_group(@localized_base_foo, @group, true)
+          group.hierarchy_path.should == '/Pods/BananaLib/Dir/SubDir/Foo'
+          group.class.should == Xcodeproj::Project::Object::PBXVariantGroup
+        end
+
+        it 'sets variant group path to the folder that contains .lproj bundles' do
+          group = @project.group_for_path_in_group(@localized_base_foo, @group, false)
+          group.real_path.should == config.sandbox.pod_dir('BananaLib') + 'Dir/SubDir'
+        end
+
+        it "doesn't duplicate variant groups for same name and directory" do
+          group_1 = @project.group_for_path_in_group(@localized_base_foo, @group, false)
+          group_2 = @project.group_for_path_in_group(@localized_de_foo, @group, false)
+          group_1.uuid.should == group_2.uuid
+          @group.children.count.should == 1
+        end
+
+        it 'makes separate variant groups for different names' do
+          group_1 = @project.group_for_path_in_group(@localized_base_foo, @group, false)
+          group_2 = @project.group_for_path_in_group(@localized_de_bar, @group, false)
+          group_1.uuid.should != group_2.uuid
+          @group.children.count.should == 2
+        end
+
+        it 'makes separate variant groups for different directory levels' do
+          group_1 = @project.group_for_path_in_group(@localized_base_foo, @group, false)
+          group_2 = @project.group_for_path_in_group(@localized_different_foo, @group, false)
+          group_1.uuid.should != group_2.uuid
+          @group.children.count.should == 2
+        end
+
+        it 'raises if the given path is not absolute' do
+          should.raise ArgumentError do
+            @project.add_file_reference('relative/path/to/file.m', @group, true)
+          end.message.should.match /Paths must be absolute/
+          should.raise ArgumentError do
+            @project.add_file_reference('relative/path/to/file.m', @group, false)
+          end.message.should.match /Paths must be absolute/
+        end
+      end
+
+      #----------------------------------------#
+
       describe '#reference_for_path' do
         before do
           @project.add_pod_group('BananaLib', config.sandbox.pod_dir('BananaLib'), false)