Skip to content

Comments

Disallow the "watchOS" supported destination for multiplatform apps#1470

Merged
yonaskolb merged 4 commits intoyonaskolb:masterfrom
tatsuky:supported-destination-watchos-fix
May 17, 2024
Merged

Disallow the "watchOS" supported destination for multiplatform apps#1470
yonaskolb merged 4 commits intoyonaskolb:masterfrom
tatsuky:supported-destination-watchos-fix

Conversation

@tatsuky
Copy link
Contributor

@tatsuky tatsuky commented Apr 18, 2024

This PR addresses #1463, where the "Embed Watch Content" build phase isn't automatically generated when a watchOS app is created using the supportedDestinations configuration.

According to Apple's documentation:

iOS, iPadOS, macOS, visionOS, and tvOS apps can share a single target. watchOS apps remain in a separate target.

Xcode 15.3 is also not capable of creating multiplatform apps that contain the watchOS supported destination. Such the option does not show up on the UI:

The "watchOS" destination does not show up for multiplatform apps on Xcode 15.3

Provided Xcode doesn't support it now, I had XcodeGen error out when supportedDestinations for an application contains watchOS. I added a new validation error case because I think this is an exceptional case that needs a special consideration.

We can continue to create a watchOS app as an independent target by using the platform configuration as before. This PR does not affect the configurations of non-application targets.

Tests

You can use the following example specs to verify the changes.

A "success" case using platform 
name: MyApp
options:
  bundleIdPrefix: com.myapp
targets:
  UniversalApp:
    dependencies:
      - target: WatchApp
    supportedDestinations: [iOS, macOS]
    type: application
    sources:
      - Universal
    settings:
      base:
        PRODUCT_BUNDLE_IDENTIFIER: com.myapp.UniversalApp
    info:
      path: Universal/Info.plist
  WatchApp:
    platform: watchOS
    type: application
    sources:
      - Watch
    settings:
      base:
        PRODUCT_BUNDLE_IDENTIFIER: com.myapp.UniversalApp.WatchApp
    info:
      path: Watch/Info.plist
      properties:
        WKCompanionAppBundleIdentifier: com.myapp.UniversalApp
        WKApplication: true
An "error" case using supportedDestinations 
name: MyApp
options:
  bundleIdPrefix: com.myapp
targets:
  UniversalApp:
    dependencies:
      - target: WatchApp
    supportedDestinations: [iOS, macOS]
    type: application
    sources:
      - Universal
    settings:
      base:
        PRODUCT_BUNDLE_IDENTIFIER: com.myapp.UniversalApp
    info:
      path: Universal/Info.plist
  WatchApp:
    supportedDestinations: [watchOS]
    type: application
    sources:
      - Watch
    settings:
      base:
        PRODUCT_BUNDLE_IDENTIFIER: com.myapp.UniversalApp.WatchApp
    info:
      path: Watch/Info.plist
      properties:
        WKCompanionAppBundleIdentifier: com.myapp.UniversalApp
        WKApplication: true

@tatsuky
Copy link
Contributor Author

tatsuky commented Apr 18, 2024

Hi @yonaskolb, @bcardarella and @FelixLisczyk

I'm aware that this fix uses a little different approach than what was suggested in the issue discussion. Could you take a look at the change and let me know what you think? Thanks.

freddi-kit
freddi-kit reviewed Apr 18, 2024
View reviewed changes
Copy link
Collaborator

@freddi-kit freddi-kit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also update https://github.com/yonaskolb/XcodeGen/blob/master/CHANGELOG.md

giginet
giginet approved these changes Apr 18, 2024
View reviewed changes
Copy link
Collaborator

@giginet giginet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Please update CHANGELOG.md

@tatsuky
Copy link
Contributor Author

tatsuky commented Apr 18, 2024

@freddi-kit, @giginet
Just updated CHANGELOG - PTAL.

FelixLisczyk
FelixLisczyk approved these changes Apr 18, 2024
View reviewed changes
Copy link

@FelixLisczyk FelixLisczyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you! 👍

@freddi-kit freddi-kit requested a review from yonaskolb April 22, 2024 05:01
yonaskolb
yonaskolb approved these changes May 17, 2024
View reviewed changes
Copy link
Owner

@yonaskolb yonaskolb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thank @tatsuky!

@yonaskolb yonaskolb merged commit 274ce73 into yonaskolb:master May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@freddi-kit freddi-kit freddi-kit left review comments

@giginet giginet giginet approved these changes

@yonaskolb yonaskolb yonaskolb approved these changes

+1 more reviewer

@FelixLisczyk FelixLisczyk FelixLisczyk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tatsuky @giginet @yonaskolb @FelixLisczyk @freddi-kit