-
Notifications
You must be signed in to change notification settings - Fork 466
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
Add capabilities for detecting app extensions and host targets #382
Add capabilities for detecting app extensions and host targets #382
Conversation
- Not all projects have BUNDLE_IDENTIFIER in build settings
Example usage can be seen in my CocoaPods branch here. |
# | ||
# @note watchOS 2 extensions are extensions of a watch target, not | ||
# an app target, whereas this is not the case for watchOS 1 | ||
def app_extension? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there's already a helper for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see there's extension_target_type?
, which I either didn't see or was newer than when I initially started this branch. That said, app_extension?
specifically doesn't consider watch os 2 targets (unsure about tv targets), since the intent is to use this flag to determine if it's okay to embed frameworks in the target, which it's not for "app" extensions but I believe is okay to do for watch os 2 targets (cc @neonichu). Maybe I just need a better name here. How about allows_embed_frameworks?
(and then I'll invert the condition)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That naming (and maybe this method) is more targeted toward how it's going to be used in CocoaPods. Maybe this should just be a method/helper on AggregateTarget
in CocoaPods that checks the user_target
's product_type
? The other methods in this PR could be refactored to use the existing helper extension_target_type?
. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, this feels like it belongs in CocoaPods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @segiddins
@benasher44 you're correct that watchOS 2 extensions need their frameworks embedded, because the binaries are build for a different architecture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okie doke. Thanks! I'll refactor and update soon.
Initial refactor is done. I'll update again once I've tested this with the CocoaPods branch :) |
Okay CocoaPod branch refactor is done, and it's all working! It even cleaned it up a bit 😃. Looks good! |
LGTM 👍 Just needs a changelog entry |
@benasher44 want me to merge this? |
Please! Now that I've worked out most of my test/dev setup issues with my CocoaPods branch that uses this, I'm hoping to have that one up for PR by the end of the week. |
This is step 1 towards fixing CocoaPods/CocoaPods#4203. This provides a few new bits of functionality: