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

Latest release unhides DangerDeps product causing failures #475

Closed
AvdLee opened this issue Oct 12, 2021 · 14 comments · Fixed by #480
Closed

Latest release unhides DangerDeps product causing failures #475

AvdLee opened this issue Oct 12, 2021 · 14 comments · Fixed by #480
Labels

Comments

@AvdLee
Copy link
Contributor

AvdLee commented Oct 12, 2021

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 AvdLee added the bug label Oct 12, 2021
@AvdLee
Copy link
Contributor Author

AvdLee commented Oct 13, 2021

@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
Copy link
Contributor Author

AvdLee commented Oct 13, 2021

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!

@AvdLee AvdLee changed the title Plugin can no longer use DangerFixtures Latest release unhides DangerDeps product causing failures Oct 13, 2021
@417-72KI
Copy link
Member

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
Copy link
Member

Thank you very much for investigating this!
#477 should solve the issue, and then I can make a new release.

@f-meloni
Copy link
Member

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 #414

@f-meloni
Copy link
Member

I've just released 3.12.1 ac99660
let me know how it goes

@AvdLee
Copy link
Contributor Author

AvdLee commented Oct 13, 2021

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

@AvdLee AvdLee closed this as completed Oct 13, 2021
@f-meloni
Copy link
Member

Amazing! Thank you for all the help!

@AvdLee
Copy link
Contributor Author

AvdLee commented Oct 21, 2021

@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 AvdLee reopened this Oct 21, 2021
@AvdLee
Copy link
Contributor Author

AvdLee commented Oct 21, 2021

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
Copy link
Member

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
Copy link
Contributor Author

AvdLee commented Oct 22, 2021

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
Copy link
Member

@AvdLee from my very first test, everything looks ok f-meloni/danger-swift-coverage#31.

@AvdLee
Copy link
Contributor Author

AvdLee commented Oct 25, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants