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 support for Swift Package Manager only projects #1945

Open
wants to merge 11 commits into
base: master
from

Conversation

Projects
None yet
@devxoul
Copy link
Contributor

commented May 20, 2017

Background

Swift Package Manager doesn't have Xcode dependency so the library authors can distribute their code without Xcode project files. Already there are many projects that don't have Xcode project files in their Git repository (e.g. ReactorKit). One of the most important design goal of Carthage is to reduce the responsibility of Xcode:

Ultimately, we created Carthage because we wanted the simplest tool possible—a dependency manager that gets the job done without taking over the responsibility of Xcode, and without creating extra work for framework authors.

This PR makes Carthage to have less dependency of Xcode by supporting SPM-only libraries.

Summary

  • Run swift package generate-xcodeproj before each build processes if the dependency doesn't have both .xcodeproject and .xcworkspace butPackage.swift.
  • Add a fixture for testing SPM-only library.

Related Issues

@devxoul devxoul force-pushed the devxoul:spm branch from ee14b72 to 7ec5e54 May 27, 2017

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2017

@ikesyo, could you please review this PR when you're available? Thanks!

PS: I'm not sure why the test is failing. It fails just after rebasing master.

@mdiep

This comment has been minimized.

Copy link
Member

commented May 27, 2017

Last time I talked to the SwiftPM folks, it was suggested that we not rely on this functionality. I'll try to find someone at WWDC to talk to about this.

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2017

@mdiep, I'm curious why we should not rely on that function. I think unstable solution is better than no solution. Even we can offer this functionality as a beta (or with a explicit flag) and change the implementation future. I hope I could distribute my code-only libs to Carthage without Xcode :)

@mdiep

This comment has been minimized.

Copy link
Member

commented May 28, 2017

I think unstable solution is better than no solution.

I don't think that's true.

An unstable solution would likely:

  1. Greatly increase the support load for Carthage. Which would decrease the time available for development.

  2. Remove the incentive for project creators to add proper Carthage support—which will hurt everyone when/if this function breaks

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2017

You might be true if the swift package generate-xcodeproj is definitely unstable, but I can't understand why it is unstable :)

Anyway, do you have any suggestion for adding Carthage support to existing SPM-only project?

@mdiep

This comment has been minimized.

Copy link
Member

commented May 28, 2017

I think generating the xcodeproj is a fine way to start Carthage support. Generate, share the scheme, and check it in, and you should be good to go.

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2017

@mdiep You mean the Xcode project from Xcode.app not by swift package generate-xcodeproj?

@mdiep

This comment has been minimized.

Copy link
Member

commented May 28, 2017

No, I mean generating with swift package generate-xcodeproj, sharing the scheme, and then checking it in.

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2017

Hmm, I'm confused because you just said that swift package generate-xcodeproj is unstable so we should not rely on it. 🤔🤔🤔 Is there any difference between doing that on Carthage and each lib? Maybe the schemes?

@mdiep

This comment has been minimized.

Copy link
Member

commented May 28, 2017

It's not guaranteed to work for all projects. But if it's a fine starting place. If it works out of the box for your project, great! This acts as a QA step for projects—they have to confirm that the generated project works.

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2017

Thanks for your kind reply. I hope we could find a better solution 👍 Feel free to close or keep this PR open.

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented Jun 13, 2017

@mdiep, was there any updates on WWDC? :)

@mdiep

This comment has been minimized.

Copy link
Member

commented Jun 16, 2017

After talking to someone from the SwiftPM team, I'm tentatively on board with adding support for this. It will require some testing. I'll try to play with this soon, but it will need to be a lower priority than some of my other OSS maintenance.

@mdiep
Copy link
Member

left a comment

This will need some documentation updates as well.

public func generateSwiftPackageManagerXcodeProjectIfAvailable(forDependencyAt dependencyPath: String) -> SignalProducer<(), NoError> {
let files = (try? FileManager.default.contentsOfDirectory(atPath: dependencyPath)) ?? []

let hasXcodeProjectFile = files.lazy.filter { $0.hasSuffix(".xcodeproj") || $0.hasSuffix(".xcworkspace") }.first != nil

This comment has been minimized.

Copy link
@mdiep

mdiep Jun 16, 2017

Member

Instead of doing foo.lazy.filter { ... }.first, you should be able to do foo.first { ... }.

let hasPackageFile = files.lazy.filter { $0 == "Package.swift" }.first != nil
guard hasPackageFile else { return .init(value: ()) }

let task = Task("/usr/bin/swift", arguments: [ "package", "generate-xcodeproj" ], workingDirectoryPath: dependencyPath)

This comment has been minimized.

Copy link
@mdiep

mdiep Jun 16, 2017

Member

This should definitely have a project event that gets printed to the console.

This comment has been minimized.

Copy link
@ikesyo

ikesyo Jun 16, 2017

Member

Generating an Xcode project to the dependency directory makes it dirty so that should be avoided. You should generate a project to a temporary directory.

This comment has been minimized.

Copy link
@yhkaplan

yhkaplan Oct 13, 2018

@ikesyo For a temporary directory, would $TMPDIR be preferred?

.flatten(.concat)
.ignoreTaskData()
.on(value: { (project, scheme) in
NSLog("Building scheme \"\(scheme)\" in \(project)")

This comment has been minimized.

Copy link
@mdiep

mdiep Jun 16, 2017

Member

Can you remove this log from the test?

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented Jun 16, 2017

@mdiep, Sounds cool. I found that SPM generated project can cause a problem if the library has dependencies. I'll update this PR soon with more specific tests. Thanks!

public func generateSwiftPackageManagerXcodeProjectIfAvailable(forDependencyAt dependencyPath: String) -> SignalProducer<(), NoError> {
let files = (try? FileManager.default.contentsOfDirectory(atPath: dependencyPath)) ?? []

let hasXcodeProjectFile = files.lazy.filter { $0.hasSuffix(".xcodeproj") || $0.hasSuffix(".xcworkspace") }.first != nil

This comment has been minimized.

Copy link
@ikesyo

ikesyo Jun 16, 2017

Member

This will not be sufficient––Carthage allows having Xcode projects or workspaces in any subdirectories so you should need to check the dependency directory recursively.

let hasPackageFile = files.lazy.filter { $0 == "Package.swift" }.first != nil
guard hasPackageFile else { return .init(value: ()) }

let task = Task("/usr/bin/swift", arguments: [ "package", "generate-xcodeproj" ], workingDirectoryPath: dependencyPath)

This comment has been minimized.

Copy link
@ikesyo

ikesyo Jun 16, 2017

Member

Generating an Xcode project to the dependency directory makes it dirty so that should be avoided. You should generate a project to a temporary directory.

@ikesyo

This comment has been minimized.

Copy link
Member

commented Jun 16, 2017

Another viewpoint: Deployment target settings of a generated Xcode project are not set for iOS, tvOS, and watchOS (macOS is set to 10.10). That means that the actual deployment target determined for each platforms will be its latest SDK (iOS 10.3, tvOS 10.2, watchOS 3.2 in Xcode 8.3.x). So actually the generated project should be useless without any modification (are your application targeting iOS 10.3 only? 🙂).

I'm still reluctant to add this feature.

@mdiep

This comment has been minimized.

Copy link
Member

commented Jun 16, 2017

Another viewpoint: Deployment target settings of a generated Xcode project are not set for iOS, tvOS, and watchOS (macOS is set to 10.10). That means that the actual deployment target determined for each platforms will be its latest SDK (iOS 10.3, tvOS 10.2, watchOS 3.2 in Xcode 8.3.x). So actually the generated project should be useless without any modification (are your application targeting iOS 10.3 only? 🙂).

When talking to the SwiftPM folks, this did seem like a potential issue. I'm not super knowledgable about it, but they mentioned the idea of passing an .xcconfig file when building to set those flags.

I guess any SwiftPM project wouldn't depend on the SDK, so Carthage could hardcode the oldest viable option?

I'm still reluctant to add this feature.

I think you're more knowledgable about this than I am, so definitely speak up if you think something is a bad idea.

@ikesyo

This comment has been minimized.

Copy link
Member

commented Jun 16, 2017

When talking to the SwiftPM folks, this did seem like a potential issue. I'm not super knowledgable about it, but they mentioned the idea of passing an .xcconfig file when building to set those flags.

I guess any SwiftPM project wouldn't depend on the SDK, so Carthage could hardcode the oldest viable option?

Sounds reasonable to me. We can pass --xcconfig-overrides option to swift package generate-xcodeproj command which will be better than overriding on building.

@devxoul

This comment has been minimized.

Copy link
Contributor Author

commented Jun 16, 2017

We can set deployment version by providing a xcconfig and bundle version by modifying an Info.plist. Currently I'm archiving SPM generated xcodeproj as following:

I think I can do similar with Swift but I'm not sure this would be the best solution 🤔

@yonaskolb yonaskolb referenced this pull request Aug 9, 2017

Closed

Support Carthage #41

@olbrichj

This comment has been minimized.

Copy link

commented Aug 9, 2017

Is there any update on this one?

@mdiep

This comment has been minimized.

Copy link
Member

commented Aug 9, 2017

There's some unaddressed feedback above.

@mdiep

This comment has been minimized.

Copy link
Member

commented Jan 22, 2019

I'm interested in finishing this

Awesome!

Though it’s not completely clear to me what is left to do, just getting deployment targets set?

As far as I know, yes.

@yhkaplan

This comment has been minimized.

Copy link

commented Jan 24, 2019

If we wait for Swift 5.0, then minimum deployment target settings can be written directly in the Package.swift file: https://github.com/apple/swift-evolution/blob/master/proposals/0236-package-manager-platform-deployment-settings.md

This should remove the need to read in xcconfig files, making things a whole lot easier!

Xcode 10.2 beta release notes:

Packages can now customize the minimum deployment target setting for Apple platforms when using the Swift 5 Package.swift tools-version. Building a package emits an error if any of the package dependencies of the package specify a minimum deployment target greater than the package’s own minimum deployment target. (SE-0236)

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2019

How feasible is only allowing this feature if you have Xcode 10.2? Like, I think it’s fine.

@yhkaplan

This comment has been minimized.

Copy link

commented Jan 25, 2019

Good point. I think on Carthage’s side, it would involve checking the Package.swift file’s version (if that property is available), otherwise spitting out a warning about targets not being supported in Swift versions lower than 5.0, but still building.

The reason I say warning and not error is that some people who only need the latest minimum target version supported use Carthage and SPM today, so it might be good to not break this behavior.

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2019

The version can be checked. The top of Package.swift specifies the version it supports, it would specify: swift-tools-version: 5.0, eg: https://github.com/mxcl/Path.swift/blob/fixes/Package%40swift-5.0.swift#L1

Notably, packages can have multiple Package.swifts like in the above example it is named Package@swift-5.0.swift.

I think these are reasonable criterions though. Any package author that wants to support Carthage with only a swift-package would have no reason to object to providing a swift-5-tools SwiftPM manifest.

@mdiep

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

If we wait for Swift 5.0, then minimum deployment target settings can be written directly in the Package.swift file

If you specify an iOS deployment target and use swift package genarate-xcodeproj, does the generated dynamic framework target have all the right settings for use on iOS?

Any package author that wants to support Carthage with only a swift-package would have no reason to object to providing a swift-5-tools SwiftPM manifest

Agreed

@yhkaplan

This comment has been minimized.

Copy link

commented Jan 25, 2019

If you specify an iOS deployment target and use swift package genarate-xcodeproj, does the generated dynamic framework target have all the right settings for use on iOS?

Let me download Xcode 10.2 beta and find out 😉

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2019

It can be built for iOS but it won’t have anything special set. It won’t have app-extension only specified for example.

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2019

The generated xcodeproj has all platforms available as build targets, I spoke to @aciidb0mb3r about it and he said that the platform specification in Package.swift files doesn't actually restrict the platforms it merely allows deployment targets to be set. So this idea is bunk, we still need xcconfig files otherwise the generated project will almost certainly fail to build since it will pretend that it can build for eg. tvOS and then fail if it (likely) doesn't support tvOS actually.

Edit: well the deployment targets ARE set correctly. So maybe the trick is to only build for the platforms that the target project sets? Not fool-proof, but perhaps sufficient?

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2019

Hey everyone, so I've been thinking about this a lot and here is my current plan:

  • Only support this if the toolchain is a Swift 5 toolchain
  • Only support this if the package provides a Swift 5.0 or higher tool-version manifest
  • Generate the xcodeproj
  • Only build for platforms explicitly declared in the manifest

I will begin work on this, this weekend, so if there are concerns with my approach, please speak up!

Thanks for the help 🙏

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2019

As an update here I have submitted a PR to SwiftPM to get the platform information from the dump-package command. I hope it will be released with Xcode 10.2, but if not we can figure out another way.

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2019

Update: the dump-package changes are finally in Xcode 10.2-beta4 so I will attempt to finish the patch.

@rynecheow

This comment has been minimized.

Copy link

commented Mar 28, 2019

May I ask how are we doing with this PR?

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

I am unlikely to find time for this in near future, here’s my progress though:

https://github.com/mxcl/Carthage/tree/spm

@JonasVautherin

This comment has been minimized.

Copy link

commented Mar 28, 2019

@mxcl: Would you mind elaborating a bit on what would need to be done on your PR?

@mxcl

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

I believe the remaining part is parsing dump-package and setting deployment targets, nothing else.

@ollieatkinson

This comment has been minimized.

Copy link

commented Apr 16, 2019

From what I can see these are the following settings which would need to be added to the generated Xcodeproj:

IPHONEOS_DEPLOYMENT_TARGET = 8.0
SUPPORTED_PLATFORMS = iphonesimulator iphoneos
SDKROOT = iphoneos
DEFINES_MODULE = YES
MODULEMAP_FILE = $(PACKAGE_NAME).xcodeproj/GeneratedModuleMap/$(TARGET_NAME)/module.modulemap

Steps:

  1. Run swift package dump-package, grab the following information:
"platforms" : [
    {
      "platformName" : "ios",
      "version" : "8.0"
    }
],
  1. Create an xcconfig file inside of the temporary with the values above, transformed appropriately
  2. When generating the xcodeproj pass --xcconfig-overrides path/to/tmp/config.xcconfig

If the project does not specify platform information, I'm guessing we should just fail out - but some steer on that from the core contributors would be good.

If the above sounds reasonable, I can give it a go - but I will hang fire until I get some feedback.

Edit: Have I found a further complication?

Unless I'm missing something, I think we will also need to generate an xcodeproj for each platform we want to build for because only one target is generated by SwiftPM.

@pixlwave pixlwave referenced this pull request May 5, 2019

Closed

Xcode projects? #7

@thedavidharris

This comment has been minimized.

Copy link

commented Jun 27, 2019

Edit: Have I found a further complication?

Unless I'm missing something, I think we will also need to generate an xcodeproj for each platform we want to build for because only one target is generated by SwiftPM.

I don't think so, SUPPORTED PLATFORMS and the OS flags should be enough. https://davedelong.com/blog/2018/11/15/building-a-crossplatform-framework/ describes a way of doing this, then when the platforms are targeted it'll pick up the right version of everything.

@ollieatkinson @mxcl any continued work on this? I may be able to help pick it up, as we currently are writing a bunch of SPM modules internally that need Carthage integration for some of our current apps and support both is a bit of a pain for the interim, so would be interested in helping out on this

@thedavidharris

This comment has been minimized.

Copy link

commented Jul 10, 2019

So, looks like there's an edge case that won't make this workable for us, and that's when an SPM project has dependencies that are also Carthage dependencies by the consuming project. Probably won't take this on anymore myself, as I don't think there's any solution to that problem :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.