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

Current resource bundle target setup doesn't play well with image assets containing universal images #2292

Closed
nikolaykasyanov opened this Issue Jul 7, 2014 · 17 comments

Comments

Projects
None yet
10 participants
@nikolaykasyanov
Contributor

nikolaykasyanov commented Jul 7, 2014

I have an asset catalog with universal image set (I'm using different images for iPhone and iPad) included in pod resource bundle like this:

s.resource_bundle = {'LibName' => ['LibName/Media.xcassets']}

And here's the problem: build setting "Targeted Device Family" of LibName resource bundle target is set to iPhone, so I got no iPad images in resulting LibName.bundle.

I think that target device family should be inferred from the application target device family or user should be able to specify target device family in podfile so podspecs could inherit this setting.

@nikolaykasyanov

This comment has been minimized.

Show comment
Hide comment
@nikolaykasyanov

nikolaykasyanov Jul 7, 2014

Contributor

As far as I understand, the fix could be trivial, we just should take spec consumer target device family here: https://github.com/CocoaPods/CocoaPods/blob/master/lib/cocoapods/installer/target_installer/pod_target_installer.rb#L68

I'd be happy to provide a fix but I need some guidance.

Contributor

nikolaykasyanov commented Jul 7, 2014

As far as I understand, the fix could be trivial, we just should take spec consumer target device family here: https://github.com/CocoaPods/CocoaPods/blob/master/lib/cocoapods/installer/target_installer/pod_target_installer.rb#L68

I'd be happy to provide a fix but I need some guidance.

@segiddins segiddins added the t2:defect label Jul 7, 2014

@alloy

This comment has been minimized.

Show comment
Hide comment
@alloy

alloy Jul 8, 2014

Member

Yeah that sounds about right, although I don’t believe we actually collect from the user’s project for what device(s) it builds atm, so the Analyser would have to be extended to fetch that info.

Member

alloy commented Jul 8, 2014

Yeah that sounds about right, although I don’t believe we actually collect from the user’s project for what device(s) it builds atm, so the Analyser would have to be extended to fetch that info.

@nikolaykasyanov

This comment has been minimized.

Show comment
Hide comment
@nikolaykasyanov

nikolaykasyanov Jul 9, 2014

Contributor

@alloy thanks for the info.

I just realized that there's not so much sense in supporting asset catalogs inside resource bundles after everyone moves to deployment target >= 7.0, because it seems impossible to load anything from the Assets.car located inside resource bundle.

Contributor

nikolaykasyanov commented Jul 9, 2014

@alloy thanks for the info.

I just realized that there's not so much sense in supporting asset catalogs inside resource bundles after everyone moves to deployment target >= 7.0, because it seems impossible to load anything from the Assets.car located inside resource bundle.

@fabiopelosin

This comment has been minimized.

Show comment
Hide comment
@fabiopelosin

fabiopelosin Jul 21, 2014

Member

So should this issue be closed?

Member

fabiopelosin commented Jul 21, 2014

So should this issue be closed?

@nikolaykasyanov

This comment has been minimized.

Show comment
Hide comment
@nikolaykasyanov

nikolaykasyanov Jul 21, 2014

Contributor

@irrationalfab if so, resource bundles with xcassets should be deprecated as well.

Contributor

nikolaykasyanov commented Jul 21, 2014

@irrationalfab if so, resource bundles with xcassets should be deprecated as well.

@fabiopelosin

This comment has been minimized.

Show comment
Hide comment
@fabiopelosin

fabiopelosin Jul 21, 2014

Member

I just realized that there's not so much sense in supporting asset catalogs inside resource bundles after everyone moves to deployment target >= 7.0, because it seems impossible to load anything from the Assets.car located inside resource bundle.

@Corristo, as the discussion is not completely clear to me, can you clarify this sentence?

Member

fabiopelosin commented Jul 21, 2014

I just realized that there's not so much sense in supporting asset catalogs inside resource bundles after everyone moves to deployment target >= 7.0, because it seems impossible to load anything from the Assets.car located inside resource bundle.

@Corristo, as the discussion is not completely clear to me, can you clarify this sentence?

@nikolaykasyanov

This comment has been minimized.

Show comment
Hide comment
@nikolaykasyanov

nikolaykasyanov Jul 21, 2014

Contributor

@irrationalfab actool compiles *.xcassets contents to Assets.car blob if project deployment target is 7.0 or higher.

It seems that only Assets.car from application bundle root is considered by iOS when calling +imageNamed:, so it makes no sense to create resource bundles with Assets.car inside, you won't be able to load anything from it.

Contributor

nikolaykasyanov commented Jul 21, 2014

@irrationalfab actool compiles *.xcassets contents to Assets.car blob if project deployment target is 7.0 or higher.

It seems that only Assets.car from application bundle root is considered by iOS when calling +imageNamed:, so it makes no sense to create resource bundles with Assets.car inside, you won't be able to load anything from it.

@nikolaykasyanov

This comment has been minimized.

Show comment
Hide comment
@nikolaykasyanov

nikolaykasyanov Oct 5, 2014

Contributor

Update to my previous comment:
Looks like it's possible to access images compiled to Assets.car in a bundle using method introduced in iOS 8 +[UIImage imageNamed:inBundle:compatibleWithTraitCollection:]

Contributor

nikolaykasyanov commented Oct 5, 2014

Update to my previous comment:
Looks like it's possible to access images compiled to Assets.car in a bundle using method introduced in iOS 8 +[UIImage imageNamed:inBundle:compatibleWithTraitCollection:]

@onekiloparsec

This comment has been minimized.

Show comment
Hide comment
@onekiloparsec

onekiloparsec Apr 15, 2015

Hi. Not sure what's the status of this, but the problem still exists.I am using 0.36.0 right now, and I have libraries with resources bundles containing not images but xibs. Once the Pods project is integrated into the client project, the "Target Device Family" setting of the Pods root project is blank (i.e. not iPhone, not iPad, and not iPhone/iPad). Upon building, resources get created for iPhone only. Selecting "iPhone/iPad" in the build setting of the Pods project fixes the problem, at least manually. But it has to be done every time we update the pods of the project. A thing that occurs many times when you are using the pods your are currently developing for an app. If I could provide help, I would be happy to do so.

onekiloparsec commented Apr 15, 2015

Hi. Not sure what's the status of this, but the problem still exists.I am using 0.36.0 right now, and I have libraries with resources bundles containing not images but xibs. Once the Pods project is integrated into the client project, the "Target Device Family" setting of the Pods root project is blank (i.e. not iPhone, not iPad, and not iPhone/iPad). Upon building, resources get created for iPhone only. Selecting "iPhone/iPad" in the build setting of the Pods project fixes the problem, at least manually. But it has to be done every time we update the pods of the project. A thing that occurs many times when you are using the pods your are currently developing for an app. If I could provide help, I would be happy to do so.

@levigroker

This comment has been minimized.

Show comment
Hide comment
@levigroker

levigroker Apr 23, 2015

This is also happening for me with a storyboard. I'm using Xcode 6.3.1 targeting iOS 7.0 and up. My pod has a storyboard in it which recently does not show up for the iPad (but did previous to 6.3 and 8.3).

levigroker commented Apr 23, 2015

This is also happening for me with a storyboard. I'm using Xcode 6.3.1 targeting iOS 7.0 and up. My pod has a storyboard in it which recently does not show up for the iPad (but did previous to 6.3 and 8.3).

@levigroker

This comment has been minimized.

Show comment
Hide comment
@levigroker

levigroker Apr 23, 2015

Well, a workaround is to manually specify the desired Targeted Device Families in the post_install hook:

post_install do |installer_representation|
  # Directly set the Targeted Device Family
  # See https://github.com/CocoaPods/CocoaPods/issues/2292
  installer_representation.project.build_configurations.each do |config|
    config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2'
  end
end

See Apple's XcodeBuildSettingRef for specifics about TARGETED_DEVICE_FAMILY.

Essentially:

1: iPhone/iPod touch.
2: iPad.

It would be preferable for CocoaPods to inherit the targeted device family of the hosting Project's Targets, if possible.

Levi

levigroker commented Apr 23, 2015

Well, a workaround is to manually specify the desired Targeted Device Families in the post_install hook:

post_install do |installer_representation|
  # Directly set the Targeted Device Family
  # See https://github.com/CocoaPods/CocoaPods/issues/2292
  installer_representation.project.build_configurations.each do |config|
    config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2'
  end
end

See Apple's XcodeBuildSettingRef for specifics about TARGETED_DEVICE_FAMILY.

Essentially:

1: iPhone/iPod touch.
2: iPad.

It would be preferable for CocoaPods to inherit the targeted device family of the hosting Project's Targets, if possible.

Levi

@knox

This comment has been minimized.

Show comment
Hide comment
@knox

knox Jun 3, 2015

Based on levi's idea i am using the following post_install hook

post_install do |installer|
  installer.project.targets.each do |target|
    next unless target.product_type == 'com.apple.product-type.bundle'
    target.build_configurations.each do |config|
      #config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2' # iPhone, iPad
      config.build_settings['TARGETED_DEVICE_FAMILY'] = '2'
    end
  end
end

knox commented Jun 3, 2015

Based on levi's idea i am using the following post_install hook

post_install do |installer|
  installer.project.targets.each do |target|
    next unless target.product_type == 'com.apple.product-type.bundle'
    target.build_configurations.each do |config|
      #config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2' # iPhone, iPad
      config.build_settings['TARGETED_DEVICE_FAMILY'] = '2'
    end
  end
end
@brion

This comment has been minimized.

Show comment
Hide comment
@brion

brion Jul 13, 2015

Note that the above fails on 0.38 beta -- must use installer.pods_project instead of installer.project.

This works for me, but is a bit verbose:

post_install do |installer|
  if installer.respond_to?(:project)
    project = installer.project
  else
    project = installer.pods_project
  end
  project.targets.each do |target|
    if target.product_reference.name == 'OGVKitResources.bundle' then
      target.build_configurations.each do |config|
        config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2' # iPhone, iPad
      end
    end
  end
end

brion commented Jul 13, 2015

Note that the above fails on 0.38 beta -- must use installer.pods_project instead of installer.project.

This works for me, but is a bit verbose:

post_install do |installer|
  if installer.respond_to?(:project)
    project = installer.project
  else
    project = installer.pods_project
  end
  project.targets.each do |target|
    if target.product_reference.name == 'OGVKitResources.bundle' then
      target.build_configurations.each do |config|
        config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2' # iPhone, iPad
      end
    end
  end
end
@Grubas7

This comment has been minimized.

Show comment
Hide comment
@Grubas7

Grubas7 Aug 5, 2015

@brion Thanks a lot! Your changes helps if somebody wants backward compatibility.

Grubas7 commented Aug 5, 2015

@brion Thanks a lot! Your changes helps if somebody wants backward compatibility.

@levigroker

This comment has been minimized.

Show comment
Hide comment
@levigroker

levigroker Aug 10, 2015

@brion Thanks for the update. Seems to work well with Cocoapods 0.38.2. Cheers.

For the record, my updated post_install hook looks like this:

post_install do |installer_representation|

  # Grab the `project` object from the installer (cocoapods < 0.38 use `project`, cocoapods >= 0.38 use pods_project)
  # See https://github.com/CocoaPods/CocoaPods/issues/2292
  if installer_representation.respond_to?(:project)
    project = installer_representation.project
  else
    project = installer_representation.pods_project
  end

  # Directly set the Targeted Device Family
  # See https://github.com/CocoaPods/CocoaPods/issues/2292
  project.build_configurations.each do |config|
    config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2'
  end

end

levigroker commented Aug 10, 2015

@brion Thanks for the update. Seems to work well with Cocoapods 0.38.2. Cheers.

For the record, my updated post_install hook looks like this:

post_install do |installer_representation|

  # Grab the `project` object from the installer (cocoapods < 0.38 use `project`, cocoapods >= 0.38 use pods_project)
  # See https://github.com/CocoaPods/CocoaPods/issues/2292
  if installer_representation.respond_to?(:project)
    project = installer_representation.project
  else
    project = installer_representation.pods_project
  end

  # Directly set the Targeted Device Family
  # See https://github.com/CocoaPods/CocoaPods/issues/2292
  project.build_configurations.each do |config|
    config.build_settings['TARGETED_DEVICE_FAMILY'] = '1,2'
  end

end
@neonichu

This comment has been minimized.

Show comment
Hide comment
@neonichu

neonichu May 2, 2016

Member

This issue has been inactive for a long time and will be closed because we want to move forward with a clean slate after our 1.0 release. If you believe this is still relevant, please retest with the 1.0 release candidate and comment with a reproducible project in order for this to become actionable again. Thanks!

Member

neonichu commented May 2, 2016

This issue has been inactive for a long time and will be closed because we want to move forward with a clean slate after our 1.0 release. If you believe this is still relevant, please retest with the 1.0 release candidate and comment with a reproducible project in order for this to become actionable again. Thanks!

@neonichu neonichu closed this May 2, 2016

@brion

This comment has been minimized.

Show comment
Hide comment
@brion

brion May 2, 2016

This appears to be resolved for me in the use_frameworks era.

brion commented May 2, 2016

This appears to be resolved for me in the use_frameworks era.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment