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

fix(icon-patcher): also set CFBundleIconName #11

Closed
wants to merge 1 commit into from

Conversation

sommestad
Copy link

Required by iOS 11. Assuming that CFBundleIconName is the same as the name of the iconset (to avoid exposing an additional required property).

@sommestad
Copy link
Author

Can't really understand why Travis fails:

An error occurred while generating the build script.

https://travis-ci.org/richardszalay/fastlane-plugin-act/builds/295187574

Both rake and rubocop pass locally (ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin16]).

Required by iOS 11. Assuming that CFBundleIconName is the same as the name of the iconset (to avoid exposing an additional required property).
@richardszalay
Copy link
Owner

Thanks for the PR. It looks like it will fail if you patch the same archive twice, since you always call Add :CFBundleIconName. Can you create a test that calls it twice in a row?

@sommestad
Copy link
Author

Thanks @richardszalay, will look at that.

Also, I'm actually not 100% confident that this is a good thing; we've experienced a heap of issues with getting icons to work with Xcode 9 / iOS 11 (app icon not being found).

So despite setting the icon name, we're still getting problems with the app not being found (= blank icon in simulator and not possible to upload). Internet is overloaded with similar issues, but none have worked for us.

Our set-up is generally:

  1. build once
  2. for each variation of our app; replace icons and re-sign app

This has worked great up until now, and we'd really not want to make individual builds per variation.

Have you seen this plugin work fine with iOS11 or experienced similar issues?

(Sorry for it being a bit out of context)

@richardszalay
Copy link
Owner

FWIW, your setup is the same that I've worked with in the past. I started with the build artefact being an IPA but in order to drop the "legacy" build APIs, I moved to using xcarchive instead. Unfortunately I'm not currently involved with iOS development and don't have access to a Mac so it's difficult for me to look into this further.

Based on past experience, it's possible that either the CFBundleIconName actually points into the asset library or the colour profile of the icons isn't supported. I'd recommend testing the following:

  1. Try removing CFBundleIconName entirely. It may force I to fallback to a pre-iOS11 method of handling icons
  2. Take a look at the colour profile of your icon images (this one is more of a longshot)

@richardszalay
Copy link
Owner

Ok, this is definitely an "icons in the asset catalog" thing. From the rejection notice that Apple gives:

Dear developer,
We have discovered one or more issues with your recent delivery for "". To process your delivery, the following issues must be corrected:
Missing Info.plist value - A value for the Info.plist key CFBundleIconName is missing in the bundle '<project_id>'. Apps that provide icons in the asset catalog must also provide this Info.plist key. For more information see http://help.apple.com/xcode/mac/current/#/dev10510b1f7.
Once these issues have been corrected, you can then redeliver the corrected binary.
Regards,
The App Store team

As for where we should go from here, I have a few possible short term workarounds:

  1. Remove CFBundleIconName from your bundle entirely and upload the icon into iTunes Connect. According to this StackOverflow answer there is support for publishing apps built by Xcode 8.3 so hopefully this triggers it.
  2. Include every possible app icon (as 1024x1024) in your application, and use act to patch CFBundleIconName with the correct value. This isn't ideal if you have a lot of variants as it will push out the size of your app.

Longer term, I think I'll need to add support for patching asset catalogs as part of act. I've only done very basic research on this in the past and I'm not sure how feasible it will be. If you'd like to assist (in either PR's or just research), it will make it happen faster.

@sommestad
Copy link
Author

(2) is actually what we're trying to do (with this PR). We're currently fine with shipping with several asset bundles and vary with the .plist. But it still doesn't work well enough, as the icon isn't found at all in such cases...

So even with several complete icon assets, the icon cannot be found (not upon upload, and not when building with e.g. Fabric).

We're trying to back away entirely from setting the app icon name now and see if that even works, and start over from there.

We might want to hold off with more work on this PR until it's verified.

@sommestad
Copy link
Author

Seems like removing CFBundleIconName doesn't work better. I assume that anything built with SDKs for iOS11/Xcode 9 would use the "newer" method...

@richardszalay
Copy link
Owner

Out of interest, are you patching an IPA or an xcarchive?

@richardszalay
Copy link
Owner

richardszalay commented Nov 8, 2017

I just noticed a comment from this stackoverflow question:

Adding only that image will not pass the iTunes Archive upload process so we need to add all the App Icons images of the following sizes:
20pt 1x, 2x, 3x
29pt 1x, 2x, 3x
40pt 1x, 2x, 3x
60pt 1x, 2x, 3x
76pt 1x, 2x
85.5pt 2x
1024pt 1x

So it looks like you'll need to set CFBundleIconName and ensure that the new asset it points to has the above variations. The old properties that act patches presumably still need to be set in order to support iOS < 11.

@thatjuan
Copy link

Did this PR end up working out?

@richardszalay
Copy link
Owner

Not that I know of. @sommestad ?

@sommestad sommestad closed this Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants