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

Copy resources v2 #743

Closed
alloy opened this issue Jan 16, 2013 · 22 comments
Closed

Copy resources v2 #743

alloy opened this issue Jan 16, 2013 · 22 comments
Labels
t1:enhancement Enhancements that have not been picked up yet. Please comment if you plan to work on it t3:discussion These are issues that can be non-issues, and encompass best practices, or plans for the future.

Comments

@alloy
Copy link
Member

alloy commented Jan 16, 2013

The outcome of a discussion @irrationalfab and I had today:

We’re going to:

  • Perform processing on resources at pod install time. This includes:
    • Run pngcrush
    • Compile Core Data model.
  • All resources (after processing, if needed) will be placed in Pods/Resources and the copy-resources script will reference them from there.
  • Use rsync with options such as these to copy new/updated resources into the product.

Except in the case of a local :path pod:

  • No processing. The :path option is a DURING DEVELOPMENT ONLY feature and should not be used for release products.
  • Symlink resources into Pods/Resources, instead of copying them.

Updates:

  • mention the updated :path instead of :local directive, which is going to be deprecated
@alloy
Copy link
Member Author

alloy commented Jan 16, 2013

Related: #245

@alloy
Copy link
Member Author

alloy commented Jan 16, 2013

pngcrush comes with Xcode: http://developer.apple.com/library/ios/#qa/qa1681/_index.html

@fabiopelosin
Copy link
Member

It would be great to offer the possiblity of defining how each type of resource is optimized.

For example, png files appear to be best optimized with imageoptim:

@keith
Copy link
Member

keith commented Jan 16, 2013

I've used imageoptim with great success in the past

@0xced
Copy link
Contributor

0xced commented Jan 17, 2013

Here is what I think would be a good way to handle resources.

Xcode already has everything to package resources (compile xibs into nibs, compress png images etc.) into a resources bundle, why not leverage it? In MapBox, this problem is solved with a post install hook. In CoconutKit, this problem is solved with a separate repository for CocoaPods only! I think we can do better.

I will use the MapBox example to describe what Cocoapods could do when running pod install or pod update.

In Pods.xcodeproj:

  • Add a Resources group with subgroups for each pods needing a resources bundle and add all the resources defined in the podspec.
  • Create a target named MapBox Resources which must be a bundle target (explicitFileType = wrapper.cfbundle in the .pbxproj file)
    MapBox Resources - Build Settings
  • In this target, add all the resources in the Copy Bundle Resources build phase.
    MapBox Resources - Build Phases
  • Make the Pods target depend on the MapBox Resources target.
    Pods - Target Dependencies

In MyApp.xcodeproj:

  • Add MapBox.bundle (the product of the MapBox Resources target) in the Copy Bundle Resources phase of the app target with the same technique used to add libPods.a to the Link Binary With Libraries phase.
    MyApp - Copy Bundle Resources

As for resources which are not packed into a resources bundle, maybe cocoapods could just add them to the Copy Bundle Resources phase of the app target just like it added the MapBox.bundle. It would thus totally eliminate the Pods-resources.sh file and its associated script phase.

Cocoapods would need to know when to generate a resources bundle and when copying resources directly into the app. Here is quick proposition for the podspec syntax. I’m not familiar with Ruby, so maybe it doesn’t make much sense. Feel free to suggest something better.

  • Without resources bundle (same as current)

    s.resources = 'TSMiniWebBrowser/images/*.png', 'TSMiniWebBrowser/*.xib'
  • With resources bundle

    s.resources = { :files => ['MapView/Map/Resources/*.png'], :bundle => 'MapBox' }

Don’t hesitate to ask me if something is not clear.

@alloy
Copy link
Member Author

alloy commented Jan 17, 2013

@0xced There are a couple of issues with this.

  • First of all (and this is because I omitted some info in the original text), we were originally thinking of changing it by adding the pods resources to the user’s project, which is comparable to your suggestion. However, we’ve decided that we don’t want to touch the user’s project any more than absolutely necessary. We currently only do so when the user’s target needs configuring and I would very much like to keep it as minimal as that.
  • Your idea, however, to have the Pods Xcode project do the processing is very smart. Except, I don’t see how us making bundles is going to work. I love resource bundles, but referencing resources from them requires specific code, so we can’t just do that for the pod and expect everything to work. I do think we should mention in the ‘guides’ that people should use resource bundles for their libraries. (On a related note, I don’t see why CoconutKit has a resource bundle for CP and not for other types of usage.)

Nonetheless, it might definitely be worth it to check if adding the resources to the Pods Xcode project and have that process them and store them into the Pods/Resources dir (like suggested in the original text) is handier than having our own script.

@fabiopelosin
Copy link
Member

I really like where this discussion is going. So the idea is to use Xcode for creating the bundle, and hence doing the processing, is awesome. We could store set the products dir of the resource bundle targets in Pods/Resources and then use rsync with a custom script for copying the resources to the Pods project.

Advantages:

  • No specific code required for accessing the resources.
  • It would still be possible to post-process the resources with the post install hook of the Podfile.

Disadvantages:

  • Pods installed with the :local option would use a copy of the resources, defeating the purpose of the option in this regard.
    • They could be handled with something similar to the current system (the local option is for development only).
    • Or we could make the Pod lib targets dependent on the resources as described by @0xced.
  • Using the resources in this way (as it is in the current implementation) eliminates the name spacing provided by the bundles and will cause resources from different pods but with the same name to override each other.
  • The bundles are shared across all the Pod libs targets, so if a Pod is using some iOS and OS X specific resources, the two apps will contain all the resources.
  • Unless we create a bundle per Pod and per Target, making the implementation more complex.

@alloy
Copy link
Member Author

alloy commented Jan 17, 2013

My comment was probably a bit to dense. For clarity sake:

I don’t see how us making bundles is going to work. I love resource bundles, but referencing resources from them requires specific code, so we can’t just do that for the pod and expect everything to work.

@alloy
Copy link
Member Author

alloy commented Jan 17, 2013

Pods installed with the :local option would use a copy of the resources, defeating the purpose of the option in this regard.

To expand on what @irrationalfab said:

What we want is that when a user updates an existing resource it automatically makes its way into new builds. (The :local option is intended for in-development pods.)

For removed or new resources you will have to run pod install again in any case.

@alloy
Copy link
Member Author

alloy commented Jan 17, 2013

As it stands, I think that Xcode does just a couple of things to resources, and keeping a script like we currently have will allow us to deal with all cases.

The list we know of is:

  • Optimization of images: png for sure maybe also jpg and tiff
  • Compilation of Core Data models
  • If I recall correctly, it converts plist files to the binary format for iOS.
  • Compilation of xibs.

Do you know of others?

@fabiopelosin
Copy link
Member

After taking a look at the Xcode internals it turns out that it has some specification of store in xcspecs files (binary plist). The specs provide a good insight of what Xcode does to process resources. I've built a quick script to inspect those files and the results can be found at https://gist.github.com/4560417.

@0xced
Copy link
Contributor

0xced commented Jan 20, 2013

Let me first reply to this concern:

I don’t see how us making bundles is going to work. I love resource bundles, but referencing resources from them requires specific code, so we can’t just do that for the pod and expect everything to work.

Obviously we can not force the creation of a resources bundle by CocoaPods. That’s precisely why I suggested two different syntaxes for defining resources. For example, the TSMiniWebBrowser podspec author would write this:

s.resources = 'TSMiniWebBrowser/images/*.png', 'TSMiniWebBrowser/*.xib'

That would tell CocoaPods to copy all the resources into the main bundle of the target, which is what the TSMiniWebBrowser code is expecting.

For MapBox, the podspec author would write this:

s.resources = { :files => ['MapView/Map/Resources/*.png'], :bundle => 'MapBox' }

That would tell CocoaPods to create a MapBox.bundle resources bundle to copy into the main bundle of the target. Thus, podspec authors wouldn’t need a post install hook in order to create a resources bundle.


Now, about this:

However, we’ve decided that we don’t want to touch the user’s project any more than absolutely necessary.

Resources are equally necessary as code, I’m sure we all agree on that. ;-) CocoaPods is already modifying the user’s project by adding the Copy Pods Resources build phase, so I don’t see a big difference between adding a build phase and adding the resources directly to the Copy Bundle Resources phase. By the way, people seem to naturally expect that the resources would end up in the Copy Bundle Resources phase (emphasis mine):

You can specify resources that need to be copied from your pod into the target app. Those are not to be found in the Copy Bundle Resources build phase as you might expect, but there is a custom shell script “Copy Pods Resources” taking care of that.


Finally, about this:

What we want is that when a user updates an existing resource it automatically makes its way into new builds.

It works perfectly well thanks to Xcode’s built-in dependency mechanism. This is what happened when I modified the LoadingTile.png and LoadingTileZoom.png files and rebuilt the app target:
Resources Updated


Let me give you a few more arguments in favor of suppressing Pods-resources.sh and letting Xcode handle all the resources processing.

  • When writing CocoaPods, I’m sure you did not consider compiling the sources by adding a Compile Pods Sources build phase that would run a Pods-sources.sh script that would call clang for each file to compile and link them together into libPods.a. Why should this approach make more sense for resources ?
  • The output of a native build phase looks much better than the output of a script phase. Compare this:
    Copy Resources Bundle Phase
    with this:
    Copy Pods Resources Script Phase

