Add --show-dependencies option to dump external dependencies #302

bhargavg · 2016-04-30T20:41:25Z

Dump the external dependencies (along with their dependencies) to console.

Sample output for Vapor (v0.7.0):

$ swift-build --show-dependencies
.
├── C7<https://github.com/open-swift/C7.git@0.5.0>
├── S4<https://github.com/open-swift/S4.git@0.4.0>
│   └── C7<https://github.com/open-swift/C7.git@0.5.0>
├── String<https://github.com/Zewo/String.git@0.5.1>
├── StructuredData<https://github.com/Zewo/StructuredData.git@0.5.0>
│   └── C7<https://github.com/open-swift/C7.git@0.5.0>
├── JSON<https://github.com/Zewo/JSON.git@0.5.0>
│   └── StructuredData<https://github.com/Zewo/StructuredData.git@0.5.0>
│       └── C7<https://github.com/open-swift/C7.git@0.5.0>
├── Strand<https://github.com/ketzusaka/Strand.git@1.2.3>
├── Hummingbird<https://github.com/ketzusaka/Hummingbird.git@1.4.0>
│   ├── Strand<https://github.com/ketzusaka/Strand.git@1.2.3>
│   └── C7<https://github.com/open-swift/C7.git@0.5.0>
├── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>
├── HMAC<https://github.com/CryptoKitten/HMAC.git@0.5.2>
│   └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>
├── SHA2<https://github.com/CryptoKitten/SHA2.git@0.5.1>
│   └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>
└── Vapor<https://github.com/qutheory/vapor@0.7.0>
    ├── S4<https://github.com/open-swift/S4.git@0.4.0>
    │   └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── String<https://github.com/Zewo/String.git@0.5.1>
    ├── JSON<https://github.com/Zewo/JSON.git@0.5.0>
    │   └── StructuredData<https://github.com/Zewo/StructuredData.git@0.5.0>
    │       └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── Hummingbird<https://github.com/ketzusaka/Hummingbird.git@1.4.0>
    │   ├── Strand<https://github.com/ketzusaka/Strand.git@1.2.3>
    │   └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── HMAC<https://github.com/CryptoKitten/HMAC.git@0.5.2>
    │   └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>
    └── SHA2<https://github.com/CryptoKitten/SHA2.git@0.5.1>
        └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>

Only plain text option is supported as of now. Code is extensible to support additional formats like Dot and JSON.

Will add support for other formats in my next PRs.

czechboy0 · 2016-04-30T21:14:53Z

Hi @bhargavg, great stuff!

In the example, I would only expect the Vapor dependency and its dependency tree to be shown. AFAI understand the output, you're actually printing the dependency tree of all the leaf dependencies. So I'd only expect this to be output:

─── Vapor<https://github.com/qutheory/vapor@0.7.0>
    ├── S4<https://github.com/open-swift/S4.git@0.4.0>
    │   └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── String<https://github.com/Zewo/String.git@0.5.1>
    ├── JSON<https://github.com/Zewo/JSON.git@0.5.0>
    │   └── StructuredData<https://github.com/Zewo/StructuredData.git@0.5.0>
    │       └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── Hummingbird<https://github.com/ketzusaka/Hummingbird.git@1.4.0>
    │   ├── Strand<https://github.com/ketzusaka/Strand.git@1.2.3>
    │   └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── HMAC<https://github.com/CryptoKitten/HMAC.git@0.5.2>
    │   └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>
    └── SHA2<https://github.com/CryptoKitten/SHA2.git@0.5.1>
        └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>

Or what was the Package.swift of the example above?

czechboy0 · 2016-04-30T21:16:18Z

Also, I think it would be valuable to print the allowed version range, instead of the resolved version in each dependency. That's so that if there's a version conflict, it'd be obvious which two versions are clashing. Does that make sense?

ddunbar · 2016-04-30T23:40:39Z

Would it be too "magic" to hide the URL string if a package is already depended upon by something earlier in the graph? I think this would make it easier to read and see shared dependencies.

czechboy0 · 2016-05-01T06:02:55Z

@ddunbar I actually wanted to see the full URLs so that I can easily spot conflicting forks of the same project.

kostiakoval · 2016-05-01T09:16:50Z

Sources/swift-build/--show-dependencies.swift

+                    pkgVersion = version.description
+                } else {
+                    pkgVersion = "unspecified"
+                }


how about small code simplification?

let pkgVersion = package.version?.description ?? "unspecified"

bhargavg · 2016-05-01T13:56:41Z

AFAI understand the output, you're actually printing the dependency tree of all the leaf dependencies.

Good catch, I was actually printing all the leaf dependencies, which is wrong. Modified it to start the graph from root external dependencies.

$ cat Package.swift
import PackageDescription

let package = Package(
    name: "testvapor",
    dependencies: [
        .Package(url: "https://github.com/qutheory/vapor", Version(0,7,0))
    ]
)

$ swift-build --show-dependencies
.
└── Vapor<https://github.com/qutheory/vapor@0.7.0>
    ├── S4<https://github.com/open-swift/S4.git@0.4.0>
    │   └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── String<https://github.com/Zewo/String.git@0.5.1>
    ├── JSON<https://github.com/Zewo/JSON.git@0.5.0>
    │   └── StructuredData<https://github.com/Zewo/StructuredData.git@0.5.0>
    │       └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── Hummingbird<https://github.com/ketzusaka/Hummingbird.git@1.4.0>
    │   ├── Strand<https://github.com/ketzusaka/Strand.git@1.2.3>
    │   └── C7<https://github.com/open-swift/C7.git@0.5.0>
    ├── HMAC<https://github.com/CryptoKitten/HMAC.git@0.5.2>
    │   └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>
    └── SHA2<https://github.com/CryptoKitten/SHA2.git@0.5.1>
        └── CryptoEssentials<https://github.com/CryptoKitten/CryptoEssentials.git@0.5.2>

Now for the second point you mentioned:

Also, I think it would be valuable to print the allowed version range, instead of the resolved version in each dependency. That's so that if there's a version conflict, it'd be obvious which two versions are clashing.

I think, there are few issues with this approach:

Indicating version range doesn't tell you which version SwiftPM picked up
Incase of dependency hell, user has to manually scan through all the dependencies looking for a version clash

IMHO, indicating the version conflicts can be handled differently.

When there are unsolvable version requirements, we show an error message as follows:

error: The dependency graph could not be satisfied (url/of/dependency)

This error doesn't give enough info as to which all dependencies caused the failure.

If we can modify the error as follows, I think it will better help user in finding the problematic dependencies:

error: The dependency graph could not be satisfied
DepC(@v0.1.0) requires DepA(@v0.1.0)
DepB(@v0.1.0) requires DepA(@v0.2.0)

So, I suggest, we modify error reporting code rather than modifying --show-dependencies to handle this.

@ddunbar @mxcl thoughts?

czechboy0 · 2016-05-01T17:17:57Z

@bhargavg Awesome! Yeah you make a good point, better diagnostics in the dependency clashes would probably solve the problem even better. On the other hand, the dependency range is what each package requires, not a specific version. As I see it the dependencies should represent what the packages "want" (version range), not what they "get" (resolved version).

kostiakoval · 2016-05-01T17:33:12Z

Sources/swift-build/--show-dependencies.swift

+import PackageType
+import struct PackageDescription.Version
+
+final class ShowDependencies {


Do we need ShowDependencies class here? I think we can make dumpDependenciesOf a free function. It would be nicer to call it. ShowDependencies.dumpDependenciesOf( vs dumpDependenciesOf(

bhargavg · 2016-05-01T17:49:25Z

On the other hand, the dependency range is what each package requires, not a specific version. As I see it the dependencies should represent what the packages "want" (version range), not what they "get" (resolved version).

@czechboy0 If i understand you correctly, you are talking about the modified error output I proposed right? Yeah, that error message should have the range or explicit version specified in Package.swift file.

czechboy0 · 2016-05-01T18:04:34Z

@bhargavg I actually meant in the dump dependencies function. That I'd expect the representation of the wanted version range instead of the resolved function. But it's definitely something to discuss, we can change that later.

ddunbar · 2016-05-02T15:27:37Z

@czechboy0 I don't think you should be allowed to have two versions of the same package name in a graph, so conflicting forks shouldn't be possible (unless someone renamed it in the fork, which would then show up). I thought we currently enforced this (pretty sure it will fail to build, even if we don't).

mxcl · 2016-05-02T17:02:42Z

Lovely!

mxcl · 2016-05-02T17:04:57Z

If we'd like it to be more readable already (IMO this is fine to merge and we can tweak the usability after we've used it in real world settings), then I'd go with hiding the URLs unless there is a flag specified, same for ranges.

But this is a really great addition, thanks.

mxcl · 2016-05-03T21:04:37Z

Will merge unless there are objections.

I may change the mode to --print-dependencies, maybe, don't know.

bhargavg · 2016-05-03T21:15:50Z

@mxcl, the reason why this is not yet merged is, there is no decision made on whether this option should print resolved dependencies -or- the dependent ranges for each dependency as mentioned in their Package.swift

Also, @czechboy0 brought up a case when the graph is unresolvable, how best this option helps the user in identifying the conflicting dependencies.

mxcl · 2016-05-03T21:33:46Z

I don't think it is necessary to solve those issues before merging this PR. We will learn better what we need with this base functionality there to guide us.

bhargavg · 2016-05-04T04:46:40Z

Cool...!!! Can't wait to see this get merged.

Currently only plain text option is supported. But infrastructure is written to support additional formats like Dot and JSON.

aciidgh · 2016-05-04T18:21:35Z

Huh. I think swift ci only available for me on my PRs?
@modocache @shahmishal Do you have any info about this?

aciidgh · 2016-05-04T18:24:18Z

@swift-ci please test

bhargavg · 2016-05-04T18:44:36Z

Seems like CI is not started, @mxcl Could you please start CI on this and merge if no modifications are needed?

mxcl · 2016-05-04T19:50:03Z

@swift-ci Please test

shahmishal · 2016-05-04T19:52:22Z

@aciidb0mb3r Can you please try again when you get a chance? Thanks!

aciidgh · 2016-05-05T03:50:18Z

@swift-ci please test

aciidgh · 2016-05-05T03:50:59Z

@shahmishal tried again, din work

ddunbar · 2016-05-05T06:06:52Z

The test failures were unrelated, merging.

Add support for configuring the build system scheduler

kostiakoval reviewed May 1, 2016
View reviewed changes

bhargavg force-pushed the add-show-dependencies branch 3 times, most recently from 406dc94 to 7e05898 Compare May 1, 2016 13:59

kostiakoval reviewed May 1, 2016
View reviewed changes

bhargavg force-pushed the add-show-dependencies branch 2 times, most recently from 28380fa to 4868efd Compare May 4, 2016 18:17

Add --show-dependencies option to dump external dependencies

50e5ba1

Currently only plain text option is supported. But infrastructure is written to support additional formats like Dot and JSON.

bhargavg force-pushed the add-show-dependencies branch from 4868efd to 50e5ba1 Compare May 4, 2016 18:18

ddunbar merged commit 67cbcc6 into swiftlang:master May 5, 2016

aciidgh pushed a commit to aciidgh/swift-package-manager that referenced this pull request Jan 11, 2019

Merge pull request swiftlang#302 from dmbryson/lanewidth

5572520

Add support for configuring the build system scheduler

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --show-dependencies option to dump external dependencies #302

Add --show-dependencies option to dump external dependencies #302

bhargavg commented Apr 30, 2016 •

edited

Loading

czechboy0 commented Apr 30, 2016

czechboy0 commented Apr 30, 2016

ddunbar commented Apr 30, 2016

czechboy0 commented May 1, 2016

kostiakoval May 1, 2016 •

edited

Loading

bhargavg commented May 1, 2016

czechboy0 commented May 1, 2016

kostiakoval May 1, 2016

bhargavg May 1, 2016

bhargavg commented May 1, 2016

czechboy0 commented May 1, 2016

ddunbar commented May 2, 2016

mxcl commented May 2, 2016

mxcl commented May 2, 2016

mxcl commented May 3, 2016 •

edited

Loading

bhargavg commented May 3, 2016 •

edited

Loading

mxcl commented May 3, 2016

bhargavg commented May 4, 2016

aciidgh commented May 4, 2016 •

edited

Loading

aciidgh commented May 4, 2016

bhargavg commented May 4, 2016

mxcl commented May 4, 2016

shahmishal commented May 4, 2016

aciidgh commented May 5, 2016

aciidgh commented May 5, 2016

ddunbar commented May 5, 2016

Add --show-dependencies option to dump external dependencies #302

Add --show-dependencies option to dump external dependencies #302

Conversation

bhargavg commented Apr 30, 2016 • edited Loading

czechboy0 commented Apr 30, 2016

czechboy0 commented Apr 30, 2016

ddunbar commented Apr 30, 2016

czechboy0 commented May 1, 2016

kostiakoval May 1, 2016 • edited Loading

Choose a reason for hiding this comment

bhargavg commented May 1, 2016

czechboy0 commented May 1, 2016

kostiakoval May 1, 2016

Choose a reason for hiding this comment

bhargavg May 1, 2016

Choose a reason for hiding this comment

bhargavg commented May 1, 2016

czechboy0 commented May 1, 2016

ddunbar commented May 2, 2016

mxcl commented May 2, 2016

mxcl commented May 2, 2016

mxcl commented May 3, 2016 • edited Loading

bhargavg commented May 3, 2016 • edited Loading

mxcl commented May 3, 2016

bhargavg commented May 4, 2016

aciidgh commented May 4, 2016 • edited Loading

aciidgh commented May 4, 2016

bhargavg commented May 4, 2016

mxcl commented May 4, 2016

shahmishal commented May 4, 2016

aciidgh commented May 5, 2016

aciidgh commented May 5, 2016

ddunbar commented May 5, 2016

bhargavg commented Apr 30, 2016 •

edited

Loading

kostiakoval May 1, 2016 •

edited

Loading

mxcl commented May 3, 2016 •

edited

Loading

bhargavg commented May 3, 2016 •

edited

Loading

aciidgh commented May 4, 2016 •

edited

Loading