Skip to content

Commit 9bbf042

Browse files
xedinxedin
authored
[6.2] Fixes a few issues with swift package {migrate, add-setting} commands (#8812)
Cherry-pick of #8800, #8806 ### Motivation: Fixes for the following problems: - Sometimes if the previous build was unsuccessful subsequent `swift package migrate` invocation fails with `error: did not compute a build plan yet`. - Implicitly generated targets (currently only test targets) are included in migration which results in extraneous warnings and failures (i.e. to update the manifest). The inconsistency in argument formatting came up as part of SE-0486 review. This is the first step in a direction to make command formatting more consistent. - `swift package migrate`: - Rename `--targets` to `--target` - Change `--target` and `--to-feature` to accept space separated argument lists - `swift package -add-setting`: - Switch to `.upToNextOption` formatting. Instead of `--swift A=B --swift C=D` switch to `--swift A=B C=D`. ### Modifications: - Update `Migrate` command's `createBuildSystem` API to avoid build manifest caching. That ensures that features in migration mode are never persistent and requires a fresh build plan which avoid issues with previous builds. - Make it possible to mark a `Module` as `implicit` i.e. when it's a synthesized/discovered test target or a system module. - Update `Migrate` command to filter `implicit` modules when no targets are specified by the user. - Update `MigrateOptions` to use `.upToNextOption` for targets and features. - Update `SwiftPackageCommand`'s `_swiftSettings` option to use `.upToNextOption` ### Result: `swift package migrate` is made more stable with fewer unactionable warnings and errors and the command line is now shorter and has consistent formatting. Resolves: rdar://152687586 Resolves: rdar://152689053 Resolves: rdar://152687084
1 parent 3a2ab4d commit 9bbf042

File tree

15 files changed

+161
-44
lines changed

15 files changed

+161
-44
lines changed

‎Sources/Build/BuildPlan/BuildPlan+Test.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ extension BuildPlan {
102102
dependencies: testProduct.underlying.modules.map { .module($0, conditions: []) },
103103
packageAccess: true, // test target is allowed access to package decls by default
104104
testDiscoverySrc: Sources(paths: discoveryPaths, root: discoveryDerivedDir),
105-
buildSettings: discoveryBuildSettings
105+
buildSettings: discoveryBuildSettings,
106+
implicit: true
106107
)
107108
let discoveryResolvedModule = ResolvedModule(
108109
packageIdentity: testProduct.packageIdentity,
@@ -287,7 +288,8 @@ private extension PackageModel.SwiftModule {
287288
dependencies: dependencies,
288289
packageAccess: packageAccess,
289290
buildSettings: buildSettings,
290-
usesUnsafeFlags: false
291+
usesUnsafeFlags: false,
292+
implicit: true
291293
)
292294
}
293295
}