@fabiopelosin
Copy link
Member

@0xced After giving some thought to it I think that your solution would be the best. Provide support for the bundles on top of the existing implementation. Then we should encourage the lib maintainers to use resource bundles.

However I not sure that the syntax that you are proposing would be the best solution. I'm considering the possibility of adding another attribute:

s.resources = 'TSMiniWebBrowser/images/*.png', 'TSMiniWebBrowser/*.xib'
s.resource_bundle = 'MapBox', ['MapView/Map/Resources/*.png']

This is so because we are support for the optional specification of the destination folder in the bundle (see the preview of the docs)

s.resources = { :frameworks => 'frameworks/CrashReporter.framework' }
s.resource_bundle = 'MapBox', { :resources => 'MapView/Map/Resources/*.png' }

There is also the issue #773 which is related. However I'm thinking about adding an attribute similar to header_mappings_dir.

@brutella
Copy link
Contributor

We should also consider using iphoneos-optimize which compresses pngs (via pngcrush) and converts .strings files to binary format (via plutil -convert binary1).

Usage

xcrun -sdk iphoneos iphoneos-optimize "${CONFIGURATION_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}"

@odrobnik
Copy link

IMHO this is missing the conversion of strings files into plists.

@alloy
Copy link
Member Author

alloy commented Apr 17, 2013

@0xced @irrationalfab @Cocoanetics After updating myself on the discussions here, I think it might be better to start with a podspec attribute that allows one to specify a ‘resource bundle’ task in a xcodeproject and have CP use xcodebuild to generate it. This means the current specs that build it from the post_install hook can start to migrate to the new attribute (making the specs more serialisable, which we need for other tasks) and it is likely that similar code will have to be in place as a foundation for CP creating the full resource bundles, as proposed by @0xced.

@brutella
Copy link
Contributor

@alloy +1 since resource bundles are generally a good idea for libraries. Otherwise it may come to resource name clashes.

@odrobnik
Copy link

I wrote this about a year ago: http://www.cocoanetics.com/2012/05/resource-bundles/

Crushing is only one of many more actions:

  • strings files get converted into binary property lists
  • XIB files get compiled into binary NIBs
  • images get pngcrush-ed
  • and many other actions for which there are build rules set up

I want to have a method for: building a specific target from a specific xcodeproj and have the output be added to the Resources.

The advantage would be that all projects that have a resource bundle target don't have to use a .bundle trick (which foregoes the processing of files) but could have the resource bundle be created exactly like it would when adding the project as a sub-project and adding the resource bundle as dependency.

@alloy
Copy link
Member Author

alloy commented Apr 17, 2013

@Cocoanetics Yes, this is what @0xced’s proposal is and what the rest of us agreed on. My latest update was about breaking up the full proposal into:

  • supporting those projects that already have such resource bundle targets
  • have CP create resource bundle targets for projects that don’t have resource bundle targets

Do you want to work on the feature that adds support for projects that already have their own resource bundle target? (So the former.)

@odrobnik
Copy link

@alloy I am not a Ruby programmer. iOS/Mac only. So I am totally useless for that.

@rivera-ernesto
Copy link
Contributor

Hi, I'm new here and I think that this issue could be solved along #1011 and #807 .

The Pods' syntax would describe either:

a) A static library (*.a) + "flat resources" (resources copied to the main bundle, like now).

b) A static framework (*.framework) à la iOS Universal Framework which compiles the code and process all kinds of resources preserving localization folders (en.lproj, etc).

The Pods target will then just be an aggregate target that has all .a and .framework as dependencies. I believe on OS X frameworks' resources are copied just fine, and for iOS a small script would be necessary to copy the resources from the Xcode-generated PodSample.framework/Resources folder to either a') main bundle, or b') a PodSample.bundle subfolder inside the main bundle (configurable).

@AliSoftware
Copy link
Contributor

One question there: if it will be pod install that does compile the xcdatamodeld to a momd (and not a Run Script Phase like now), will the xcdatamodeld still be available/referenced in the Pods.xcodeproj Project Navigator ?

It is often useful to at least be able to look at the datamodel directly from Xcode (especially for us as we use internal pods for some components that provide their own model and often need to look quickly at the datamodel to remind us about the model graph): I appreciate to be able to click on the xcdatamodeld file in the Project Navigator and see the DataModel graph directly in the Xcode editor (actually see #1155 about that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t1:enhancement Enhancements that have not been picked up yet. Please comment if you plan to work on it t3:discussion These are issues that can be non-issues, and encompass best practices, or plans for the future.
Projects
None yet
Development

No branches or pull requests

8 participants