Swift package support. #1083
Swift package support. #1083
Conversation
|
This PR reveals several issues about imports and about structure of project.
@class DDLogMessage;
@class DDLoggerInformation;
@protocol DDLogger;
@protocol DDLogFormatter;
@interface DDLog : NSObject
@protocol DDLogger <NSObject>
@protocol DDLogFormatter <NSObject>
@protocol DDRegisteredDynamicLogging
@interface DDLogMessage : NSObject <NSCopying>
@interface DDAbstractLogger : NSObject <DDLogger>
@interface DDLoggerInformation : NSObjectAs we can see, it is a blob of various definitions and interfaces. I suggest to cleanup DDLog (and move protocols and Data structures to appropriate headers) in separate PR. |
lolgear
requested review from
ffried,
diederich,
bpoplauschi,
hhanesand and
sushichop
Invalid copyright: Package.swift Generated by 🚫 Danger |
|
Also, it seems that only relative paths are permitted. In case of
|
…ot handle old framework import.
Codecov Report
@@ Coverage Diff @@
## master #1083 +/- ##
==========================================
- Coverage 38.43% 37.95% -0.49%
==========================================
Files 27 27
Lines 3944 3944
==========================================
- Hits 1516 1497 -19
- Misses 2428 2447 +19
Continue to review full report at Codecov.
|
|
@CocoaLumberjack/collaborators |
Package.swift
Outdated
| // Products define the executables and libraries produced by a package, and make them visible to other packages. | ||
| .library( | ||
| name: "CocoaLumberjack", | ||
| type: .dynamic, |
There was a problem hiding this comment.
CocoaLumberjack also supports static linking I think. Could remove this and let clients decide?
There was a problem hiding this comment.
If Swift Package manager supports static linking, then, we should add static library also.
There was a problem hiding this comment.
/// A library's product can either be statically or dynamically linked. It
/// is recommended to not declare the type of library explicitly to let the
/// Swift Package Manager choose between static or dynamic linking depending
/// on the consumer of the package.
https://github.com/apple/swift-package-manager/blob/master/Documentation/PackageDescription.md
Removing it should be fine?
|
@ffried |
|
@lolgear It's less about the discoverability (although I find |
|
@lolgear I'd suggest to put the .xcodeproj also in |
|
@lolgear I've also just verified this with one of the bigger projects I'm using CocoaLumberjack in. Switched the dependency to your branch and it still builds just fine! I've also tried the symlink solution above - works just fine, too. |
|
Great works! But... I could not confirm the following steps work fine(I could not build using Xcode and I needed to add AppKit.framework for CocoaLumberjack.framework target to build it using Xcode...😢). Package.swift // swift-tools-version:5.0
// The swift-tools-version declares the minimum version of Swift required to build this package.
import PackageDescription
let package = Package(
name: "Dependencies",
products: [
// Products define the executables and libraries produced by a package, and make them visible to other packages.
.library(
name: "Dependencies",
targets: ["Dependencies"]),
],
dependencies: [
// Dependencies declare other packages that this package depends on.
.package(url: "git@github.com:lolgear/CocoaLumberjack.git", .branch("swift_package")),
],
targets: [
// Targets are the basic building blocks of a package. A target can define a module or a test suite.
// Targets can depend on other targets in this package, and on products in packages which this package depends on.
.target(
name: "Dependencies",
dependencies: ["CocoaLumberjackSwift"]),
.testTarget(
name: "DependenciesTests",
dependencies: ["Dependencies"]),
]
)And I drag-and-dropped the above Dependencies.xcodeproj to some macOS app project. As a whole, am I wrong with how to use SwiftPM? |
|
@sushichop Wow, thanks for testing this. I totally missed that one. I only tested building CocoaLumberjack directly... 😨 Just to clarify: Building your own package on the terminal (using We could work around it by conditionally linking AppKit (conditional, since we cannot link it on non-macOS targets). Can you test Xcode 11? If it's fixed there, I'd simply declare SPM support as of Xcode 11. I'd say drag-and-dropping the project is not really a valid SPM case. You'd be creating another package and declaring a dependency there. |
|
I tested on Xcode 11 beta 4 by adding https://github.com/lolgear/CocoaLumberjack.git, branch: swift_package as a new Swift Package dependency. The imports work fine for me. |
|
@dehlen So the generated Xcode project builds fine, too? |
|
When I right click the package -> Show in Finder -> and open the Lumberjack.xcodeproj in Xcode 11 beta 4 the CocoaLumberjack and CocoaLumberjackSwift Target build succeeds. |
|
You guys had a very good conversation here, including testing. |
|
@bpoplauschi |
|
@lolgear I'd say for the initial support we don't need any new samples. SPM is pretty straightforward. I'd only include it in the README for now and add samples as necessary. Apart from that I'd really like to move the .xcodeproj into the Xcode folder and (for now) provide a symlink) to prevent confusion with the SPM generated xcodeproj - which should also be added in the .gitignore as |
|
Hi guys! I created Package.swift from Xcode menu. Package.swift is here. // swift-tools-version:5.1
// The swift-tools-version declares the minimum version of Swift required to build this package.
import PackageDescription
let package = Package(
name: "MyLibrary",
platforms: [.iOS(.v8)],
products: [
.library(
name: "MyLibrary",
targets: ["MyLibrary"]),
],
dependencies: [
.package(url: "https://github.com/lolgear/CocoaLumberjack.git", .branch("swift_package"))
],
targets: [
.target(
name: "MyLibrary",
dependencies: ["CocoaLumberjackSwift"]),
.testTarget(
name: "MyLibraryTests",
dependencies: ["MyLibrary"]),
]
)On the other hand, So I'm glad that we can say CocoaLumberjack will completely support SwiftPM with Xcode 11(Sep. this year?)🎉 Can I add README to this PR or create another PR for it? |
|
@suchichop About PR.. Also, if you like to complete final steps... Do you dare to move Xcode project to subdirectory? I don’t know if Apple have plans to release migration tool which allows to move Xcode project and repair relative paths of files in it. |
|
@sushichop @lolgear IMHO it's not our job to provide an intro on "how to use SPM with Xcode". That's Apple's job. If you don't know how to use SPM, first learn that, then come back and include CocoaLumberjack as dependency. Most SPM projects (including Apple's own Swift NIO) simply list their dependency-line and I think this is fine. E.g. we also don't provide instructions on how to install Carthage oder how to setup CocoaPods:
Regarding the move of the Xcode project: Since our package is named "CocoaLumberjack", SPM will generate a "CocoaLumberjack.xcodeproj". The Xcode project we currently have is named "Lumberjack.xcodeproj". This means that for now it would work, but I assume it could cause irritation on what is what... Given all the custom settings we have with the xcconfig files, I'd rather not rely on the SPM generated project. |
|
I think there is something slightly wrong with the SPM support as it is currently. Here is a sample project that uses it, and it builds/runs fine when running the app, but when trying to build the app's unit tests it will throw |
|
@amayers I'd say this is an Xcode issue. I'd bet this issue also occurs with any other SPM project that has several sub-dependencies. To prove this, you can simply link your "FrameworkSomething" target into the "SPM_TestTests" target by adding it to the "Link Binary With Libraries" build phase. This is all it takes for the Unit Tests to build fine. |
|
@ffried ok that worked. Although in my main project Cocoalumberjack was the only package out of 6 that had this issue. |
|
@amayers Do the other packages have internal dependencies? |
|
@ffried No, it doesn't look like any of them do. |
|
@amayers Thanks for the confirmation. I'll try to create a small reproduction project that is not using any external dependencies and file a radar with Apple about that. |
|
I'll update the Changelog on master directly. |
New Pull Request Checklist
I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (
pod lib lint)Pull Request Description
Integration
Cleanup