Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The output file entries of the Embed Pods Frameworks phase should be deduplicated. #7259

Closed
andersio opened this issue Dec 5, 2017 · 10 comments
Labels
r:new build system Issues related to Xcode's new build system introduced in Xcode 9
Milestone

Comments

@andersio
Copy link

andersio commented Dec 5, 2017

Report

What did you do?

Given the following dependency specification:

    pod 'Onfido'            , '~> 5.0', :configurations => [ 'Debug' ]
    pod 'Onfido-Release'    , '~> 5.0', :configurations => [ 'Release' ]

The CocoaPods integration creates a Embed Pods Framework phase, which has the two entries of the identical file path as its output file — in this case ${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/Onfido.framework. While this does not cause an issue with the old build system, the Xcode 9 new build system refuses to proceed unless either of the identical entries is manually removed.

What did you expect to happen?

It should work with the Xcode 9 new build system.

What happened instead?

The Xcode 9 new build system refuses to build the project with the said generated build phase, unless manual change is made (which gonna be overwritten by CocoaPods the next time it is invoked).

CocoaPods Environment

Stack

   CocoaPods : 1.3.1
        Ruby : ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin16]
    RubyGems : 2.6.14
        Host : Mac OS X 10.13.1 (17B1003)
       Xcode : 9.2 (9C40b)
         Git : git version 2.14.3 (Apple Git-98)
Ruby lib dir : /Users/anders/.rvm/rubies/ruby-2.4.1/lib
Repositories : master - https://github.com/CocoaPods/Specs.git @ f7e60e550a59a83a2cd6e970052fded553aab7b1

Installation Source

Executable Path: /Users/anders/.rvm/gems/ruby-2.4.1/bin/pod

Plugins

cocoapods-deintegrate : 1.0.1
cocoapods-plugins     : 1.0.0
cocoapods-search      : 1.0.0
cocoapods-stats       : 1.0.0
cocoapods-trunk       : 1.3.0
cocoapods-try         : 1.1.0

Example

https://github.com/andersio/TestCocoaPods

@andersio andersio changed the title Deduplicate Embed Pods Frameworks output files. The output file entries of the Embed Pods Frameworks phase should be deduplicated. Dec 5, 2017
@amorde
Copy link
Member

amorde commented Dec 5, 2017

Verified, removing the duplicate fixes the issue. Only an issue on the new Xcode 9 build system

Thank you for the detailed report and example project!

@dnkoutso
Copy link
Contributor

dnkoutso commented Dec 5, 2017

Isnt this fixed by 1.4.0? @amorde care to open a PR for this?

@amorde
Copy link
Member

amorde commented Dec 5, 2017

I'll check against master, forgot to do that. But yes I think I can tackle this

@dnkoutso
Copy link
Contributor

dnkoutso commented Dec 5, 2017

<3

@amorde
Copy link
Member

amorde commented Dec 5, 2017

master actually introduces another duplicate, it changed the resources script to this:

--- a/TestCocoaPods.xcodeproj/project.pbxproj
+++ b/TestCocoaPods.xcodeproj/project.pbxproj
@@ -217,7 +217,8 @@
                        );
                        name = "[CP] Copy Pods Resources";
                        outputPaths = (
-                               "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}",
+                               "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/Onfido-Onfido.bundle",
+                               "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/Onfido-Onfido.bundle",
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                        shellPath = /bin/sh;

I will try to get a fix in later today - @dnkoutso can you add the new build system tag?

@dnkoutso
Copy link
Contributor

dnkoutso commented Dec 5, 2017

ah master now correctly lists all bundles individually instead of my initial lazy approach to use the folder.

We need to inspect the Copy Pods Resources sh script and figure out if they end up in a different folder depending on the configuration (Debug vs Release, etc). If they are not, then we just need to uniq that list.

@dnkoutso
Copy link
Contributor

dnkoutso commented Dec 5, 2017

@amorde seems like configuration does not play role into the Pods-Target-resources.sh script so we just need to uniq that list I believe.

@dnkoutso
Copy link
Contributor

dnkoutso commented Dec 5, 2017

another workaround is the Onfido-Release should not use the same bundle name, but I can see this being a problem in code where you have to branch out to select the correct bundle name based on the configuration...

@dnkoutso dnkoutso added the r:new build system Issues related to Xcode's new build system introduced in Xcode 9 label Dec 5, 2017
@dnkoutso
Copy link
Contributor

dnkoutso commented Dec 5, 2017

@amorde I put a bit more thought on this. This diff should do it.

diff --git a/lib/cocoapods/installer/user_project_integrator/target_integrator.rb b/lib/cocoapods/installer/user_project_integrator/target_integrator.rb
index cf07812afd3..074583ba875 100644
--- a/lib/cocoapods/installer/user_project_integrator/target_integrator.rb
+++ b/lib/cocoapods/installer/user_project_integrator/target_integrator.rb
@@ -287,7 +287,7 @@ module Pod
                 base_path = '${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}'
                 output_extension = TargetIntegrator.output_extension_for_resource(File.extname(input_path))
                 File.join(base_path, File.basename(input_path, File.extname(input_path)) + output_extension)
-              end
+              end.uniq
             end
             TargetIntegrator.add_copy_resources_script_phase_to_target(native_target, script_path, input_paths, output_paths)
           end
@@ -320,7 +320,7 @@ module Pod
             output_paths = []
             unless framework_paths_by_config.all?(&:empty?)
               input_paths = [target.embed_frameworks_script_relative_path, *framework_paths_by_config.map { |fw| [fw[:input_path], fw[:dsym_input_path]] }.flatten.compact]
-              output_paths = framework_paths_by_config.map { |fw| [fw[:output_path], fw[:dsym_output_path]] }.flatten.compact
+              output_paths = framework_paths_by_config.map { |fw| [fw[:output_path], fw[:dsym_output_path]] }.flatten.compact.uniq
             end
             TargetIntegrator.add_embed_frameworks_script_phase_to_target(native_target, script_path, input_paths, output_paths)
           end

However, it is important to note that two pods that have the same bundle or framework on the SAME configuration should probably be a warning or an error to the user.

You can add such a check within target_validator.rb to ensure that for a single config (e.g. Debug) all resource bundle names and vendored artifact names are uniq.

The above diff fixes this issue which is valid since the same bundle name is being used for different configurations.

With the diff I above I now see:

diff --git a/TestCocoaPods.xcodeproj/project.pbxproj b/TestCocoaPods.xcodeproj/project.pbxproj
index 4a0059b..4bdd344 100644
--- a/TestCocoaPods.xcodeproj/project.pbxproj
+++ b/TestCocoaPods.xcodeproj/project.pbxproj
@@ -217,7 +217,7 @@
                        );
                        name = "[CP] Copy Pods Resources";
                        outputPaths = (
-                               "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}",
+                               "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/Onfido-Onfido.bundle",
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                        shellPath = /bin/sh;

@amorde
Copy link
Member

amorde commented Dec 5, 2017

That's actually exactly what I did :) just finishing up some tests for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r:new build system Issues related to Xcode's new build system introduced in Xcode 9
Projects
None yet
Development

No branches or pull requests

3 participants