yonaskolb / XcodeGen

A Swift command line tool for generating your Xcode project
MIT License
7.07k stars 820 forks source link

2.20.0 Potential regression with Info.plist build phases when multiple targets share the same sources #1058

Open liamnichols opened 3 years ago

liamnichols commented 3 years ago

Report from @smaljaar - https://github.com/yonaskolb/XcodeGen/issues/1055#issuecomment-820209136 Report form - @honkmaster https://github.com/yonaskolb/XcodeGen/issues/1055#issuecomment-820238485

Please share your project spec's that reproduce the issue so that I can take a further look 🙏 If its possible for you to share a repo that also replicates the issue then that would be even better 👍

honkmaster commented 3 years ago

Here you go. I had to delete some parts. I think my problem is that I create the Info.plist of the test targets in a place that is included by the app itself (path: Tenants/${target_name}). This can be fixed, it think. However, this was not a problem before.

name: *redacted*

options:
  minimumXcodeGenVersion: 2.10.1

packages:
  *redacted*

targetTemplates:
  App:
    platform: iOS
    type: application
    deploymentTarget: "14.0"

    dependencies:
      *redacted*

    sources:
      - *redacted*
      - path: Tenants/${target_name}
        group: *redacted*

    configFiles:
      Debug: Tenants/${target_name}/XcodeConfigs/Debug.xcconfig
      Release: Tenants/${target_name}/XcodeConfigs/Release.xcconfig

    entitlements:
      *redacted*

    info:
      path: Tenants/${target_name}/Info.plist
      properties:
        *redacted*

    settings:
      *redacted*

  AppTest:
    platform: iOS
    type: bundle.unit-test
    deploymentTarget: "14.0"
    sources:
      - *redacted*AppTests
    dependencies:
      - target: ${targetApplication}
    info:
      path: Tenants/${targetApplication}/${target_name}/Info.plist

    settings:
      PRODUCT_BUNDLE_IDENTIFIER: ${bundleIdentifier}

  AppUITests:
    platform: iOS
    type: bundle.ui-testing
    deploymentTarget: "14.0"
    sources:
      - *redacted*AppUITests
    dependencies:
      - target: ${targetApplication}

    info:
      path: Tenants/${targetApplication}/${target_name}/Info.plist

    settings:
      PRODUCT_BUNDLE_IDENTIFIER: ${bundleIdentifier}

targets:
  SAMPLE:
    templates:
      - App
    templateAttributes:
      bundleIdentifier: de.*redacted*.*redacted*
  SAMPLETests:
    templates:
      - AppTests
    templateAttributes:
      targetApplication: SAMPLE
      bundleIdentifier: de.*redacted*.*redacted*.test
  SAMPLEUITests:
    templates:
      - AppUITests
    templateAttributes:
      targetApplication: SAMPLE
      bundleIdentifier: de.*redacted*.*redacted*.uitest

  ... MORE TARGETS FOLLOW
liamnichols commented 3 years ago

Thanks @honkmaster. So like you identified, this is a problem because your app target's sources also include sources from your test target, which includes the second Info.plist. This (by luck) wasn't a problem before XcodeGen 0.20.0 but let me first explain why the problem occurs now in a theoretical setup....

Lets say that we have the following project spec:

targets:
  App:
    type: application
    info:
      path: App/Info.plist
    sources:
      - App
      - AppTests
  AppTests:
    type: bundle.unit-test
    info:
      path: AppTests/Info.plist
    sources:
      - AppTests

This scenario seems to be more common than had originally anticipated, but essentially this spec describes that the App target includes sources from both the App and AppTests directory and at the same time, the AppTests target includes sources from just its AppTests directory.

If we take a look in the changelog for 0.20.0, it states the following:

  • Fixed files with names ending in Info.plist (such as GoogleServices-Info.plist) from being omitted from the Copy Resources build phase. Now, only the resolved info plist file for each specific target is omitted. yonaskolb/XcodeGen#1027

Because the App target includes sources for another target:

sources:
  - App
  - AppTests

XcodeGen 0.20.0 is actually finding two Info.plist files in App's sources:

  1. App/Info.plist
  2. AppTests/Info.plist

In XcodeGen 2.19.0 or earlier, any file named Info.plist had its build phase set to none by default, in 2.20.0, only the actual Info.plist file that is resolved to the INFOPLIST_FILE build settings for the generated target is set to none, others follow .plist default rules (which cause them to be treated as resources).

In your case, we see this build error:

Multiple commands produce '.../DerivedData/.../Build/Products/Debug-iphonesimulator/App.app/Info.plist': 1) Target 'App' (project 'Project') has copy command from './AppTests/Info.plist' to '.../DerivedData/.../Build/Products/Debug-iphonesimulator/App.app/Info.plist' 2) Target 'App' (project 'Project') has process command with output '.../DerivedData/.../Build/Products/Debug-iphonesimulator/App.app/Info.plist'

What is happening here is that AppTests/Info.plist is being copied as a resource into the build output by the Copy Bundle Resources phase (it copies to a top-level file named Info.plist and also the CopyInfoPlist build step is trying to copy App/Info.plist to the same location.

It wasn't very clear, but the fix is to update the sources definition to omit the first Info.plist as we don't want this plist included in the application target since its designed for the test target only.

sources:
  - App
  - path: AppTests
    excludes: [Info.plist]

Hopefully ☝️ should help explain what is going on, but the question about if this is the right thing to do is a different story 😬

Legacy behaviour before 0.20.0 was to treat every file named Info.plist as the special INFOPLIST_FILE that is processed separately to other resources however this was problematic for some setups which use different filenames (legacy Xcode projects defaulted the name to $(TARGET_NAME)-Info.plist) so some people considered the prior behaviour a bug as well.

We made the decision in #1027 to only treat the file that was being resolved as INFOPLIST_FILE for the target being generated to be given the special Info.plist file (and omit it from copy resources) as this seemed to be the fairest option. But like you've discovered, this breaks project specs like yours that were taking advantage of having files named exactly Info.plist.

When evaluating if this was the right thing to do, I came to the conclusion that we want to try and align as closely to Xcode's behaviour as possible. In that sense, if I dragged a series of files into Xcode and added them to my target, by default those files would end up being added to some kind of build phase, and in the case of property lists, they'd end up in the Copy Resources build phase regardless of their name. While I agree that the new behaviour is a bit inconvenient in this particular example, I also think that this is the most logical and predictable behaviour.

Of course though, I'd love to hear other options on this as maybe there are other things that I haven't considered.

Hope it helps!