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

Using 'supportedDestinations' with watchOS app doesn't generate an 'Embed Watch Content' build phase #1463

Open
FelixLisczyk opened this issue Mar 23, 2024 · 12 comments

Comments

@FelixLisczyk
Copy link

I'm currently in the process of migrating my project specs from platform to supportedDestinations. According to the project spec, this should work for all target types:

Note that the definition of supported destinations can be applied to every type of bundle making everything more easy to manage (app targets, unit tests, UI tests etc).

I've noticed when I configure my watch app target with supportedDestinations, XcodeGen no longer generates an 'Embed Watch Content' build phase for the iOS app. Here is an example:

name: MyApp
options:
  bundleIdPrefix: com.example
targets:
  UniversalApp:
    dependencies:
      - target: WatchApp
    supportedDestinations: [iOS, macOS]
    type: application
    sources:
      - Universal
  WatchApp:
    supportedDestinations: [watchOS]
    type: application
    sources:
      - Watch

Is this a bug or intended behavior? Should I continue to use platform for my watchOS targets? Thank you!

@bcardarella
Copy link

I am also looking for an answer on this. The doc's aren't clear if watchOS is supported for supportedDestinations or not. It's not included in the list of supported destrinations but if watchOS isn't supported I was curious as to why and what our options are for including it?

@bcardarella
Copy link

Perhaps it is fixed with #1438

@yonaskolb
Copy link
Owner

yonaskolb commented Apr 7, 2024

@atsuky do you have any info on this? Did your PR #1438 get this working?
I've just added watchOS as a supported destination in the docs, as that PR missed that out

Also pinging @amatig as the original implementor of the supported destinations work, in case he has any insights

@tatsuky
Copy link
Contributor

tatsuky commented Apr 9, 2024

Hi, thank you for the doc update. I'll take a look at the issue.

@tatsuky
Copy link
Contributor

tatsuky commented Apr 9, 2024

Looks like the following part is causing the issue.

} else if dependencyTarget.type.isApp && dependencyTarget.platform == .watchOS {
copyWatchReferences.append(embedFile)

platform is auto when supportedDestinations is set. I think it's why the "Embed Watch Content" phase gets skipped even if the destinations include watchOS.

I think we can fix it by also checking if supportedDestinations == [.watchOS] here. It's not supportedDestinations.contains(.watchOS) so that we can rule out the apps with e.g., supportedDestinations: [iOS, watchOS] just in case (I find this type of combination (watchOS + other destinations) uncommon for an application, though).

Let me know what you all think. I'll create a PR if this approach looks good.

@bcardarella
Copy link

@tatsuky does this mean that it would skip watchos?

@tatsuky
Copy link
Contributor

tatsuky commented Apr 9, 2024

@bcardarella By "it" do you mean:

so that we can rule out the apps with e.g., supportedDestinations: [iOS, watchOS] just in case

If so - it will just skip adding the "Embed Watch Content" phase for the target. The supportedDestinations will be kept as initially configured.


After reading Apple's Configuring a multiplatform app - apparently the watchOS destination for the multiplatform apps is not supported at the moment. The destination is also unavailable (not showing up) for a multiplatform app on Xcode 15.3.

Destination options for multiplatform apps on Xcode 15.3

Given this information & on second thought, we should maybe disallow the watchOS supportedDestination for an application instead of add the logic fix I mentioned in my previous comment?

@bcardarella
Copy link

@tatsuky perhaps emmit a warning or error if it is included and point to this issue so understand why it isn't (yet) supported. Looking at the issue tracker and it seems to be a common question

@yonaskolb
Copy link
Owner

@tatsuky, I support the change to add the check for supportedDestinations == [.watchOS] when embedding watch content, as well as add an error for configuring a destination to be watchOS AND iOS, as well as highlighting that in the documentation.
If you still want to create the PR that would be fantastic

@tatsuky
Copy link
Contributor

tatsuky commented Apr 10, 2024

@yonaskolb Just to confirm, can we still add the changes you suggested, given Xcode 15.3 seemingly doesn't allow us to create multiplatform apps that contain the watchOS destination? (at least not on the UI, that is)

Alternatively, I think we could consider it as an invalid configuration and have xcodegen error out like @bcardarella suggested.

@yonaskolb
Copy link
Owner

@tatsuky yes agree. That's what I meant by an error when watchOS and iOS are in the same destinations array 👍

@tatsuky
Copy link
Contributor

tatsuky commented Apr 11, 2024

OK, I'll work on the fix and open a PR one of these days

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

No branches or pull requests

4 participants