Cocoapods: If there are one pod using xcassets via resources spec syntax, then all xcassets will be copied and compied twice during [CP] Copy Pods Resources build phase

Created on 18 Jan 2019  Â·  29Comments  Â·  Source: CocoaPods/CocoaPods

Report

I've filled the issue template. But I've provide a detailed repo about this. You can check it if my description have some misunderstanding : CocoaPodsXCAssetsIssue

What did you do?

  • Have a iOS App project using CocoaPods
  • Have pods which use resources and resource_bundles to include xcassets resource. At least one pod using resources, others using resource_bundles
  • Run pod install
  • Build the project and check the Demo.app product

What did you expect to happen?

The Demo.app have two Assets.car, one is Demo.app/Assets.car, one is Demo.app/TestLibrary/Assets.car.

And each assets.car should contains individual image resource. Which means, using cartool, the output folder should have no common images.

What happened instead?

The Demo.app have two Assets.car, one is Demo.app/Assets.car, one is Demo.app/TestLibrary/Assets.car.

But, the main bundle's Assets.car, contains all the images inside that Demo.app/TestLibrary/Assets.car. The images in Pods and also be copied and compiled into the main bundle, cause a duplicated resouce and increase the ipa size.

CocoaPods Environment

Stack

   CocoaPods : 1.5.3
        Ruby : ruby 2.3.7p456 (2018-03-28 revision 63024) [universal.x86_64-darwin18]
    RubyGems : 2.5.2.3
        Host : Mac OS X 10.14 (18A391)
       Xcode : 10.1 (10B61)
         Git : git version 2.17.2 (Apple Git-113)
Ruby lib dir : /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib
Repositories : master - https://github.com/CocoaPods/Specs.git @ 77b6b8a7c9b07dbaf8f197b1446fddabec0b8ca7

Installation Source

Executable Path: /usr/local/bin/pod

Plugins

cocoapods-deintegrate : 1.0.2
cocoapods-plugins     : 1.0.0
cocoapods-search      : 1.0.0
cocoapods-stats       : 1.0.0
cocoapods-trunk       : 1.3.1
cocoapods-try         : 1.1.0

Project that demonstrates the issue

Please see the demo project here. Which can 100% trigger the issue and provide detail information

https://github.com/dreampiggy/CocoaPodsXCAssetsIssue

The issue seems like that #6159, but I provide more detailed information and reproduce demo and fix for this issue.

hard defect

Most helpful comment

All 29 comments

Yes this is a dup. I will keep this and close the other one we do not need two.

I want to tackle this once and for all for 1.7.0. It has about 5 issues open due to this and its a very bad issue.

@dnkoutso Good. One suggestion, I think the better way to solve this is not using something patch. If there are resource syntax with xcassets, we should use a better and correct way to collect the xcassets should be compiled.

For example, that Find all other xcassets logic, should use xcodeproj or somethinig, to check the Xcode Project's group, which is actually built for the current build target. Base on the file directory path is not a good idea. And even you can filter the Pods folder, what about the other folder, like WatchKit App's xcassets / iMessage / etc ? They may all inside the current working directory. That solution is not safe at all.

Our temp solution is to patch that Find all other xcassets with a custom ruby script do the things above. Maybe this can help to provide a better, robust official solution ?

Here it is:

The ruby script:

#!/usr/bin/ruby

require 'xcodeproj'

Encoding.default_external = 'utf-8'
Encoding.default_internal = 'utf-8'

path = ARGV[0]
target_name = ARGV[1]
project_path = path + '/Demo.xcodeproj/'
project = Xcodeproj::Project.open(project_path)
project.targets.each do |target|
    if target.name != target_name then next end
    target.resources_build_phase.files.each do |file|
        if file.file_ref.path && file.file_ref.path.end_with?("xcassets")
            puts file.file_ref.real_path 
        end
    end
end

The temp patch we used to solve the xcasses search (Hack but worked) :

image

Thanks, I will see what I can do. Planning to tackle this in February hopefully. No promises always :)

I have an idea on how to tackle this. Will implement and see and close it once and for all.

I think I have a patch ready for this.

not yet. My patch works but I realized how broken this is even today...I do have a plan though hoping to get to it by end of this week.

@dnkoutso Thanks for your hard-work. I think this is a historical issue, and it may contains many edge cases. Hope you can get a better and robust solution.

I still have a plan for this and still aim 1.7.0

