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

Substitute resource bundle's PRODUCT_BUNDLE_IDENTIFIER with a generated bundle ID #681

Closed
wants to merge 1 commit into from

Conversation

thii
Copy link
Member

@thii thii commented Dec 20, 2019

The Info.plist file in resource bundle targets generated by Xcode
contains a PRODUCT_BUNDLE_IDENTIFIER variable even though it doesn't seem
necessary. We generate a bundle ID for each target and substitute this
variable to that, so that we can build resource bundles coming from Xcode
without having to modify the Info.plist file.

Resolves #679.

@keith
Copy link
Member

keith commented Dec 20, 2019

IMO unless this value is something super stable for access with the NSBundle API it's nicer to have users set this by relying on other substituted variables like TARGET_NAME which is what we do

@thomasvl
Copy link
Member

This definitely feels wrong because it puts every bundle into the build.bazel namespace when used. The chances for collisions also seems like it could be pretty high.

@thii
Copy link
Member Author

thii commented Dec 20, 2019

How about adding a bundle_id attribute?

@thii thii force-pushed the resource-bundle-id branch 4 times, most recently from e942040 to 19e10e5 Compare December 26, 2019 03:33
bundle ID

The Info.plist file in resource bundle targets generated by Xcode
contains a PRODUCT_BUNDLE_IDENTIFIER variable even though it doesn't seem
necessary. We generate a bundle ID for each target and substitute this
variable to that, so that we can build resource bundles coming from Xcode
without having to modify the Info.plist file.
@thii
Copy link
Member Author

thii commented Dec 26, 2019

Adding a bundle_id didn't seem trivial as apple_resource_bundle is not a packaging rule and it will only get processed when you build its depender. I modified the bundle id generating logic to derive from the parent target's label.

Users will still be able to set this themselves by hard-coding the bundle id.

@acecilia
Copy link

acecilia commented May 6, 2020

Just hit an issue that brought me here :) A PRODUCT_BUNDLE_IDENTIFIER is mandatory in the Info.plist of a resource bundle if you want to use an asset catalog using UIImage?(named name: String, in bundle: Bundle?, compatibleWith traitCollection: UITraitCollection?). For some more context see: CocoaPods/CocoaPods#1264 and #765

@keith
Copy link
Member

keith commented Aug 10, 2020

PRODUCT_BUNDLE_IDENTIFIER is mandatory in the Info.plist

This is a bit misleading, what is required is the CFBundleIdentifier which you can create differently without having this specific replacement.

I'm super wary about a change like this because these values changing, or being inferred in an unexpected way will lead to runtime crashes. The current recommendation for how to deal with this is use a different replacement to create a stable identifier that you choose, such as by doing this in your plist:

<key>CFBundleIdentifier</key>
<string>com.lyft.$(PRODUCT_NAME)</string>

@segiddins
Copy link
Contributor

I agree that adding this default is potentially dangerous -- but I do think a change adding a bundle_id attr to apple_resource_bundle is something we want to do (even if it might not be a quick fix)

@thii
Copy link
Member Author

thii commented Aug 11, 2020

We pre-process our plists now so this isn't needed anymore.

@thii thii closed this Aug 11, 2020
@thii thii deleted the resource-bundle-id branch August 11, 2020 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants