From e8f28461ec543d85c4e61b9505e0efc263ac8196 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 10 Jun 2025 13:18:48 -0700 Subject: [PATCH 1/3] [Commands] Migrate: Don't attempt to cache manifest with temporary feature flags The build system requested by the `swift package migrate` command is transitory and shouldn't be persisted. This would also make sure that the build plan is always available regardless of what builds happened before `swift package migrate` invocation. Resolves: rdar://152687084 --- Sources/Commands/PackageCommands/Migrate.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/Commands/PackageCommands/Migrate.swift b/Sources/Commands/PackageCommands/Migrate.swift index ff8aac0019b..3e4bb933b30 100644 --- a/Sources/Commands/PackageCommands/Migrate.swift +++ b/Sources/Commands/PackageCommands/Migrate.swift @@ -204,6 +204,9 @@ extension SwiftPackageCommand { return try await swiftCommandState.createBuildSystem( traitConfiguration: .init(), + // Don't attempt to cache manifests with temporary + // feature flags added just for migration purposes. + cacheBuildManifest: false, productsBuildParameters: destinationBuildParameters, toolsBuildParameters: toolsBuildParameters, // command result output goes on stdout From c2b15ffdf490331b5562add96edae854750467c1 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 11 Jun 2025 00:14:02 -0700 Subject: [PATCH 2/3] [Package{Model, Graph}] Make it possible to mark modules as implicit It's currently impossible to distringuish between modules that are created from the declarations in a manifest and the ones synthesized as part of module graph building and/or build plan creation. This was only a problem for `SystemLibraryModule` before, but with the introduction of `swift package migrate` command the situation changed because the command can only operate on modules that appear in the manifest file because it won't be possible to update code or settings of the synthesized modules. --- Sources/Build/BuildPlan/BuildPlan+Test.swift | 6 +- .../PackageGraph/ModulesGraph+Loading.swift | 7 +- .../Resolution/ResolvedModule.swift | 6 ++ Sources/PackageLoading/PackageBuilder.swift | 9 +- .../PackageModel/Module/BinaryModule.swift | 3 +- Sources/PackageModel/Module/ClangModule.swift | 6 +- Sources/PackageModel/Module/Module.swift | 8 +- .../PackageModel/Module/PluginModule.swift | 3 +- Sources/PackageModel/Module/SwiftModule.swift | 15 ++-- .../Module/SystemLibraryModule.swift | 7 +- .../ResolvedModule+Mock.swift | 3 +- Tests/BuildTests/BuildPlanTests.swift | 88 +++++++++++++++++++ .../ClangTargetBuildDescriptionTests.swift | 3 +- 13 files changed, 136 insertions(+), 28 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan+Test.swift b/Sources/Build/BuildPlan/BuildPlan+Test.swift index 8bb0042761f..545ecb2702f 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Test.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Test.swift @@ -102,7 +102,8 @@ extension BuildPlan { dependencies: testProduct.underlying.modules.map { .module($0, conditions: []) }, packageAccess: true, // test target is allowed access to package decls by default testDiscoverySrc: Sources(paths: discoveryPaths, root: discoveryDerivedDir), - buildSettings: discoveryBuildSettings + buildSettings: discoveryBuildSettings, + implicit: true ) let discoveryResolvedModule = ResolvedModule( packageIdentity: testProduct.packageIdentity, @@ -287,7 +288,8 @@ private extension PackageModel.SwiftModule { dependencies: dependencies, packageAccess: packageAccess, buildSettings: buildSettings, - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: true ) } } diff --git a/Sources/PackageGraph/ModulesGraph+Loading.swift b/Sources/PackageGraph/ModulesGraph+Loading.swift index fb4c3a1bc7c..507fcad87d7 100644 --- a/Sources/PackageGraph/ModulesGraph+Loading.swift +++ b/Sources/PackageGraph/ModulesGraph+Loading.swift @@ -680,12 +680,7 @@ private func createResolvedPackages( // Get all implicit system library dependencies in this package. let implicitSystemLibraryDeps = packageBuilder.dependencies .flatMap(\.modules) - .filter { - if case let systemLibrary as SystemLibraryModule = $0.module { - return systemLibrary.isImplicit - } - return false - } + .filter(\.module.implicit) let packageDoesNotSupportProductAliases = packageBuilder.package.doesNotSupportProductAliases let lookupByProductIDs = !packageDoesNotSupportProductAliases && diff --git a/Sources/PackageGraph/Resolution/ResolvedModule.swift b/Sources/PackageGraph/Resolution/ResolvedModule.swift index 5f4aec5625c..786a61b7c28 100644 --- a/Sources/PackageGraph/Resolution/ResolvedModule.swift +++ b/Sources/PackageGraph/Resolution/ResolvedModule.swift @@ -180,6 +180,12 @@ public struct ResolvedModule { }) } + /// Whether this module comes from a declaration in the manifest file + /// or was synthesized (i.e. some test modules are synthesized). + public var implicit: Bool { + self.underlying.implicit + } + /// Create a resolved module instance. public init( packageIdentity: PackageIdentity, diff --git a/Sources/PackageLoading/PackageBuilder.swift b/Sources/PackageLoading/PackageBuilder.swift index 880a3da3dae..cc853edc979 100644 --- a/Sources/PackageLoading/PackageBuilder.swift +++ b/Sources/PackageLoading/PackageBuilder.swift @@ -1079,7 +1079,8 @@ public final class PackageBuilder { declaredSwiftVersions: self.declaredSwiftVersions(), buildSettings: buildSettings, buildSettingsDescription: manifestTarget.settings, - usesUnsafeFlags: manifestTarget.usesUnsafeFlags + usesUnsafeFlags: manifestTarget.usesUnsafeFlags, + implicit: false ) } else { // 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 { dependencies: dependencies, buildSettings: buildSettings, buildSettingsDescription: manifestTarget.settings, - usesUnsafeFlags: manifestTarget.usesUnsafeFlags + usesUnsafeFlags: manifestTarget.usesUnsafeFlags, + implicit: false ) } } @@ -1903,7 +1905,8 @@ extension PackageBuilder { packageAccess: false, buildSettings: buildSettings, buildSettingsDescription: targetDescription.settings, - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: true ) } } diff --git a/Sources/PackageModel/Module/BinaryModule.swift b/Sources/PackageModel/Module/BinaryModule.swift index ec5ea395727..31d47fbb948 100644 --- a/Sources/PackageModel/Module/BinaryModule.swift +++ b/Sources/PackageModel/Module/BinaryModule.swift @@ -49,7 +49,8 @@ public final class BinaryModule: Module { buildSettings: .init(), buildSettingsDescription: [], pluginUsages: [], - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: false ) } diff --git a/Sources/PackageModel/Module/ClangModule.swift b/Sources/PackageModel/Module/ClangModule.swift index bf066a6ecea..5105b7bb563 100644 --- a/Sources/PackageModel/Module/ClangModule.swift +++ b/Sources/PackageModel/Module/ClangModule.swift @@ -61,7 +61,8 @@ public final class ClangModule: Module { dependencies: [Module.Dependency] = [], buildSettings: BuildSettings.AssignmentTable = .init(), buildSettingsDescription: [TargetBuildSettingDescription.Setting] = [], - usesUnsafeFlags: Bool + usesUnsafeFlags: Bool, + implicit: Bool ) throws { guard includeDir.isDescendantOfOrEqual(to: sources.root) else { throw StringError("\(includeDir) should be contained in the source root \(sources.root)") @@ -86,7 +87,8 @@ public final class ClangModule: Module { buildSettings: buildSettings, buildSettingsDescription: buildSettingsDescription, pluginUsages: [], - usesUnsafeFlags: usesUnsafeFlags + usesUnsafeFlags: usesUnsafeFlags, + implicit: implicit ) } } diff --git a/Sources/PackageModel/Module/Module.swift b/Sources/PackageModel/Module/Module.swift index 39ea5c891ab..34d6f72c8f8 100644 --- a/Sources/PackageModel/Module/Module.swift +++ b/Sources/PackageModel/Module/Module.swift @@ -242,6 +242,10 @@ public class Module { /// Whether or not this target uses any custom unsafe flags. public let usesUnsafeFlags: Bool + /// Whether this module comes from a declaration in the manifest file + /// or was synthesized (i.e. some test modules are synthesized). + public let implicit: Bool + init( name: String, potentialBundleName: String? = nil, @@ -256,7 +260,8 @@ public class Module { buildSettings: BuildSettings.AssignmentTable, buildSettingsDescription: [TargetBuildSettingDescription.Setting], pluginUsages: [PluginUsage], - usesUnsafeFlags: Bool + usesUnsafeFlags: Bool, + implicit: Bool ) { self.name = name self.potentialBundleName = potentialBundleName @@ -273,6 +278,7 @@ public class Module { self.buildSettingsDescription = buildSettingsDescription self.pluginUsages = pluginUsages self.usesUnsafeFlags = usesUnsafeFlags + self.implicit = implicit } } diff --git a/Sources/PackageModel/Module/PluginModule.swift b/Sources/PackageModel/Module/PluginModule.swift index 94c5d8eed1c..ec180285b93 100644 --- a/Sources/PackageModel/Module/PluginModule.swift +++ b/Sources/PackageModel/Module/PluginModule.swift @@ -45,7 +45,8 @@ public final class PluginModule: Module { buildSettings: .init(), buildSettingsDescription: [], pluginUsages: [], - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: false ) } } diff --git a/Sources/PackageModel/Module/SwiftModule.swift b/Sources/PackageModel/Module/SwiftModule.swift index 93b083b1ebf..1fbb2a257d4 100644 --- a/Sources/PackageModel/Module/SwiftModule.swift +++ b/Sources/PackageModel/Module/SwiftModule.swift @@ -33,7 +33,8 @@ public final class SwiftModule: Module { dependencies: [Module.Dependency], packageAccess: Bool, testDiscoverySrc: Sources, - buildSettings: BuildSettings.AssignmentTable = .init()) { + buildSettings: BuildSettings.AssignmentTable = .init(), + implicit: Bool) { self.declaredSwiftVersions = [] super.init( @@ -46,7 +47,8 @@ public final class SwiftModule: Module { buildSettings: buildSettings, buildSettingsDescription: [], pluginUsages: [], - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: implicit ) } @@ -68,7 +70,8 @@ public final class SwiftModule: Module { buildSettings: BuildSettings.AssignmentTable = .init(), buildSettingsDescription: [TargetBuildSettingDescription.Setting] = [], pluginUsages: [PluginUsage] = [], - usesUnsafeFlags: Bool + usesUnsafeFlags: Bool, + implicit: Bool ) { self.declaredSwiftVersions = declaredSwiftVersions super.init( @@ -85,7 +88,8 @@ public final class SwiftModule: Module { buildSettings: buildSettings, buildSettingsDescription: buildSettingsDescription, pluginUsages: pluginUsages, - usesUnsafeFlags: usesUnsafeFlags + usesUnsafeFlags: usesUnsafeFlags, + implicit: implicit ) } @@ -133,7 +137,8 @@ public final class SwiftModule: Module { buildSettings: buildSettings, buildSettingsDescription: [], pluginUsages: [], - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: true ) } diff --git a/Sources/PackageModel/Module/SystemLibraryModule.swift b/Sources/PackageModel/Module/SystemLibraryModule.swift index b1d17abcbac..f9a42d2e51a 100644 --- a/Sources/PackageModel/Module/SystemLibraryModule.swift +++ b/Sources/PackageModel/Module/SystemLibraryModule.swift @@ -25,9 +25,6 @@ public final class SystemLibraryModule: Module { /// List of system package providers, if any. public let providers: [SystemPackageProviderDescription]? - /// True if this system library should become implicit dependency of its dependent packages. - public let isImplicit: Bool - public init( name: String, path: AbsolutePath, @@ -38,7 +35,6 @@ public final class SystemLibraryModule: Module { let sources = Sources(paths: [], root: path) self.pkgConfig = pkgConfig self.providers = providers - self.isImplicit = isImplicit super.init( name: name, type: .systemModule, @@ -49,7 +45,8 @@ public final class SystemLibraryModule: Module { buildSettings: .init(), buildSettingsDescription: [], pluginUsages: [], - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: isImplicit ) } } diff --git a/Sources/_InternalTestSupport/ResolvedModule+Mock.swift b/Sources/_InternalTestSupport/ResolvedModule+Mock.swift index 24df377acbb..aa2818d49e0 100644 --- a/Sources/_InternalTestSupport/ResolvedModule+Mock.swift +++ b/Sources/_InternalTestSupport/ResolvedModule+Mock.swift @@ -29,7 +29,8 @@ extension ResolvedModule { sources: Sources(paths: [], root: "/"), dependencies: [], packageAccess: false, - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: true ), dependencies: deps.map { .module($0, conditions: conditions) }, defaultLocalization: nil, diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index 576065d8c9f..058b580586c 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -7435,6 +7435,94 @@ class BuildPlanTestCase: BuildSystemProviderTestCase { } } } + + func testImplicitModules() async throws { + let fileSystem = InMemoryFileSystem( + emptyFiles: + "/A/Sources/ATarget/foo.swift", + "/A/Sources/AMacro/macro.swift", + "/A/Sources/AExecutable/main.swift", + "/A/Sources/ASystemLib/module.modulemap", + "/A/Plugins/APlugin/main.swift", + "/A/Tests/ATargetTests/foo.swift" + ) + + let observability = ObservabilitySystem.makeForTesting() + let graph = try loadModulesGraph( + fileSystem: fileSystem, + manifests: [ + Manifest.createRootManifest( + displayName: "A", + path: "/A", + dependencies: [ + ], + targets: [ + TargetDescription(name: "ATarget"), + TargetDescription( + name: "AMacro", + dependencies: [], + type: .`macro` + ), + TargetDescription( + name: "AExecutable", + dependencies: ["ATarget"], + type: .executable + ), + TargetDescription( + name: "APlugin", + type: .plugin, + pluginCapability: .buildTool + ), + TargetDescription( + name: "ASystemLib", + type: .system + ), + TargetDescription( + name: "ATargetTests", + dependencies: ["ATarget"], + type: .test + ), + ] + ), + ], + observabilityScope: observability.topScope + ) + XCTAssertNoDiagnostics(observability.diagnostics) + + let result = try await BuildPlanResult(plan: mockBuildPlan( + graph: graph, + fileSystem: fileSystem, + observabilityScope: observability.topScope + )) + + struct ExpectedTarget: Hashable, Equatable { + let name: String + let implicit: Bool + } + + var expectedTargets: Set = [ + .init(name: "ATarget", implicit: false), + .init(name: "AMacro", implicit: false), + .init(name: "AExecutable", implicit: false), + .init(name: "ATargetTests", implicit: false), + .init(name: "APackageTests", implicit: true), + ] + #if !os(macOS) + expectedTargets.insert(.init(name: "APackageDiscoveredTests", implicit: true)) + #endif + XCTAssertEqual( + Set(result.targetMap.map { ExpectedTarget(name: $0.module.name, implicit: $0.module.implicit) }), + expectedTargets + ) + XCTAssertEqual( + result.plan.graph.module(for: "APlugin")?.implicit, + false + ) + XCTAssertEqual( + result.plan.graph.module(for: "ASystemLib")?.implicit, + false + ) + } } class BuildPlanNativeTests: BuildPlanTestCase { diff --git a/Tests/BuildTests/ClangTargetBuildDescriptionTests.swift b/Tests/BuildTests/ClangTargetBuildDescriptionTests.swift index 317bd624c95..493f3db64df 100644 --- a/Tests/BuildTests/ClangTargetBuildDescriptionTests.swift +++ b/Tests/BuildTests/ClangTargetBuildDescriptionTests.swift @@ -61,7 +61,8 @@ final class ClangTargetBuildDescriptionTests: XCTestCase { type: .library, path: .root, sources: .init(paths: [.root.appending(component: "foo.c")], root: .root), - usesUnsafeFlags: false + usesUnsafeFlags: false, + implicit: true ) } From 273f390e365917a8421c065abd8ccedd3145a22c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 11 Jun 2025 14:17:08 -0700 Subject: [PATCH 3/3] [Commands] Migrate: Exclude implicit modules from migration Fixes and setting updates to such modules cannot be applied. Resolves: rdar://152689053 --- Sources/Commands/PackageCommands/Migrate.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Commands/PackageCommands/Migrate.swift b/Sources/Commands/PackageCommands/Migrate.swift index 3e4bb933b30..b54e2b1081e 100644 --- a/Sources/Commands/PackageCommands/Migrate.swift +++ b/Sources/Commands/PackageCommands/Migrate.swift @@ -114,8 +114,12 @@ extension SwiftPackageCommand { } else { let graph = try await buildSystem.getPackageGraph() for buildDescription in buildPlan.buildModules - where graph.isRootPackage(buildDescription.package) && buildDescription.module.type != .plugin + where graph.isRootPackage(buildDescription.package) { + let module = buildDescription.module + guard module.type != .plugin, !module.implicit else { + continue + } modules.append(buildDescription) } }