Support for multiple deployment targets with xcode 14 #1336

amatig · 2023-03-16T00:36:37Z

Hi, I am Giovanni Amati.
I am new, I have never contributed to this project but because in my company we are using XcodeGen, and we are starting a new app, it was good to have this new feature to support iOS and AppleTV in one single target.

This fixes #1333 , opened by me then I decided to try to solve it.

What I have done:

Introduced an optional supportedDestinations array in target to create the right combination of build settings for multi platform destinations.
Introduced a new enum supportedDestination (iOS, tvOS, macOS, macCatalyst and visionOS)
Introduced auto has platform value that you can use only with supported destinations
The definition of the platform for supported destinations is optional, it fallbacks to auto if it is not defined.
Introduced an optional destinationFilters array for sources and dependencies to filter things for platforms
Introduced an optional inferDestinationFiltersByPath boolean, this is a convenience filter that helps you to filter sources (and resources) if their pathes match these patterns **/<supportedDestination>/* or *_<supportedDestination>.swift
if destinationFilters and inferDestinationFiltersByPath are both defined, inferDestinationFiltersByPath is ignored
if supportedDestinations is defined and platform is an array, an error is raised because we are using to ways to use multi platforms
Added checks to improve the spec validation
Added visionOS support
Added unit tests
Updated docs / changelog

Ex. of spec

targets:
  MyApp:
    type: application
    supportedDestinations: [iOS, tvOS]
    sources:
      - path: ../Project/GLEAP/Sources
        inferDestinationFiltersByPath: true
      - path: ../Project/GLEAP/Storyboards
        destinationFilters: [iOS]
    dependencies:
      - package: Yams
        product: Yams
        destinationFilters: [tvOS]
      - package: Lottie
        product: Lottie

Everything works with every bundle so it's possible to do the same for UI tests, 1 single target and if you have different file you can filter them by platform. It is very useful.

This spec becomes:

Many thanks!

amatig · 2023-08-25T02:55:39Z

Hi, I did a couple of things and now I am investigating about "auto" for baseSDK and visionOS.

About "auto" I have a couple of things to ask. At the moment, a lot of logics are based on the platform that is mandatory. If I add a new case auto in the enum we are going to lose presets that we are applying now (Platform and Product_Platform) so the result will be a project a little bite more empty where the user needs to configure more build settings stuff in the spec because we can not help the generation.

Is this an issue? Or isn't it better to leave to he user the decision to define one "baseSDK" so we can give him few presets and then he needs to customised the extra platforms things if he wants a multi-platform solution?

Or both? You can put what ever you want iOS, tvOS or auto (or nothing that fallbacks to auto). The user will lose presets only in auto.

What do you think? @yonaskolb

I have an idea.
Because the destinations have a priority defined in the enum of SupportedDestination, I can assign to the target the platform preset and product_preset of the destination with higher priority. So the user can have something if the platform is "auto" or undefined.

I pushed the changes have a look, if this approach is ok I will add unit tests and add stuff in the doc.

I have also removed "isResolved" as a property of the Target (the new field that I have added), I am adding that information only in the dictionary to help me to check if the spec is correct. For this reason I moved the error of that check in SpecParsingError.

Added visionOS support and fixed doc.

Everything is done, I have also added more unit tests and checks to prevent issue in the spec and help the user.

yonaskolb

Nice work @amatig

yonaskolb · 2023-09-10T12:19:01Z

Docs/ProjectSpec.md

+- [ ] **removeHeaders**: **Bool** - Whether the `removeHeadersOnCopy` setting is applied when embedding the framework. Defaults to true.
+- [ ] **weak**: **Bool** - Whether the `Weak` setting is applied when linking the framework. Defaults to false.
+- [ ] **platformFilter**: **String** - This field is specific to Mac Catalyst. It corresponds to the "Platforms" dropdown in the Frameworks & Libraries section of Target settings in Xcode. Available options are: **iOS**, **macOS** and **all**. Defaults is **all**.
+- [ ] **platformFilters**: **[[Supported Destinations](#supported-destinations)]** - List of supported platform destinations this dependency should filter to. Defaults to all supported destinations.


Reading this perhaps platformFilters should be called destinationFilters to match the new name. Also differentiates it from the existing platformFilter. Thoughts?

ah yes, I can do this. This is ok, as input I can change it but when the value arrives to XcodeProj we must use this field name because that project accept that.

yonaskolb · 2023-09-10T12:31:41Z

Sources/ProjectSpec/Target.swift

+
+                    /// This value is set to help us to check, in Target init, that there are no conflicts in the definition of the platforms. We want to ensure that the user didn't define, at the same time,
+                    /// the new Xcode 14 supported destinations and the XcodeGen generation of Multiple Platform Targets (when you define the platform field as an array).
+                    platformTarget["isResolved"] = true


Maybe this could have a more specific name like isMultiPlatformTarget?

yonaskolb · 2023-09-10T12:50:06Z

Sources/ProjectSpec/Target.swift

+        if supportedDestinations?.contains(.macCatalyst) == true,
+           supportedDestinations?.contains(.iOS) == false {
+
+            supportedDestinations?.append(.iOS)


This is a little bit too "magic" in this part of the pipeline for my tastes, but happy to let it in. I can tell you're trying to provide a nice user experience and can appreciate that.

If you don't like this I could raise an error instead

yonaskolb · 2023-09-10T12:57:29Z

Docs/ProjectSpec.md

@@ -483,7 +515,9 @@ A source can be provided via a string (the path) or an object of the form:
 - [ ] **compilerFlags**: **[String]** or **String** - A list of compilerFlags to add to files under this specific path provided as a list or a space delimited string. Defaults to empty.
 - [ ] **excludes**: **[String]** - A list of [global patterns](https://en.wikipedia.org/wiki/Glob_(programming)) representing the files to exclude. These rules are relative to `path` and _not the directory where `project.yml` resides_. XcodeGen uses Bash 4's Glob behaviors where globstar (**) is enabled.
 - [ ] **includes**: **[String]** - A list of global patterns in the same format as `excludes` representing the files to include. These rules are relative to `path` and _not the directory where `project.yml` resides_. If **excludes** is present and file conflicts with **includes**, **excludes** will override the **includes** behavior.
- [ ] **createIntermediateGroups**: **Bool** - This overrides the value in [Options](#options)
+- [ ] **platformFilters**: **[[Supported Destinations](#supported-destinations)]** - List of supported platform destinations the files should filter to. Defaults to all supported destinations.
+- [ ] **inferPlatformFiltersByPath**: **Bool** - This is a convenience filter that helps you to filter the files if their paths match these patterns `**/<supportedDestination>/*` or `*_<supportedDestination>.swift`. Note, if you use `platformFilters` this flag will be ignored.


Same comment here as below about a possible rename from filters to destinations for these 2 props?

yonaskolb · 2023-09-10T13:03:37Z

Sources/XcodeGenKit/SourceGenerator.swift

+        } else if inferPlatformFiltersByPath == true {
+            for supportedDestination in SupportedDestination.allCases {
+                let regex1 = try? NSRegularExpression(pattern: "\\/\(supportedDestination)\\/", options: .caseInsensitive)
+                let regex2 = try? NSRegularExpression(pattern: "\\_\(supportedDestination)\\.swift$", options: .caseInsensitive)


I'm slightly worried about the performance implications of this, but it's opt in

I know but this thing is very helpful, you can use this instead of define folders one by one for each specific filter.

amatig · 2023-09-25T09:23:19Z

Any news?

FelixLisczyk · 2023-10-13T08:11:22Z

This feature would significantly reduce the number of framework targets in my project. 👍 When can we merge it?

amatig · 2023-10-14T16:20:12Z

This feature would significantly reduce the number of framework targets in my project. 👍 When can we merge it?

Hope soon, I made the last changes after the review I am waiting for @yonaskolb

yonaskolb

Thank you for your patience @amatig, and all your great work here. I think this is good to merge now 👍

FelixLisczyk · 2023-11-09T10:53:10Z

@amatig I've noticed that the supportedDestination enum doesn't contain the watchOS platform. Is there any reason for this? I can add it manually in Xcode.

Giovanni Amati added 3 commits March 15, 2023 16:24

platformFilters on Dependecies

39123d4

platformFilters on sources

bc179e8

fixed current unit tests

f6f7cf3

amatig changed the title ~~Support for multiple deployment targets with xcode 14~~ [Feature] Support for multiple deployment targets with xcode 14 Mar 16, 2023

Giovanni Amati added 26 commits March 16, 2023 14:04

renamed enum to SupportedPlatforms

755ba8c

supportedPlatforms field for target

0569732

errors

9e44b3f

renamed errors

ec0cc4f

inferPlatformFiltersByPath flag

7ba8817

changed priority to generate filter

4a4b94d

fixed parsing

803feca

fixed init

940ce0e

unit test supportedPlatforms

dc80b2e

unit tests for errors

d764618

fixing build settings and unit tests

3c27fdf

added new settingsPresets

3318a9b

new check errors and unit tests

b36ddf9

case insensitive match

f0daaa5

fixed skipping cross platform target

30217cd

json decode

696b470

unit tests inferPlatformFiltersByPath and platformFilters for sources

77bef23

mocked files

b8c938b

fixing unit tests

459c1bf

first test on dependecies

93aeacb

unit tests completed

529a3fc

fixed unit tests

856fa82

changelog

8f31810

doc changes

be7012b

doc changes

959e13e

doc changes

c2d99d2

renamed errors

2e6ebff

Giovanni Amati added 2 commits August 25, 2023 12:23

fixed presets

ac795ff

remamed index to priority

b2705ba

amatig requested a review from yonaskolb August 25, 2023 04:26

Giovanni Amati added 12 commits August 25, 2023 16:17

small comments

4e54235

removed isResolved in target and fixed error check

fc1086f

added unit tests

a371ef8

fixed doc

c555f9d

fixed doc

83ff2d1

fixed doc

b284d26

fixed doc

64c9cfc

fixed test app

fe0558e

add visionOS and more error check and testing

8976ec6

fixed supported destinations priority and tests

142dabe

fixed doc

a68883f

merged main

5baba62

yonaskolb reviewed Sep 10, 2023

View reviewed changes

Giovanni Amati added 4 commits September 11, 2023 13:14

solved conflicts

f6a3914

solved conflicts

5af696c

fixed conflicts

03fd40d

renamed everything

7882cf7

amatig requested a review from yonaskolb September 11, 2023 13:26

yonaskolb approved these changes Oct 31, 2023

View reviewed changes

yonaskolb merged commit 97d36fd into yonaskolb:master Oct 31, 2023
2 checks passed

tatsuky mentioned this pull request Jan 23, 2024

Add watchOS as a supported destination #1438

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for multiple deployment targets with xcode 14 #1336

Support for multiple deployment targets with xcode 14 #1336

amatig commented Mar 16, 2023 •

edited

Loading

amatig commented Aug 25, 2023 •

edited

Loading

yonaskolb left a comment

yonaskolb Sep 10, 2023

amatig Sep 11, 2023

amatig Sep 11, 2023

yonaskolb Sep 10, 2023

amatig Sep 11, 2023

amatig Sep 11, 2023

yonaskolb Sep 10, 2023

amatig Sep 11, 2023 •

edited

Loading

yonaskolb Sep 10, 2023

amatig Sep 11, 2023

amatig Sep 11, 2023

yonaskolb Sep 10, 2023

amatig Sep 11, 2023 •

edited

Loading

amatig commented Sep 25, 2023

FelixLisczyk commented Oct 13, 2023

amatig commented Oct 14, 2023

yonaskolb left a comment

FelixLisczyk commented Nov 9, 2023 •

edited

Loading

Support for multiple deployment targets with xcode 14 #1336

Support for multiple deployment targets with xcode 14 #1336

Conversation

amatig commented Mar 16, 2023 • edited Loading

amatig commented Aug 25, 2023 • edited Loading

yonaskolb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amatig Sep 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amatig Sep 11, 2023 • edited Loading

Choose a reason for hiding this comment

amatig commented Sep 25, 2023

FelixLisczyk commented Oct 13, 2023

amatig commented Oct 14, 2023

yonaskolb left a comment

Choose a reason for hiding this comment

FelixLisczyk commented Nov 9, 2023 • edited Loading

amatig commented Mar 16, 2023 •

edited

Loading

amatig commented Aug 25, 2023 •

edited

Loading

amatig Sep 11, 2023 •

edited

Loading

amatig Sep 11, 2023 •

edited

Loading

FelixLisczyk commented Nov 9, 2023 •

edited

Loading