‎Sources/Commands/PackageCommands/AddSetting.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ extension SwiftPackageCommand {
4444

4545
@Option(
4646
name: .customLong("swift"),
47-
parsing: .unconditionalSingleValue,
47+
parsing: .upToNextOption,
4848
help: "The Swift language setting(s) to add. Supported settings: \(SwiftSetting.allCases.map(\.rawValue).joined(separator: ", "))."
4949
)
5050
var _swiftSettings: [String]

‎Sources/Commands/PackageCommands/Migrate.swift

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import CoreCommands
1919

2020
import Foundation
2121

22+
import OrderedCollections
23+
2224
import PackageGraph
2325
import PackageModel
2426

@@ -29,18 +31,15 @@ import var TSCBasic.stdoutStream
2931

3032
struct MigrateOptions: ParsableArguments {
3133
@Option(
32-
name: .customLong("targets"),
34+
name: .customLong("target"),
35+
parsing: .upToNextOption,
3336
help: "The targets to migrate to specified set of features."
3437
)
35-
var _targets: String?
36-
37-
var targets: Set<String>? {
38-
self._targets.flatMap { Set($0.components(separatedBy: ",")) }
39-
}
38+
var targets: [String] = []
4039

4140
@Option(
4241
name: .customLong("to-feature"),
43-
parsing: .unconditionalSingleValue,
42+
parsing: .upToNextOption,
4443
help: "The Swift language upcoming/experimental feature to migrate to."
4544
)
4645
var features: [String]
@@ -85,18 +84,20 @@ extension SwiftPackageCommand {
8584
features.append(feature)
8685
}
8786

87+
let uniqueTargets = OrderedSet(self.options.targets)
88+
8889
let buildSystem = try await createBuildSystem(
8990
swiftCommandState,
90-
targets: self.options.targets,
91+
targets: uniqueTargets,
9192
features: features
9293
)
9394

9495
// Next, let's build all of the individual targets or the
9596
// whole project to get diagnostic files.
9697

9798
print("> Starting the build")
98-
if let targets = self.options.targets {
99-
for target in targets {
99+
if !uniqueTargets.isEmpty {
100+
for target in uniqueTargets {
100101
try await buildSystem.build(subset: .target(target))
101102
}
102103
} else {
@@ -107,15 +108,20 @@ extension SwiftPackageCommand {
107108
let buildPlan = try buildSystem.buildPlan
108109

109110
var modules: [any ModuleBuildDescription] = []
110-
if let targets = self.options.targets {
111-
for buildDescription in buildPlan.buildModules where targets.contains(buildDescription.module.name) {
111+
if !uniqueTargets.isEmpty {
112+
for buildDescription in buildPlan.buildModules
113+
where uniqueTargets.contains(buildDescription.module.name) {
112114
modules.append(buildDescription)
113115
}
114116
} else {
115117
let graph = try await buildSystem.getPackageGraph()
116118
for buildDescription in buildPlan.buildModules
117-
where graph.isRootPackage(buildDescription.package) && buildDescription.module.type != .plugin
119+
where graph.isRootPackage(buildDescription.package)
118120
{
121+
let module = buildDescription.module
122+
guard module.type != .plugin, !module.implicit else {
123+
continue
124+
}
119125
modules.append(buildDescription)
120126
}
121127
}
@@ -176,7 +182,7 @@ extension SwiftPackageCommand {
176182

177183
private func createBuildSystem(
178184
_ swiftCommandState: SwiftCommandState,
179-
targets: Set<String>? = .none,
185+
targets: OrderedSet<String>,
180186
features: [SwiftCompilerFeature]
181187
) async throws -> BuildSystem {
182188
let toolsBuildParameters = try swiftCommandState.toolsBuildParameters
@@ -190,7 +196,7 @@ extension SwiftPackageCommand {
190196
}
191197
}
192198

193-
if let targets {
199+
if !targets.isEmpty {
194200
targets.lazy.compactMap {
195201
modulesGraph.module(for: $0)
196202
}.forEach(addFeaturesToModule)
@@ -204,6 +210,9 @@ extension SwiftPackageCommand {
204210

205211
return try await swiftCommandState.createBuildSystem(
206212
traitConfiguration: .init(),
213+
// Don't attempt to cache manifests with temporary
214+
// feature flags added just for migration purposes.
215+
cacheBuildManifest: false,
207216
productsBuildParameters: destinationBuildParameters,
208217
toolsBuildParameters: toolsBuildParameters,
209218
// command result output goes on stdout

‎Sources/PackageGraph/ModulesGraph+Loading.swift

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -680,12 +680,7 @@ private func createResolvedPackages(
680680
// Get all implicit system library dependencies in this package.
681681
let implicitSystemLibraryDeps = packageBuilder.dependencies
682682
.flatMap(\.modules)
683-
.filter {
684-
if case let systemLibrary as SystemLibraryModule = $0.module {
685-
return systemLibrary.isImplicit
686-
}
687-
return false
688-
}
683+
.filter(\.module.implicit)
689684

690685
let packageDoesNotSupportProductAliases = packageBuilder.package.doesNotSupportProductAliases
691686
let lookupByProductIDs = !packageDoesNotSupportProductAliases &&

‎Sources/PackageGraph/Resolution/ResolvedModule.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,12 @@ public struct ResolvedModule {
180180
})
181181
}
182182

183+
/// Whether this module comes from a declaration in the manifest file
184+
/// or was synthesized (i.e. some test modules are synthesized).
185+
public var implicit: Bool {
186+
self.underlying.implicit
187+
}
188+
183189
/// Create a resolved module instance.
184190
public init(
185191
packageIdentity: PackageIdentity,

‎Sources/PackageLoading/PackageBuilder.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,8 @@ public final class PackageBuilder {
10791079
declaredSwiftVersions: self.declaredSwiftVersions(),
10801080
buildSettings: buildSettings,
10811081
buildSettingsDescription: manifestTarget.settings,
1082-
usesUnsafeFlags: manifestTarget.usesUnsafeFlags
1082+
usesUnsafeFlags: manifestTarget.usesUnsafeFlags,
1083+
implicit: false
10831084
)
10841085
} else {
10851086
// It's not a Swift target, so it's a Clang target (those are the only two types of source target currently
@@ -1124,7 +1125,8 @@ public final class PackageBuilder {
11241125
dependencies: dependencies,
11251126
buildSettings: buildSettings,
11261127
buildSettingsDescription: manifestTarget.settings,
1127-
usesUnsafeFlags: manifestTarget.usesUnsafeFlags
1128+
usesUnsafeFlags: manifestTarget.usesUnsafeFlags,
1129+
implicit: false
11281130
)
11291131
}
11301132
}
@@ -1903,7 +1905,8 @@ extension PackageBuilder {
19031905
packageAccess: false,
19041906
buildSettings: buildSettings,
19051907
buildSettingsDescription: targetDescription.settings,
1906-
usesUnsafeFlags: false
1908+
usesUnsafeFlags: false,
1909+
implicit: true
19071910
)
19081911
}
19091912
}

‎Sources/PackageModel/Module/BinaryModule.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ public final class BinaryModule: Module {
4949
buildSettings: .init(),
5050
buildSettingsDescription: [],
5151
pluginUsages: [],
52-
usesUnsafeFlags: false
52+
usesUnsafeFlags: false,
53+
implicit: false
5354
)
5455
}
5556

‎Sources/PackageModel/Module/ClangModule.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public final class ClangModule: Module {
6161
dependencies: [Module.Dependency] = [],
6262
buildSettings: BuildSettings.AssignmentTable = .init(),
6363
buildSettingsDescription: [TargetBuildSettingDescription.Setting] = [],
64-
usesUnsafeFlags: Bool
64+
usesUnsafeFlags: Bool,
65+
implicit: Bool
6566
) throws {
6667
guard includeDir.isDescendantOfOrEqual(to: sources.root) else {
6768
throw StringError("\(includeDir) should be contained in the source root \(sources.root)")
@@ -86,7 +87,8 @@ public final class ClangModule: Module {
8687
buildSettings: buildSettings,
8788
buildSettingsDescription: buildSettingsDescription,
8889
pluginUsages: [],
89-
usesUnsafeFlags: usesUnsafeFlags
90+
usesUnsafeFlags: usesUnsafeFlags,
91+
implicit: implicit
9092
)
9193
}
9294
}

‎Sources/PackageModel/Module/Module.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,10 @@ public class Module {
242242
/// Whether or not this target uses any custom unsafe flags.
243243
public let usesUnsafeFlags: Bool
244244

245+
/// Whether this module comes from a declaration in the manifest file
246+
/// or was synthesized (i.e. some test modules are synthesized).
247+
public let implicit: Bool
248+
245249
init(
246250
name: String,
247251
potentialBundleName: String? = nil,
@@ -256,7 +260,8 @@ public class Module {
256260
buildSettings: BuildSettings.AssignmentTable,
257261
buildSettingsDescription: [TargetBuildSettingDescription.Setting],
258262
pluginUsages: [PluginUsage],
259-
usesUnsafeFlags: Bool
263+
usesUnsafeFlags: Bool,
264+
implicit: Bool
260265
) {
261266
self.name = name
262267
self.potentialBundleName = potentialBundleName
@@ -273,6 +278,7 @@ public class Module {
273278
self.buildSettingsDescription = buildSettingsDescription
274279
self.pluginUsages = pluginUsages
275280
self.usesUnsafeFlags = usesUnsafeFlags
281+
self.implicit = implicit
276282
}
277283
}
278284

‎Sources/PackageModel/Module/PluginModule.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ public final class PluginModule: Module {
4545
buildSettings: .init(),
4646
buildSettingsDescription: [],
4747
pluginUsages: [],
48-
usesUnsafeFlags: false
48+
usesUnsafeFlags: false,
49+
implicit: false
4950
)
5051
}
5152
}

‎Sources/PackageModel/Module/SwiftModule.swift

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ public final class SwiftModule: Module {
3333
dependencies: [Module.Dependency],
3434
packageAccess: Bool,
3535
testDiscoverySrc: Sources,
36-
buildSettings: BuildSettings.AssignmentTable = .init()) {
36+
buildSettings: BuildSettings.AssignmentTable = .init(),
37+
implicit: Bool) {
3738
self.declaredSwiftVersions = []
3839

3940
super.init(
@@ -46,7 +47,8 @@ public final class SwiftModule: Module {
4647
buildSettings: buildSettings,
4748
buildSettingsDescription: [],
4849
pluginUsages: [],
49-
usesUnsafeFlags: false
50+
usesUnsafeFlags: false,
51+
implicit: implicit
5052
)
5153
}
5254

@@ -68,7 +70,8 @@ public final class SwiftModule: Module {
6870
buildSettings: BuildSettings.AssignmentTable = .init(),
6971
buildSettingsDescription: [TargetBuildSettingDescription.Setting] = [],
7072
pluginUsages: [PluginUsage] = [],
71-
usesUnsafeFlags: Bool
73+
usesUnsafeFlags: Bool,
74+
implicit: Bool
7275
) {
7376
self.declaredSwiftVersions = declaredSwiftVersions
7477
super.init(
@@ -85,7 +88,8 @@ public final class SwiftModule: Module {
8588
buildSettings: buildSettings,
8689
buildSettingsDescription: buildSettingsDescription,
8790
pluginUsages: pluginUsages,
88-
usesUnsafeFlags: usesUnsafeFlags
91+
usesUnsafeFlags: usesUnsafeFlags,
92+
implicit: implicit
8993
)
9094
}
9195

@@ -133,7 +137,8 @@ public final class SwiftModule: Module {
133137
buildSettings: buildSettings,
134138
buildSettingsDescription: [],
135139
pluginUsages: [],
136-
usesUnsafeFlags: false
140+
usesUnsafeFlags: false,
141+
implicit: true
137142
)
138143
}
139144

‎Sources/PackageModel/Module/SystemLibraryModule.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ public final class SystemLibraryModule: Module {
2525
/// List of system package providers, if any.
2626
public let providers: [SystemPackageProviderDescription]?
2727

28-
/// True if this system library should become implicit dependency of its dependent packages.
29-
public let isImplicit: Bool
30-
3128
public init(
3229
name: String,
3330
path: AbsolutePath,
@@ -38,7 +35,6 @@ public final class SystemLibraryModule: Module {
3835
let sources = Sources(paths: [], root: path)
3936
self.pkgConfig = pkgConfig
4037
self.providers = providers
41-
self.isImplicit = isImplicit
4238
super.init(
4339
name: name,
4440
type: .systemModule,
@@ -49,7 +45,8 @@ public final class SystemLibraryModule: Module {
4945
buildSettings: .init(),
5046
buildSettingsDescription: [],
5147
pluginUsages: [],
52-
usesUnsafeFlags: false
48+
usesUnsafeFlags: false,
49+
implicit: isImplicit
5350
)
5451
}
5552
}

‎Sources/_InternalTestSupport/ResolvedModule+Mock.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ extension ResolvedModule {
2929
sources: Sources(paths: [], root: "/"),
3030
dependencies: [],
3131
packageAccess: false,
32-
usesUnsafeFlags: false
32+
usesUnsafeFlags: false,
33+
implicit: true
3334
),
3435
dependencies: deps.map { .module($0, conditions: conditions) },
3536
defaultLocalization: nil,

0 commit comments

Comments
 (0)