Skip to content

Commit 83bcb5c

Browse files
tomerdtomerd
committed
Update to manifest API to make it impossible to create an invalid target dependency condition.
motivation: reduce use of preconditions, make editing manifest safer changes: * deprecate TargetDependencyCondition "when" initializor that uses a precondition * move TargetDependencyCondition code to a more approprate place * add tests * update change log
1 parent 25dd417 commit 83bcb5c

File tree

6 files changed

+102
-33
lines changed

6 files changed

+102
-33
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@ Swift 5.7
1313

1414
* [#4131]
1515

16-
Update to manifest API to make it impossible to create an invalid build setttings condition.
16+
Update to manifest API to make it impossible to create an invalid build settings condition.
1717

1818
* [#4135]
1919

2020
Enable linker dead stripping for all platforms. This can be disabled with `--disable-dead-strip`
2121

22+
* [#4168]
23+
24+
Update to manifest API to make it impossible to create an invalid target dependency condition.
25+
2226
Swift 5.6
2327
-----------
2428
* [SE-0332]
@@ -232,4 +236,4 @@ Swift 3.0
232236
[#4119]: https://github.com/apple/swift-package-manager/pull/4119
233237
[#4131]: https://github.com/apple/swift-package-manager/pull/4131
234238
[#4135]: https://github.com/apple/swift-package-manager/pull/4135
235-
239+
[#4168]: https://github.com/apple/swift-package-manager/pull/4168

‎Sources/PackageDescription/BuildSettings.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public struct BuildSettingCondition: Encodable {
5858
self.config = config
5959
}
6060

61-
@available(_PackageDescription, deprecated: 999.0)
61+
@available(_PackageDescription, deprecated: 5.7)
6262
public static func when(
6363
platforms: [Platform]? = nil,
6464
configuration: BuildConfiguration? = nil
@@ -72,7 +72,7 @@ public struct BuildSettingCondition: Encodable {
7272
/// - Parameters:
7373
/// - platforms: The applicable platforms for this build setting condition.
7474
/// - configuration: The applicable build configuration for this build setting condition.
75-
@available(_PackageDescription, introduced: 999.0)
75+
@available(_PackageDescription, introduced: 5.7)
7676
public static func when(platforms: [Platform], configuration: BuildConfiguration) -> BuildSettingCondition {
7777
BuildSettingCondition(platforms: platforms, config: configuration)
7878
}
@@ -81,7 +81,7 @@ public struct BuildSettingCondition: Encodable {
8181
///
8282
/// - Parameters:
8383
/// - platforms: The applicable platforms for this build setting condition.
84-
@available(_PackageDescription, introduced: 999.0)
84+
@available(_PackageDescription, introduced: 5.7)
8585
public static func when(platforms: [Platform]) -> BuildSettingCondition {
8686
BuildSettingCondition(platforms: platforms, config: .none)
8787
}
@@ -90,7 +90,7 @@ public struct BuildSettingCondition: Encodable {
9090
///
9191
/// - Parameters:
9292
/// - configuration: The applicable build configuration for this build setting condition.
93-
@available(_PackageDescription, introduced: 999.0)
93+
@available(_PackageDescription, introduced: 5.7)
9494
public static func when(configuration: BuildConfiguration) -> BuildSettingCondition {
9595
BuildSettingCondition(platforms: .none, config: configuration)
9696
}

‎Sources/PackageDescription/PackageDescriptionSerialization.swift

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -384,28 +384,6 @@ extension Target: Encodable {
384384
}
385385
}
386386

387-
/// A condition that limits the application of a target's dependency.
388-
public struct TargetDependencyCondition: Encodable {
389-
390-
private let platforms: [Platform]?
391-
392-
private init(platforms: [Platform]?) {
393-
self.platforms = platforms
394-
}
395-
396-
/// Creates a target dependency condition.
397-
///
398-
/// - Parameters:
399-
/// - platforms: The applicable platforms for this target dependency condition.
400-
public static func when(
401-
platforms: [Platform]? = nil
402-
) -> TargetDependencyCondition {
403-
// FIXME: This should be an error, not a precondition.
404-
precondition(!(platforms == nil))
405-
return TargetDependencyCondition(platforms: platforms)
406-
}
407-
}
408-
409387
extension Target.PluginUsage: Encodable {
410388
private enum CodingKeys: CodingKey {
411389
case type, name, package

‎Sources/PackageDescription/Target.swift

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,8 @@ extension Target.Dependency {
992992
/// - package: The name of the package.
993993
/// - condition: A condition that limits the application of the target dependency. For example, only apply a
994994
/// dependency for a specific platform.
995-
@available(_PackageDescription, introduced: 5.3, obsoleted: 999.0)
995+
@_disfavoredOverload
996+
@available(_PackageDescription, introduced: 5.3, obsoleted: 5.7)
996997
public static func product(
997998
name: String,
998999
package: String,
@@ -1009,7 +1010,7 @@ extension Target.Dependency {
10091010
/// - moduleAliases: The module aliases for targets in the product.
10101011
/// - condition: A condition that limits the application of the target dependency. For example, only apply a
10111012
/// dependency for a specific platform.
1012-
@available(_PackageDescription, introduced: 999.0)
1013+
@available(_PackageDescription, introduced: 5.7)
10131014
public static func product(
10141015
name: String,
10151016
package: String,
@@ -1032,6 +1033,40 @@ extension Target.Dependency {
10321033
}
10331034
}
10341035

1036+
/// A condition that limits the application of a target's dependency.
1037+
public struct TargetDependencyCondition: Encodable {
1038+
private let platforms: [Platform]?
1039+
1040+
private init(platforms: [Platform]?) {
1041+
self.platforms = platforms
1042+
}
1043+
1044+
/// Creates a target dependency condition.
1045+
///
1046+
/// - Parameters:
1047+
/// - platforms: The applicable platforms for this target dependency condition.
1048+
@_disfavoredOverload
1049+
@available(_PackageDescription, obsoleted: 5.7, message: "using .when with nil platforms is obsolete")
1050+
public static func when(
1051+
platforms: [Platform]? = nil
1052+
) -> TargetDependencyCondition {
1053+
// FIXME: This should be an error, not a precondition.
1054+
precondition(!(platforms == nil))
1055+
return TargetDependencyCondition(platforms: platforms)
1056+
}
1057+
1058+
/// Creates a target dependency condition.
1059+
///
1060+
/// - Parameters:
1061+
/// - platforms: The applicable platforms for this target dependency condition.
1062+
@available(_PackageDescription, introduced: 5.7)
1063+
public static func when(
1064+
platforms: [Platform]
1065+
) -> TargetDependencyCondition? {
1066+
return !platforms.isEmpty ? TargetDependencyCondition(platforms: platforms) : .none
1067+
}
1068+
}
1069+
10351070
extension Target.PluginCapability {
10361071

10371072
/// Specifies that the plugin provides a build tool capability. The plugin
@@ -1132,7 +1167,7 @@ extension Target.PluginUsage {
11321167
// MARK: ExpressibleByStringLiteral
11331168

11341169
extension Target.Dependency: ExpressibleByStringLiteral {
1135-
1170+
11361171
/// Creates a target dependency instance with the given value.
11371172
///
11381173
/// - parameters:
@@ -1143,7 +1178,7 @@ extension Target.Dependency: ExpressibleByStringLiteral {
11431178
}
11441179

11451180
extension Target.PluginUsage: ExpressibleByStringLiteral {
1146-
1181+
11471182
/// Specifies use of a plugin target in the same package.
11481183
///
11491184
/// - parameters:

‎Tests/PackageLoadingTests/PD_5_3_LoadingTests.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,6 @@ class PackageDescription5_3LoadingTests: PackageDescriptionLoadingTests {
451451
XCTAssertEqual(dependencies[1], .target(name: "Bar", condition: .init(platformNames: ["linux"], config: nil)))
452452
XCTAssertEqual(dependencies[2], .product(name: "Baz", package: "Baz", condition: .init(platformNames: ["macos"])))
453453
XCTAssertEqual(dependencies[3], .byName(name: "Bar", condition: .init(platformNames: ["watchos", "ios"])))
454-
455454
}
456455

457456
func testDefaultLocalization() throws {

‎Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,65 @@
99
*/
1010

1111
import Basics
12+
import PackageLoading
1213
import PackageModel
1314
import SPMTestSupport
15+
import TSCBasic
1416
import XCTest
1517

1618
class PackageDescription5_7LoadingTests: PackageDescriptionLoadingTests {
1719
override var toolsVersion: ToolsVersion {
1820
.v5_7
1921
}
22+
23+
func testConditionalTargetDependencies() throws {
24+
let content = """
25+
import PackageDescription
26+
let package = Package(
27+
name: "Foo",
28+
dependencies: [],
29+
targets: [
30+
.target(name: "Foo", dependencies: [
31+
.target(name: "Bar", condition: .when(platforms: [])),
32+
.target(name: "Baz", condition: .when(platforms: [.linux])),
33+
]),
34+
.target(name: "Bar"),
35+
.target(name: "Baz"),
36+
]
37+
)
38+
"""
39+
40+
let observability = ObservabilitySystem.makeForTesting()
41+
let manifest = try loadManifest(content, observabilityScope: observability.topScope)
42+
XCTAssertNoDiagnostics(observability.diagnostics)
43+
44+
let dependencies = manifest.targets[0].dependencies
45+
XCTAssertEqual(dependencies[0], .target(name: "Bar", condition: .none))
46+
XCTAssertEqual(dependencies[1], .target(name: "Baz", condition: .init(platformNames: ["linux"], config: .none)))
47+
}
48+
49+
func testConditionalTargetDependenciesDeprecation() throws {
50+
let content = """
51+
import PackageDescription
52+
let package = Package(
53+
name: "Foo",
54+
dependencies: [],
55+
targets: [
56+
.target(name: "Foo", dependencies: [
57+
.target(name: "Bar", condition: .when(platforms: nil))
58+
]),
59+
.target(name: "Bar")
60+
]
61+
)
62+
"""
63+
64+
let observability = ObservabilitySystem.makeForTesting()
65+
XCTAssertThrowsError(try loadManifest(content, observabilityScope: observability.topScope), "expected error") { error in
66+
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
67+
XCTAssertMatch(error, .contains("when(platforms:)' was obsoleted"))
68+
} else {
69+
XCTFail("unexpected error: \(error)")
70+
}
71+
}
72+
}
2073
}

0 commit comments

Comments
 (0)