danger / swift

⚠️ Stop saying "you forgot to …" in code review
https://danger.systems/swift/
MIT License
1.04k stars 136 forks source link

Latest release unhides DangerDeps product causing failures #475

Closed AvdLee closed 2 years ago

AvdLee commented 2 years ago

This is the error I'm getting: image

Swift package target 'Danger' is linked as a static library by 'WeTransferPRLinterTests' and 'Danger', but cannot be built dynamically because there is a package product with the same name.

Using Xcode 13. The plugin is open-source, so you can try it out: https://github.com/WeTransfer/WeTransfer-iOS-CI/tree/master/WeTransferPRLinter

This is currently how the Package.swift looks:

// swift-tools-version:5.5
// The swift-tools-version declares the minimum version of Swift required to build this package.

import PackageDescription

let package = Package(
    name: "WeTransferPRLinter",
    products: [
        .library(
            name: "WeTransferPRLinter",
            targets: ["WeTransferPRLinter"]
        )
    ],
    dependencies: [
        .package(url: "https://github.com/danger/swift", .exact("3.10.2")),
        .package(name: "DangerSwiftCoverage", url: "https://github.com/f-meloni/danger-swift-coverage", from: "1.1.0"),
        .package(name: "DangerXCodeSummary", url: "https://github.com/f-meloni/danger-swift-xcodesummary", from: "1.2.1"),
        .package(name: "Files", url: "https://github.com/JohnSundell/Files", from: "4.1.1")
    ],
    targets: [
        .target(
            name: "WeTransferPRLinter",
            dependencies: [
                .product(name: "Danger", package: "swift"),
                "DangerSwiftCoverage",
                "DangerXCodeSummary",
                "Files"
            ]
        ),
        .testTarget(
            name: "WeTransferPRLinterTests",
            dependencies: [
                "WeTransferPRLinter",
                .product(name: "DangerFixtures", package: "swift")
            ]
        )
    ]
)

Commenting out the test target lets Xcode build successfully.

AvdLee commented 2 years ago

@f-meloni I feel like many of my problems have been starting with this line not being commented out in the .12 release tag: https://github.com/danger/swift/blob/3.12.0/Package.swift#L12

AvdLee commented 2 years ago

Confirmed, finally got everything working again. The above line should be commented out, right?

I've created my own fork which makes my Danger work again. I can even run it locally on an M1 machine. In other words, this would solve:

As I'm building using Xcode 13 and Swift 5.5 on a M1 MacBook

To be clear; I tried using:

.library(name: "DangerDeps[PACKAGE_NAME]", type: .dynamic, targets: ["DangerDependencies"]),

Instead of:

.library(name: "DangerDeps", type: .dynamic, targets: ["DangerDependencies"]),

But I couldn't get that to work in anyway

Updated the title of this issue!

417-72KI commented 2 years ago

According to the old tags,

.library(name: "DangerDeps", type: .dynamic, targets: ["DangerDependencies"]),

should be commented out on release, as @AvdLee said.

https://github.com/danger/swift/blob/3.11.0/Package.swift https://github.com/danger/swift/blob/3.10.0/Package.swift

f-meloni commented 2 years ago

Thank you very much for investigating this! https://github.com/danger/swift/pull/477 should solve the issue, and then I can make a new release.

f-meloni commented 2 years ago

According to the old tags,

.library(name: "DangerDeps", type: .dynamic, targets: ["DangerDependencies"]),

should be commented out on release, as @AvdLee said.

https://github.com/danger/swift/blob/3.11.0/Package.swift https://github.com/danger/swift/blob/3.10.0/Package.swift

Yes, just realised that I've missed this change on https://github.com/danger/swift/pull/414

f-meloni commented 2 years ago

I've just released 3.12.1 https://github.com/danger/swift/commit/ac99660abd8253b97f4d73e8ed756f347196d14a let me know how it goes

AvdLee commented 2 years ago

@f-meloni confirmed that release fixed all my issues. Closing this one. Thanks for the quick turnaround!

f-meloni commented 2 years ago

Amazing! Thank you for all the help!

AvdLee commented 2 years ago

@f-meloni reopening this issue for the original issue description; I'm still getting this error for my plugin:

Swift package target 'Danger' is linked as a static library by 'WeTransferPRLinterTests' and 'Danger', but cannot be built dynamically because there is a package product with the same name.

Don't you have the same for your plugins once you update it to Swift 5.5?

AvdLee commented 2 years ago

Interesting enough it seems fixable by removing the .dynamic type from the package.swift:

image

I'm not sure how this affects regular Danger usage though

f-meloni commented 2 years ago

If I remember correctly we need to link Danger when we compile the Dangerfile https://github.com/danger/swift/blob/master/Sources/RunnerLib/SPMDanger.swift#L48 https://github.com/danger/swift/blob/master/Sources/Runner/Commands/Runner.swift#L108

And dynamic is to force SPM to create a library we can link.

Don't you have the same for your plugins once you update it to Swift 5.5?

🤔 Let me give it a try

AvdLee commented 2 years ago

And dynamic is to force SPM to create a library we can link.

Back in the days .dynamic was needed due to SPM not correctly supporting static linking IIRC. I wouldn't be surprised if this is fixed in Swift 5.5, so it's worth a try! Either way, the current setup does seem to break plugin testing 🤔

f-meloni commented 2 years ago

@AvdLee from my very first test, everything looks ok https://github.com/f-meloni/danger-swift-coverage/pull/31.

AvdLee commented 2 years ago

@f-meloni posted my feedback inside the PR you opened!