All related PRs/Issues:

7785

8136

6159

7745

7779

7617

@dreampiggy Hi, maybe I have some other clues.
I found that the following shell code always return true. Therefore, cocoapods will always add all xcassets into XCASSET_FILES from OTHER_XCASSETS.

if [[ $line != "${PODS_ROOT}*" ]]; then
   XCASSET_FILES+=("$line")
fi

Maybe the correct way to use regex is: (remove the double quotation marks)

if [[ $line != ${PODS_ROOT}* ]]; then
   XCASSET_FILES+=("$line")
fi

I'm not sure if you have already noticed the problems since your temporary solution is is to limit the search zone.
You can try the following code in shell to verify the result.

#!/bin/sh
PODS_ROOT="/Users/dongxinb/Repos/Test/Pods/"
line="/Users/dongxinb/Repos/Test/Pods/TestPods/TestPods/Resources/XX.xcassets"
if [[ $line != "${PODS_ROOT}*" ]]; then
    echo "original true"
fi
if [[ $line != ${PODS_ROOT}* ]]; then
    echo "modified true"
fi

@dongxinb Correct the script is OK. But does not solve the problem from scratch. I've also point this simple fix in the demo repo in issue description: CocoaPodsXCAssetsIssue

The original script, still trying to search the xcassets, based on File System Path, but not Xcode Project Group logic part.

If I put a folder under this structure:

- *.xcodeproj
- Podfile
- Temp // <-- Temp folder and not in Xcode project, should not be included at all
-- temp1.xcassets
-- temp2.xcassets
- WatchKit
-- watchKit.xcassets // <-- WatchKit xcassets, should not be included in iOS product

Your finally iOS Application, will compile both temp1, temp2, watchKit into the ipa product. It's still a problem.

So the correct solution, should based on the Xcode Project Group, and should not change between different build times, only a new fresh pod install should generate a new xcassets list.

is this available in the 1.7.1 beta ? @dnkoutso

Thank you for your kind reply!
Yes, of course, the original way to grab the xcassets list is rude, which may include some xcassets by mistake.

But correcting the script may solve some problems in the short term. At least, pod will no longer to merge the xcassets from resource bundle into main .car file. I think this will obviously be benefical to those iOS project without extensions.
Now that the robust solution is still on the hard way, should we correct the script first? I’ll come up with a PR if necessary~

Wanted to fix this for 1.7.0 but never did. Punting to 1.8.0 for now or maybe 1.7.1.

Were the early results from your branch promising? Willing to help out if I can.

@cltnschlosser not much actually there are still issues with it and have reconsidered doing it from scratch and remove xcsasset management outside the script phase and directly add Pods_<target>_Assets.xcassets into the users Copy Bundle Resources phase. This will however make us lose per-configuration handling of xcassets that CocoaPods offers (e.g. having assets only being processed in Debug vs Release).

Is per configuration handling something that cocoapods actually exposes to end users via Podfile?

yes you can do pod 'MyPod', :configuration => 'Debug'

Oh nice. I have to imagine that doesn't work for assets currently anyway, because cocoapods is currently pulling in all xcassets, so if the pod was downloaded, it's assets would get included.

What's that rationale for checking everywhere for xcassets?

Seems like we should only need to grab xcassets referenced by the xcodeproj, and in podspec resources? or am I missing something obvious?

it is _very_ old code and was a bit difficult to manage or dig through why it was designed like this but it was naive and assumed a single app is integrated in the Podfile.

@dnkoutso Has the cocoapods team considered deprecating resources and then removing it in 2.0.0? It's not a short term fix, but resource_bundles just work 😆

@cltnschlosser there hasn't been a tremendous discussion on it but you are right this can be deprecated and a new DSL introduced that does the right thing.

I don't even know if anything new needs to be introduced. resource_bundles should be able to replace all resources usages and bundles don't have the namespace collision drawback.

Sure could be the case we deprecate and only force resource bundles.

All related PRs/Issues:

@dnkoutso , thanks for your hard working. #3405 may be a related PR/Issue

Also having a similar issue...

same problem

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dawnnnnn picture dawnnnnn  Â·  3Comments

tlandsmancars picture tlandsmancars  Â·  3Comments

pallaviMN picture pallaviMN  Â·  3Comments

iosdev-republicofapps picture iosdev-republicofapps  Â·  3Comments

marzapower picture marzapower  Â·  3Comments