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

Use carthage xcframeworks #1020

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Use carthage xcframeworks #1020

wants to merge 1 commit into from

Conversation

yonaskolb
Copy link
Owner

@yonaskolb yonaskolb commented Feb 15, 2021

This enables XcodeGen to look up the xcframeworks that Carthage 0.37 builds.
This PR is still a work in progress, but put up for reference.
Resolves #1006

For now it requires adding carthageXCFrameworks: true to the spec options

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.

Review Ongoing

@@ -677,7 +681,7 @@ public class PBXProjGenerator {

let targetSupportsDirectEmbed = !(target.platform.requiresSimulatorStripping &&
(target.type.isApp || target.type == .watch2Extension))
let directlyEmbedCarthage = target.directlyEmbedCarthageDependencies ?? targetSupportsDirectEmbed
let directlyEmbedCarthage = target.directlyEmbedCarthageDependencies ?? project.options.carthageXCFrameworks || targetSupportsDirectEmbed
Copy link
Collaborator

@freddi-kit freddi-kit May 2, 2021

Choose a reason for hiding this comment

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

↓ may be correct?

Suggested change
let directlyEmbedCarthage = target.directlyEmbedCarthageDependencies ?? project.options.carthageXCFrameworks || targetSupportsDirectEmbed
let directlyEmbedCarthage = target.directlyEmbedCarthageDependencies ?? (project.options.carthageXCFrameworks || targetSupportsDirectEmbed)

This makes one failed test passing.

Copy link
Collaborator

@kateinoigakukun kateinoigakukun left a comment

Choose a reason for hiding this comment

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

IMHO we don't need to handle carthage dependency specially any more because XCFramework removed carthage copy-framework step. We can add carthage dependency as a normal framework dependency like: framework: Carthage/Build/Alamofire.xcframework.

Yeah, but making things simple often leads some redundancy, so I understand the demand of shorthand.

But even if we provide a shorthand for framework: Carthage/Build/Alamofire.xcframework, we should separate API completely in the aspect of backward compatibility, I think.
IIUC, carthage build will produce XCFramework by default in the near future. So we may need to set carthageXCFrameworks true by default.

However, in that case, it will not be compatible with the old project.yml which uses carthage: Alamofire without explicit carthageXCFrameworks and doesn't use xcframeworks. If that project will update XcodeGen to the future version which enables carthageXCFrameworks by default, xcodeproj can't be generated without explicit carthageXCFrameworks: false.

On the other hand, if we won't make carthageXCFrameworks true by default, we have to write carthageXCFrameworks: true for the rest of our life.

So we need to re-think we should provide a shorthand or not, and the shorthand API should be separated from the current API or not.

@CraigSiemens
Copy link
Contributor

CraigSiemens commented Sep 13, 2021

I tried this out with our project and an issue I'm noticing is that since carthage supports getting precompiled frameworks, it's possible to get a mix of xcframeworks and frameworks. This flag seems to assume everything will be available as a xcframework.

As far as I can tell, there isn't a great way to know whether a dependency will be a xcframework when the project is generated. The only ways I can think of would be:

  • Use the currently added flag as a global default, allow each dependency to override that setting as needed with an isXCFramework: {true|false} or type option.
  • At project generation time, look in the Carthage/Build folder to see whether it's a xcframework or a framework
    • May slow down the project generation a bit
    • Would require that the carthage dependencies are build before the project is generated, which isn't needed currently.

@lsh-silpion
Copy link

Hi there, is there any progress here?

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.

How to declare a Carthage built XCFramework dependency?
5 participants