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

Add a new CopyFilesBuildPhase, "Embed ExtensionKit Extensions" #1230

Merged
merged 8 commits into from
Jul 21, 2022

Conversation

mtj0928
Copy link
Contributor

@mtj0928 mtj0928 commented Jul 17, 2022

close #1224

As @freddi-kit mentioned on #1224, extension kit extensions must be embedded as a separate copyFilesPhase from app extensions.
In this PR, I changed the followings related to extension kit extensions.

  • Added "Embed ExtensionKit Extensions" as a new CopyFilesBuildPhase 0371a6c
  • Fixed explicitFileType for extension kit extensions bf71a5d

FYI:
XcodeProj resolves the fileType from the file extension only, but extension kit extension and app extension have different fileType even though they have the same file extension.
So, I added productType as a new argument to fileType(path), to resolve the fileType from the combination of the file extension and the product type.

@mtj0928 mtj0928 changed the title Added a new CopyFilesBuildPhase, "Embed ExtensionKit Extensions" Add a new CopyFilesBuildPhase, "Embed ExtensionKit Extensions" Jul 17, 2022
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.

Perfect from my sight

Comment on lines 739 to 743
} else if dependencyTarget.type.isExtension && dependencyTarget.type != .extensionKitExtension {
// embed app extension
extensions.append(embedFile)
} else if dependencyTarget.type == .extensionKitExtension {
extensionKitExtensions.append(embedFile)
Copy link
Collaborator

@freddi-kit freddi-kit Jul 18, 2022

Choose a reason for hiding this comment

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

It might be better to write like below to consider the other addtional future cases of dependencyTarget.type when dependencyTarget.type.isExtension is true

else if dependencyTarget.type.isExtension {
     // we may add other properties == 
     if dependencyTarget.type == .extensionKitExtension {
        extensionKitExtensions.append(embedFile)
     } else {
         extensions.append(embedFile)
     }
}

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.

Great work @mtj0928!
Would it be possible for you to add an example extension in Test/Fixtures/TestProject? That's a project that tries to have every single XcodeGen feature in it, so that any diffs in the generation are tracked.

@mtj0928
Copy link
Contributor Author

mtj0928 commented Jul 18, 2022

Thank you for your reviews!

@freddi-kit
I fixed the if statement structure based on your comment.

@yonaskolb
I added a new example extension, but the new product type com.apple.product-type.extensionkit-extension is supported on only Xcode 14 or later.
Thus, CI failed on Xcode 12.5.1 and Xcode 13.0.
Do you have any ideas to pass the check?

@yonaskolb
Copy link
Owner

To fix CI properly, we'd need a seperate Xcode14 fixture project that only runs in a seperate action with Xcode 14 in Github Actions.
If you feel comfortable doing that, that would be fantastic. Otherwise we can wait until Xcode 14 launches before merging this.

@alessionossa
Copy link

alessionossa commented Jul 20, 2022

Otherwise we can wait until Xcode 14 launches before merging this.

This would be a blow to the teams that rely on XcodeGen and would like to implement ExtensionKit Extensions like AppIntents in time for iOS 16 launch. 😥

@yonaskolb
Copy link
Owner

Otherwise we can wait until Xcode 14 launches before merging this.

This would be a blow to the teams that rely on XcodeGen and would like to implement ExtensionKit Extensions like AppIntents in time for iOS 16 launch. 😥

@alessionossa makes sense, I can appreciate that. @mtj0928 if the CI changes are too much work, I'm happy with you just commenting out the changes to the project.yml in this PR for now. Up to you 👍

@mtj0928
Copy link
Contributor Author

mtj0928 commented Jul 20, 2022

@yonaskolb
I'd like to try adding a new fixture project which runs with Xcode 14, but even if I do it, I think we cannot run it because GitHub Actions don't support Xcode 14 yet.
Thus, in this PR, I will comment out the changes in the project.yml.

I'd like to try creating the new fixture project for Xcode 14 as another issue.

@mtj0928
Copy link
Contributor Author

mtj0928 commented Jul 20, 2022

I've created a new issue #1232.
(Please modify it if needed.)

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.

I think it is ready for merging 👍

@freddi-kit
Copy link
Collaborator

@yonaskolb I think we can merge it in master

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.

Great, thanks @mtj0928! 🙏

@yonaskolb yonaskolb merged commit da8aad0 into yonaskolb:master Jul 21, 2022
@yonaskolb
Copy link
Owner

Released in 2.31.0

@mtj0928 mtj0928 deleted the support-extension-kit branch July 25, 2022 02:14
mcfans added a commit to iftechio/XcodeGen that referenced this pull request Feb 22, 2023
commit bb02e01
Author: takeshi-1000 <[email protected]>
Date:   Thu Dec 8 05:15:58 2022 +0900

    Fixed logic related to order of elements in localization directory Path array

commit bad37c9
Author: takeshi-1000 <[email protected]>
Date:   Thu Dec 8 05:09:44 2022 +0900

    Revert "Fixed some logic in PBXVariantGroup."

    This reverts commit ea402c8.

commit ea402c8
Author: takeshi-1000 <[email protected]>
Date:   Wed Dec 7 09:23:02 2022 +0900

    Fixed some logic in PBXVariantGroup.
    Try to make the logic simpler so that an error does not occur under the Linux environment.

commit b9359fb
Author: takeshi-1000 <[email protected]>
Date:   Tue Dec 6 19:00:20 2022 +0900

    fix fixtures test

commit ca25eca
Author: takeshi-1000 <[email protected]>
Date:   Tue Dec 6 12:33:23 2022 +0900

    fix compilation error in ci enviroment

commit a40341f
Author: takeshi-1000 <[email protected]>
Date:   Mon Dec 5 21:05:49 2022 +0900

    add test

commit cb28c35
Author: takeshi-1000 <[email protected]>
Date:   Mon Dec 5 21:05:41 2022 +0900

    update SourceGenerator, PBXProjGenerator, SourceType

    ## SourceGenerator
    - adapt SourceGenerator to TargetSourceFilterable
    - refactor variant group logic
    - add logic where you can add target membership to another target

    ## PBXProjGenerator
    - apply. new PBXProjGenerator

    ## SourceType
    - add new sourceType

commit 8a33b01
Author: takeshi-1000 <[email protected]>
Date:   Mon Dec 5 21:05:08 2022 +0900

    add PBXVariantGroupGenerator, TargetSourceFilterable

commit e7f7537
Author: Mathieu Olivari <[email protected]>
Date:   Thu Nov 3 01:05:46 2022 -0700

    Fix includes related issues and improve their performances (yonaskolb#1275)

    * Fix recursive include path when relativePath is not set

    If relativePath is not set on a particular include, the first level of
    include will currently work, but starting at the second level of
    iteration, the computed include path will fail as relativePath will be
    appended over and over onto the filePath. We're fixing that recursion
    problem here and adding the corresponding tests to make sure it doesn't
    happen again.

    * Include projectRoot in include paths

    The projectRoot setting (when specified) is currently ignored when
    computing the include paths. We're fixing that in that commit.

    * Use memoization during recursive SpecFiles creation

    SpecFile objects are created by recursive through includes. On a large
    project with programatically generated SpecFile, it is not rare to have
    hundreds of SpecFiles, creating a large web of include dependencies.
    In such a case, it is not rare either for a particular SpecFile to be
    included by multiple other SpecFiles. When that happens, XcodeGen
    currently creates a SpecFile object every time a SpecFile gets included,
    which can lead to an exponential growth of includes.

    I have seen hundreds of files being turned into hundred of thousands of
    SpecFile object creations, which leads to an impractical XcodeGen run of
    tens of minutes.

    This change adds memoization during SpecFile recursion, in order to
    reuse the previously created SpecFiles, if available, instead of
    re-creating them.

    * Update CHANGELOG.md

    Add the following changes to the changelog:
    * b97bdc4 - Use memoization during recursive SpecFiles creation
    * a6b96ad - Include projectRoot in include paths
    * 557b074 - Fix recursive include path when relativePath is not set

commit 3e9fd04
Author: Roland <[email protected]>
Date:   Wed Nov 2 07:42:22 2022 +0100

    allow multiple spec files to be provided to XcodeGen (yonaskolb#1270)

    * allow spec to be a comma separated list of specs instead of one

    * update readme and --spec command documentation

    * update Changelog

    * print project name

commit 87d7c7e
Author: Yonas Kolb <[email protected]>
Date:   Sat Oct 1 20:50:18 2022 +1000

    Update CHANGELOG.md

commit 435c194
Author: SofteqDG <[email protected]>
Date:   Sat Oct 1 11:23:09 2022 +0300

    Extend possible paths for SettingsPresets (yonaskolb#1135)

    * Search for presets in Bundle.main.resourcesPath dir (if exists)

commit ed5ec74
Author: Craig Siemens <[email protected]>
Date:   Wed Sep 28 22:08:37 2022 -0600

    Added scheme generation for aggregate targets (yonaskolb#1250)

    * Updated SchemeGenerator to generate schemes for all projectTargets.

    * Added changelog entry

commit 6f33172
Author: Bobby Sudekum <[email protected]>
Date:   Fri Sep 9 01:43:39 2022 -0700

    Add enableGPUFrameCaptureMode to Scheme (yonaskolb#1251)

commit ebf70f1
Author: Yonas Kolb <[email protected]>
Date:   Fri Aug 19 00:53:34 2022 +1000

    Update to 2.32.0

commit 594c67f
Author: freddi(Yuki Aki) <[email protected]>
Date:   Fri Aug 12 15:21:43 2022 +0900

    Add `enable` option for `include` to enable optional including for addtional spec (yonaskolb#1242)

    * add new option enable for include of spec

    * fix to see the environment variable when parsing include

    * add test for include with environment variable

    * fix how to parse boolean value

    * add spec about enable for include

    * add Change Log

    * fix the number of PR in changelog

    * fix include test to make more clear

    * fix test to focus enable option more

    * fix english error

    * fix to expand variable only one time

    * add new test case by setting environment object as NO

commit e9295f1
Author: Steven Sheldon <[email protected]>
Date:   Thu Aug 11 05:45:06 2022 -0700

    Fix profile action to not run frameworks (yonaskolb#1245)

    * Fix profile action to not run frameworks

    * Add PR number to changelog

    * Update CHANGELOG.md

    Co-authored-by: Yonas Kolb <[email protected]>

commit ac525a4
Author: Shinolr <[email protected]>
Date:   Tue Aug 9 22:32:33 2022 +0800

    remove redundant bracket (yonaskolb#1243)

commit 34f50d6
Author: Isaac Halvorson <[email protected]>
Date:   Mon Aug 1 17:27:14 2022 -0500

    Correct name of package in example yaml (yonaskolb#1240)

    Hey there, I just noticed that one of the example yaml snippets had the wrong package name specified.

commit ff552f3
Author: antonsergeev88 <[email protected]>
Date:   Sun Jul 31 11:33:20 2022 +0300

    Handle mlmodelc as a single unit (yonaskolb#1237)

    * Handle mlmodelc as a single unit

    * Add mlmodelc support in changelog

commit de2a537
Author: Yonas Kolb <[email protected]>
Date:   Sun Jul 24 16:10:15 2022 +1000

    Update to 2.31.0

commit 24572da
Author: Aleksei Sapitskii <[email protected]>
Date:   Sun Jul 24 09:08:33 2022 +0300

    Added duplicate dependencies validation (yonaskolb#1234)

    **Reason**
     - More strict validation of added dependencies

    **Contents**
     - Added changelog entry
     - Added check for duplicates in validation stage
     - Added test

commit da8aad0
Author: matsuji <[email protected]>
Date:   Thu Jul 21 20:25:34 2022 +0900

    Add a new CopyFilesBuildPhase, "Embed ExtensionKit Extensions" (yonaskolb#1230)

    * Embed ExtensionKit Extensions

    * Fix explicitFileType for extensionKit

    * Update ChangeLog

    * Fix if statement structure

    * Add a new example extension to Tests/Fixtures/TestProject/

    * Update Tests/Fixtures/TestProject/Project.xcodeproj

    * Comment out example for extension kit extension in Tests/Fixtures/TestProject/

    * Update Tests/Fixtures/TestProject/Project.xcodeproj

commit c1d5c65
Author: Yonas Kolb <[email protected]>
Date:   Sat Jul 16 16:57:26 2022 +1000

    Update to 2.30.0

commit c082bc0
Author: Aleksei Sapitskii <[email protected]>
Date:   Sat Jul 16 09:46:42 2022 +0300

    Fix XcodeGen building after XcodeProj update to 8.8.0 (yonaskolb#1228)

    * Fix XcodeGen building after XcodeProj update to 8.8.0

    **Reason**
    - XcodeProj has been updated and has API breaking changes

    **Content**
    - Added new enum case handling in `Linkage`
    - Renamed the enum case name for `XCWorkspaceDataFileRef.init`

    * add new product type to docs

    * update changelog

    Co-authored-by: Yonas Kolb <[email protected]>

commit 19817f3
Author: Luca Bartoletti <[email protected]>
Date:   Sat Jul 16 07:46:31 2022 +0100

    Fix `watchapp2-container` product name (yonaskolb#1219)

    The correct name is `application.watchapp2-container`
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

Successfully merging this pull request may close these issues.

Extension Kit extension support for Xcode14
4 participants