Skip to content

[SR-14782] swift package describe is omitting product dependencies from per-target output #3556

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 21, 2021
Merged

[SR-14782] swift package describe is omitting product dependencies from per-target output #3556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Sources/Commands/Describe.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ fileprivate struct DescribedPackage: Encodable {
let sources: [String]
let resources: [PackageModel.Resource]?
let targetDependencies: [String]?
let productDependencies: [String]?
let productMemberships: [String]?

init(from target: Target, in package: Package, productMemberships: [String]?) {
Expand All @@ -166,7 +167,10 @@ fileprivate struct DescribedPackage: Encodable {
self.path = target.sources.root.relative(to: package.path).pathString
self.sources = target.sources.relativePaths.map{ $0.pathString }
self.resources = target.resources.isEmpty ? nil : target.resources
self.targetDependencies = target.dependencies.isEmpty ? nil : target.dependencies.compactMap{ $0.target?.name }
let targetDependencies = target.dependencies.compactMap{ $0.target }
self.targetDependencies = targetDependencies.isEmpty ? nil : targetDependencies.map{ $0.name }
let productDependencies = target.dependencies.compactMap{ $0.product }
self.productDependencies = productDependencies.isEmpty ? nil : productDependencies.map{ $0.name }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @yim-lee for potential impact on collection generator

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory this should have no impact since it's additive, but that's a good point. I guess this would have the effect of omitting the target_dependencies empty array for a target that had only product dependencies, in addition to adding the product_dependencies array. I don't know if product dependencies is something that you might want to start including in the collections at some point — I don't think that's in the data structures now (are target dependencies there now?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks.

The generator is not using describe yet (but will), so this change has no impact. I don't think we will include *_dependencies in the package collection data, but IIRC the intention is that it will help determine which targets to include (i.e., only targets included in a product).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. That's in the separate product_memberships array, and unaffected by this.

self.productMemberships = productMemberships
}
}
Expand Down
15 changes: 15 additions & 0 deletions Tests/CommandsTests/PackageToolTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,21 @@ final class PackageToolTests: XCTestCase {
XCTAssert(textChunk6.contains("Path: Sources/CExec"), textChunk6)
XCTAssert(textChunk6.contains("Sources:\n main.c"), textChunk6)
}

fixture(name: "DependencyResolution/External/Simple/Bar") { prefix in
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses an existing test fixture so we don't need to add more of them.

// Generate the JSON description.
let jsonResult = try SwiftPMProduct.SwiftPackage.executeProcess(["describe", "--type=json"], packagePath: prefix)
let jsonOutput = try jsonResult.utf8Output()
let json = try JSON(bytes: ByteString(encodingAsUTF8: jsonOutput))

// Check that product dependencies and memberships are as expected.
XCTAssertEqual(json["name"]?.string, "Bar")
let jsonTarget = try XCTUnwrap(json["targets"]?.array?[0])
XCTAssertEqual(jsonTarget["product_memberships"]?.array?[0].stringValue, "Bar")
XCTAssertEqual(jsonTarget["product_dependencies"]?.array?[0].stringValue, "Foo")
XCTAssertNil(jsonTarget["target_dependencies"])
}

}

func testDescribePackageUsingPlugins() throws {
Expand Down