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

Already on GitHub? Sign in to your account

CocoaPods Adds a Resource Bundle Scheme on Each `pod install` Invocation #1338

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

blakewatters commented Sep 5, 2013

So this is a weird one:

I have a pod that is installed via :path and integrated into a host Workspace in two targets: the main application and a KIF integration testing target. In the generated Pods.xcodeproj file there are two identically named resource bundles.

Each time I run pod install I am winding up with a new scheme named after the resource bundle with a monotonically increasing integer appended to the name.

Is a target identifier or something being regenerated for the resource bundle each time? Happy to drive a fix for this but I am at a loss as where to begin. Any thoughts @alloy or @irrationalfab ?

Owner

fabiopelosin commented Sep 5, 2013

I think that I know what is going on: the resource bundle should include the name of aggregate target in the same way that it is done for the Pod targets (the static libraries). This is so because different aggregate targets might have a different platform and thus different file patterns for the resource bundle. As the name of the is not included two identically named targets end up being created which confuse Xcode. The relevant logic is here

Owner

fabiopelosin commented Sep 5, 2013

The name of the pod targets is computed here. So for the moment just accessing the target definition from the Pod target and including its name in the resource bundle should do the trick.

Contributor

blakewatters commented Sep 5, 2013

Works like a champ. Okay now I really think we are all set with resource bundles :-)

Coverage Status

Coverage remained the same when pulling bf46c12 on blakewatters:bugfix/1338-include-aggregate-name-in-resource-bundle-targets into 28b23c8 on CocoaPods:master.

Contributor

blakewatters commented Sep 5, 2013

There is a secondary change that needs to be made here -- the install_resource name is now incorrect because the resource bundle is built with a fully qualified name.

Blake Watters Generate namespaced resource bundle `install_resource` invocation whe…
…n generating the copy resources script. refs #1338
ad56f5a

Coverage Status

Coverage remained the same when pulling ad56f5a on blakewatters:bugfix/1338-include-aggregate-name-in-resource-bundle-targets into 28b23c8 on CocoaPods:master.

Contributor

blakewatters commented Sep 5, 2013

Meh there's another issue with this: now the resource bundles that are included into the built app have the fully qualified name. This means you can't reliably reference the resource bundle by name because it has the Pod name + Integration target included.

Contributor

blakewatters commented Sep 5, 2013

@irrationalfab and @alloy Need some guidance on where to take this next. I can see a few possible solutions:

  1. Set the name of the build product for the resource bundle to be the unqualified name (so WhateverResource.bundle instead of YourTarget-WhateverPod-WhateverResources.bundle). This would let you refer to them by name while still supporting multiple namespaced instances of the resource bundle in the pod project.
  2. Revert this change entirely and go to a world where you can only have one instance of a resource bundle. So just don't create multiple targets for all of the usages of a resource bundle in a given app.
  3. Keep my change as-is and state that end users cannot refer to resource bundles by name. This would probably mean that we'd need to emit an Info.plist in the resource bundles so that you can look them up by bundle identifier.
  4. Emit a constant into the app environment with the name of the bundle for use at run time.
Owner

fabiopelosin commented Sep 6, 2013

Ah sorry about that, in my quick reply of yesterday I forgot that I have done this by design. From the docs:

To provide different resources per platform namespaced bundles must be used.

Accordingly, I think that 2 would be the best solution. In this way libraries already using resource bundles would be easily supported and it would not require any logic specific to CP. In the case that the author wants to provide different resources per platform it can still do it like so:

spec.ios.resource_bundle = { 'MapBox-ios' => 'MapView/Map/Resources/ios/*.png' }
spec.osx.resource_bundle = { 'MapBox-osx' => 'MapView/Map/Resources/osx/*.png' }
Owner

fabiopelosin commented Sep 6, 2013

The logic to decide wether the resource bundle should be created could just check that the project.targets.map(&:name) array doesn't include the name of the bundle.

Contributor

blakewatters commented Sep 6, 2013

Thanks for the feedback. I will make the change this morning and force push
a clean PR

On Friday, September 6, 2013, Fabio Pelosin wrote:

The logic to decide wether the resource bundle should be created could
just check that the project.targets.map(&:name) array doesn't include the
name of the bundle.


Reply to this email directly or view it on GitHubhttps://github.com/CocoaPods/CocoaPods/pull/1338#issuecomment-23920385
.

To stay sane & productive, I don't live in e-mail. If you need to reach me
quickly, try this link: https://awayfind.com/blakewatters

@blakewatters blakewatters pushed a commit to blakewatters/CocoaPods that referenced this pull request Sep 6, 2013

Blake Watters Prevent creation of multiple Resource Bundle targets with the same na…
…me. fixes #1338
1ffaf7e

@blakewatters blakewatters pushed a commit to blakewatters/CocoaPods that referenced this pull request Sep 6, 2013

Blake Watters Rework resource bundle target creation so that a single common resour…
…ce bundle target is shared by all targets. fixes #1338

FIXUP
0159ba6

@blakewatters blakewatters pushed a commit to blakewatters/CocoaPods that referenced this pull request Sep 6, 2013

Blake Watters Rework resource bundle target creation so that a single common resour…
…ce bundle target is shared by all targets. fixes #1338
4863732

@jzapater jzapater pushed a commit to jzapater/CocoaPods that referenced this pull request Sep 17, 2013

@keith keith Merge pull request #1338 from luisrecuenco/LRImageManager
Add LRImageManager 0.1
be84a7f

@fabiopelosin fabiopelosin added a commit that referenced this pull request Sep 23, 2013

@fabiopelosin fabiopelosin Merge branch 'master' into pods-project-edit-feature
* master:
  [README] Use proper wording for ‘sponsors’.
  [Bundle] Update with Xcodeproj 0.11.1
  Release 0.25.0
  [Bundle] Bump Xcodeproj to 0.11.0
  [CHANGELOG] Add link to Xcode 5 PR.
  [CHANGELOG] Update for 0.25.0
  [Examples] Set ONLY_ACTIVE_ARCH to NO for iOS builds
  [Examples] Update to Xcode 5 default project settings to fix CI build.
  [Project] Make all Xcode 5 project validations green.
  [Analyzer] Do not warn about multiple `ARCHS` if there are none at all.
  Update integration spec fixtures.
  Update CHANGELOG for #1364
  Ensure resource bundles are copied to installation location on install actions. Fixes #1364
  [Generator::Xcconfig] Add support for XCTest
  [TargetInstaller] Set the architecture according to the value of the user targets
  Add support for Xcode 5
  [Xcodeproj] Update
  Fix specs
  Rework resource bundle target creation so that a single common resource bundle target is shared by all targets. fixes #1338

Conflicts:
	Gemfile.lock
	lib/cocoapods/installer.rb
	lib/cocoapods/installer/analyzer.rb
	lib/cocoapods/installer/target_installer.rb
	lib/cocoapods/installer/target_installer/pod_target_installer.rb
	lib/cocoapods/target.rb
	spec/cocoapods-integration-specs
	spec/unit/installer/target_installer_spec.rb
	spec/unit/installer_spec.rb
6d53f28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment