Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public enum PackageDependency: Equatable, Hashable, Sendable {
/// A condition that limits the application of a dependencies trait.
package struct Condition: Hashable, Sendable, Codable {
/// The set of traits of this package that enable the dependency's trait.
private let traits: Set<String>?
package let traits: Set<String>?

public init(traits: Set<String>?) {
self.traits = traits
Expand Down Expand Up @@ -73,6 +73,11 @@ public enum PackageDependency: Equatable, Hashable, Sendable {
condition: condition
)
}

// represents `.defaults`
public var isDefaultsCase: Bool {
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the lack of explanation.

The main purpose of this patch is not to make it public.

However, since PackageModel.TraitDescription.isDefault already exists and is public, I thought it would be natural to make PackageModel.PackageDependency.Trait.isDefaultsCase public as well.

In particular, for this part, the user-facing API uses .defaults, while the internal representation uses "default", which do not match. This makes it easy to make mistakes during implementation, so I thought it would be helpful to provide an API.

name == "default" && condition == nil
}
}

case fileSystem(FileSystem)
Expand Down
149 changes: 148 additions & 1 deletion Sources/PackageModel/ManifestSourceGeneration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ fileprivate extension SourceCodeFragment {
params.append(SourceCodeFragment(key: "products", subnodes: nodes))
}

if !manifest.traits.isEmpty {
let nodes = manifest.traits.map { SourceCodeFragment(from: $0) }
params.append(SourceCodeFragment(key: "traits", subnodes: nodes))
}

if !manifest.dependencies.isEmpty {
let nodes = manifest.dependencies.map{ SourceCodeFragment(from: $0, pathAnchor: packageDirectory) }
params.append(SourceCodeFragment(key: "dependencies", subnodes: nodes))
Expand Down Expand Up @@ -192,9 +197,68 @@ fileprivate extension SourceCodeFragment {
params.append(SourceCodeFragment("\"\(range.lowerBound)\"..<\"\(range.upperBound)\""))
}
}

if let traits = dependency.traits {
// If only `.defaults` is specified, do not output `traits:` .
// This is because `traits:` is not available in toolchains earlier than 6.1.
let isDefault = traits.count == 1 &&
traits.allSatisfy(\.isDefaultsCase)

if !isDefault {
let traits = traits.sorted { a, b in
PackageDependency.Trait.precedes(a, b)
}
params.append(
SourceCodeFragment(
key: "traits",
subnodes: traits.map { SourceCodeFragment(from: $0) }
)
)
}
}

self.init(enum: "package", subnodes: params)
}


init(from trait: PackageDependency.Trait) {
if trait.isDefaultsCase {
self.init(enum: "defaults")
return
}

guard let condition = trait.condition else {
self.init(string: trait.name)
return
}

let conditionNode = SourceCodeFragment(
key: "condition",
subnode: SourceCodeFragment(from: condition)
)

self.init(enum: "trait", subnodes: [
SourceCodeFragment(key: "name", string: trait.name),
conditionNode
])
}

init(from condition: PackageDependency.Trait.Condition) {
var params: [SourceCodeFragment] = []

if let trait = condition.traits {
params.append(
SourceCodeFragment(
key: "traits",
subnodes: trait.sorted().map {
SourceCodeFragment(string: $0)
}
)
)
}

self.init(enum: "when", subnodes: params)
}

/// Instantiates a SourceCodeFragment to represent a single product. If there's a custom product generator, it gets
/// a chance to generate the source code fragments before checking the default types.
init(from product: ProductDescription, customProductTypeSourceGenerator: ManifestCustomProductTypeSourceGenerator?, toolsVersion: ToolsVersion) rethrows {
Expand Down Expand Up @@ -261,6 +325,41 @@ fileprivate extension SourceCodeFragment {
}
}

init(from trait: TraitDescription) {
let enabledTraitsNode = SourceCodeFragment(
key: "enabledTraits",
subnodes: trait.enabledTraits.sorted().map {
SourceCodeFragment(string: $0)
}
)

if trait.isDefault {
self.init(enum: "default", subnodes: [enabledTraitsNode])
return
}

if trait.description == nil, trait.enabledTraits.isEmpty {
self.init(string: trait.name)
return
}

var params: [SourceCodeFragment] = [
SourceCodeFragment(key: "name", string: trait.name)
]

if let description = trait.description {
params.append(
SourceCodeFragment(key: "description", string: description)
)
}

if !trait.enabledTraits.isEmpty {
params.append(enabledTraitsNode)
}

self.init(enum: "trait", subnodes: params)
}

/// Instantiates a SourceCodeFragment to represent a single target.
init(from target: TargetDescription) {
var params: [SourceCodeFragment] = []
Expand Down Expand Up @@ -416,6 +515,13 @@ fileprivate extension SourceCodeFragment {
if let configName = condition.config {
params.append(SourceCodeFragment(key: "configuration", enum: configName))
}
if let traits = condition.traits {
params.append(
SourceCodeFragment(key: "traits", subnodes: traits.sorted().map { trait in
SourceCodeFragment(string: trait)
})
)
}
self.init(enum: "when", subnodes: params)
}

Expand Down Expand Up @@ -1023,6 +1129,47 @@ public struct SourceCodeFragment {
}
}

extension Optional {
fileprivate static func precedes(
_ a: Wrapped?, _ b: Wrapped?,
compareWrapped: (Wrapped, Wrapped) -> Bool
) -> Bool {
switch (a, b) {
case (.none, .none): return false
case (.none, .some): return true
case (.some, .none): return false
case (.some(let a), .some(let b)):
return compareWrapped(a, b)
}
}
}

extension PackageDependency.Trait {
fileprivate static func precedes(_ a: PackageDependency.Trait, _ b: PackageDependency.Trait) -> Bool {
if a.name != b.name { return a.name < b.name }
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: I'm not sure we can ever have two traits of the same name in a single package (cc @FranzBusch is this a correct assumption?), so sorting traits that originate from the same package can probably just be done by using traits.sorted(by: { $0.name < $1.name }) (this can similarly be done for the traits listed in the condition)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, if we unify the directives, sorting becomes simpler, which is nice, but...

At present, the following is valid:

dependencies: [
    .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "601.0.1", traits: [
        .trait(name: "Foo", condition: .when(traits: ["Blue"])),
        .trait(name: "Foo", condition: .when(traits: ["Green"])),
    ]),
],

It can be unified as follows:

dependencies: [
    .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "601.0.1", traits: [
        .trait(name: "Foo", condition: .when(traits: ["Blue", "Green"])),
    ]),
],

However, for example, if it is extended like this...

dependencies: [
    .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "601.0.1", traits: [
        .trait(name: "Foo", condition: .when(platforms: [.macOS], traits: ["Blue"])),
        .trait(name: "Foo", condition: .when(platforms: [.linux], traits: ["Green"])),
    ]),
],

This does not seem to be unifiable.

In the future, if it ever comes to this, we might overlook the necessary adjustments.
What should we do about that?

Copy link
Member

Choose a reason for hiding this comment

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

There are two parts to traits.

  1. The place where they are defined
  2. The place where they are enabled for a given dependency

In the former, trait names can only be expressed once. In the latter, @omochi is right that it is totally valid to define an enabled trait multiple times with different conditions.

Copy link
Contributor

Choose a reason for hiding this comment

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

@omochi Thank you for the detailed examples! This helps a lot.


if a.condition != b.condition {
return Optional.precedes(a.condition, b.condition) { a, b in
PackageDependency.Trait.Condition.precedes(a, b)
}
}

return false
}
}

extension PackageDependency.Trait.Condition {
fileprivate static func precedes(_ a: PackageDependency.Trait.Condition, _ b: PackageDependency.Trait.Condition) -> Bool {
if a.traits != b.traits {
return Optional.precedes(a.traits, b.traits) { a, b in
a.sorted().lexicographicallyPrecedes(b.sorted())
}
}

return false
}
}

extension TargetBuildSettingDescription.Kind {
fileprivate var name: String {
switch self {
Expand Down
10 changes: 9 additions & 1 deletion Sources/_InternalTestSupport/XCTAssertHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,19 @@ public func XCTRequires(
}
}

public func XCTSkipIfCompilerLessThan6_1() throws {
swiftTestingTestCalledAnXCTestAPI()
#if compiler(>=6.1)
#else
throw XCTSkip("Skipping as compiler version is less than 6.1")
#endif
}

public func XCTSkipIfCompilerLessThan6_2() throws {
swiftTestingTestCalledAnXCTestAPI()
#if compiler(>=6.2)
#else
throw XCTSkip("Skipping as compiler version is less thann 6.2")
throw XCTSkip("Skipping as compiler version is less than 6.2")
#endif
}

Expand Down
55 changes: 55 additions & 0 deletions Tests/WorkspaceTests/ManifestSourceGenerationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ final class ManifestSourceGenerationTests: XCTestCase {
XCTAssertEqual(newManifest.pkgConfig, manifest.pkgConfig, "pkgConfig not as expected" + failureDetails, file: file, line: line)
XCTAssertEqual(newManifest.providers, manifest.providers, "providers not as expected" + failureDetails, file: file, line: line)
XCTAssertEqual(newManifest.products, manifest.products, "products not as expected" + failureDetails, file: file, line: line)
XCTAssertEqual(newManifest.traits, manifest.traits, "traits not as expected" + failureDetails, file: file, line: line)
XCTAssertEqual(newManifest.dependencies, manifest.dependencies, "dependencies not as expected" + failureDetails, file: file, line: line)
XCTAssertEqual(newManifest.targets, manifest.targets, "targets not as expected" + failureDetails, file: file, line: line)
XCTAssertEqual(newManifest.swiftLanguageVersions, manifest.swiftLanguageVersions, "swiftLanguageVersions not as expected" + failureDetails, file: file, line: line)
Expand Down Expand Up @@ -948,4 +949,58 @@ final class ManifestSourceGenerationTests: XCTestCase {
let contents = try manifest.generateManifestFileContents(packageDirectory: manifest.path.parentDirectory)
try await testManifestWritingRoundTrip(manifestContents: contents, toolsVersion: .v6_2)
}

func testTraits() async throws {
try XCTSkipIfCompilerLessThan6_1()

let manifestContents = """
// swift-tools-version: 6.1
import PackageDescription

let package = Package(
name: "TraitExample",
traits: [
"Foo",
.trait(
name: "Bar",
enabledTraits: [
"Foo",
]
),
.trait(
name: "FooBar",
enabledTraits: [
"Foo",
"Bar",
]
),
.default(enabledTraits: ["Foo"]),
],
dependencies: [
.package(
url: "https://github.com/Org/SomePackage.git",
from: "1.0.0",
traits: [
.defaults,
"SomeTrait",
.trait(name: "SomeOtherTrait", condition: .when(traits: ["Foo"])),
]
),
],
targets: [
.target(
name: "SomeTarget",
dependencies: [
.product(
name: "SomeProduct",
package: "SomePackage",
condition: .when(traits: ["Foo"])
),
]
)
]
)
"""
try await testManifestWritingRoundTrip(manifestContents: manifestContents, toolsVersion: .v6_1)
}
}