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 devDependencies support in SwiftPM #7007

Open
Kyle-Ye opened this issue Oct 16, 2023 · 6 comments
Open

Add devDependencies support in SwiftPM #7007

Kyle-Ye opened this issue Oct 16, 2023 · 6 comments

Comments

@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented Oct 16, 2023

Description

context: swiftlang/swift-markdown#153 (comment)

When integrating swift-testing into a library, the downstream user of the library will also get the dependency for swift-testing and its dependency(SwiftSyntax)

I propose to add a new devDependencies parameters for the existing Package model.

"devDependencies" is only used for development and testing. And it can't be used on target which will be exported via the products.

Other Languages and Package Manager have the same idea
See npm

Expected behavior

dependencies: [
  .package(xx),
],
// devDependencies can't be used on targets exposed by products
devDependencies: [ // or testDependencies
  .package(url: "https://github.com/apple/swift-testing.git", branch: "main"),
]

Actual behavior

dependencies: [
  .package(xx),
  // the dependency for swift-testing and its dependency(SwiftSyntax) will also be exported
  .package(url: "https://github.com/apple/swift-testing.git", branch: "main"),
],

Steps to reproduce

No response

Swift Package Manager version/commit hash

5.9

Swift & OS version (output of swift --version && uname -a)

No response

@Kyle-Ye Kyle-Ye changed the title Add testDependencies support in SwiftPM Add devDependencies support in SwiftPM Oct 16, 2023
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 16, 2023

Relevant PR: #74

There is something named privateDependencies in the repo and I don't if that's what we want. But even it were, I do not find way to specify them in a Package.swift file.

@neonichu
Copy link
Contributor

neonichu commented Oct 16, 2023

This should already happen automatically, dependencies of a transitive dependency are only considered if they're depended on by a product.

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 16, 2023

This should already happen automatically, dependencies of a transitive dependency are only considered if they're depended on by a product.

I'm not talking about the dependency on the swift code side / target side. But on the whole package graph side.

eg. swift-docc-plugin have a dependency on swift-docc-symbolkit. Then if Package A wants simple doc support and the maintainer add swift-docc-plugin as a package dependency, the downstream user of A can't use another version of swift-docc-symbolkit. (This is current a warning, but will turn into error in a future version.) Actually non targets/products shipped by A have a dependency on swift-docc-plugin non swift-docc-symbolkit.

See swiftlang/swift-docc-symbolkit#62 (comment) for more context.

This is the same for swift-testing which has a dependency on swift-syntax-5.9.0.

Generally swift-docc-plugin and swift-testing are actually for dev only. And both they and their dependency should not "pollute" the normal dependency graph.

@neonichu
Copy link
Contributor

neonichu commented Oct 16, 2023

Interesting, I think maybe the way the warning gets computed is incorrect? It may take into account all possible dependencies vs. only reachable ones.

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 16, 2023

Interesting, I think maybe the way the warning gets computed is incorrect?

IMO the warning is expected.

Given the following code as an example, we can't guarantee no target's will have a dependency on swift-docc-plugin which already has a dependency constraint on SymbolKit.

let package = Package(
    name: "Documentation",
    dependencies: [
        .package(name: "SymbolKit", path: "../."),
        .package(url: "https://github.com/apple/swift-docc-plugin", from: "1.3.0"),
    ],
    targets: [
        .target(
            name: "Empty",
            dependencies: [
                .product(name: "SymbolKit", package: "SymbolKit"),
            ],
            path: "Empty"
        )
    ]
)
  • One solution is to add "devDependencies" Semantics to differentiate the 2 case.
  • If you view it as "valid" code and think the warning is incorrect, we can also fix the warning by another solution.
    • Only when at least one target both have a dependency on swift-docc-plugin and the local SymbolKit, will we emit a warning/error. Otherwise we won't.
let package = Package(
    name: "Documentation",
    dependencies: [
        .package(name: "SymbolKit", path: "../."),
        // Not emit. Since no targets have a dependency on swift-docc-plugin
        .package(url: "https://github.com/apple/swift-docc-plugin", from: "1.3.0"),
    ],
    targets: [
        .target(
            name: "Empty",
            dependencies: [
                .product(name: "SymbolKit", package: "SymbolKit"),
            ],
            path: "Empty"
        )
    ]
)
let package = Package(
    name: "Documentation",
    dependencies: [
        .package(name: "SymbolKit", path: "../."),
        // Emit. Conflict on C
        .package(url: "https://github.com/apple/swift-docc-plugin", from: "1.3.0"),
    ],
    targets: [
        .target(
            name: "A",
            plugins: [.plugin(name: "xx", package: "swift-docc-plugin")]
        ),
        .target(
            name: "B",
            dependencies: [
                .product(name: "SymbolKit", package: "SymbolKitLocal"),
            ]
        ),
        .target(
            name: "C",
            dependencies: ["A", "B"]
        ),
    ]
)

@neonichu
Copy link
Contributor

Yah, I think we should only emit a warning for dependencies in use.

Development dependencies may be useful separately, but in the case where we can already know the intent, they seem superfluous.

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

No branches or pull requests

2 participants