From d6db4df2be1e16db4e7e3f5c50b02fd050af7ad3 Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Thu, 17 Jul 2025 11:25:25 -0400 Subject: [PATCH 1/2] Create tests cases to reproduce the module aliasing problem --- .../DirectDeps3/AppPkg/Package.swift | 29 +++++++++++++++++++ .../DirectDeps3/AppPkg/Sources/App/main.swift | 20 +++++++++++++ .../AppPkg/Sources/Utils/FileUtils.swift | 13 +++++++++ .../DirectDeps3/UtilsPkg/Package.swift | 14 +++++++++ .../UtilsPkg/Sources/Lib/Library.swift | 5 ++++ .../UtilsPkg/Sources/Utils/FileUtils.swift | 15 ++++++++++ .../NestedDeps3/AppPkg/Package.swift | 20 +++++++++++++ .../NestedDeps3/AppPkg/Sources/App/main.swift | 5 ++++ .../NestedDeps3/XPkg/Package.swift | 18 ++++++++++++ .../NestedDeps3/XPkg/Sources/Utils/File.swift | 3 ++ .../NestedDeps3/XPkg/Sources/X/File.swift | 6 ++++ .../ModuleAliasingFixtureTests.swift | 23 +++++++++++++++ 12 files changed, 171 insertions(+) create mode 100644 Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Package.swift create mode 100644 Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/App/main.swift create mode 100644 Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/Utils/FileUtils.swift create mode 100644 Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift create mode 100644 Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Lib/Library.swift create mode 100644 Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Utils/FileUtils.swift create mode 100644 Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Package.swift create mode 100644 Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Sources/App/main.swift create mode 100644 Fixtures/ModuleAliasing/NestedDeps3/XPkg/Package.swift create mode 100644 Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/Utils/File.swift create mode 100644 Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/X/File.swift diff --git a/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Package.swift b/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Package.swift new file mode 100644 index 00000000000..f66bac158ff --- /dev/null +++ b/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Package.swift @@ -0,0 +1,29 @@ +// swift-tools-version:5.7 +import PackageDescription + +let package = Package( + name: "AppPkg", + dependencies: [ + .package(path: "../UtilsPkg"), + ], + targets: [ + .executableTarget( + name: "App", + dependencies: [ + "Utils", + .product(name: "Lib", + package: "UtilsPkg", + moduleAliases: [ + "Utils": "GameUtils", + "Lib": "GameLib", + ]), + ], + path: "./Sources/App"), + .target( + name: "Utils", + dependencies: [], + path: "./Sources/Utils" + ) + ] + ) + diff --git a/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/App/main.swift b/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/App/main.swift new file mode 100644 index 00000000000..6f2240c1240 --- /dev/null +++ b/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/App/main.swift @@ -0,0 +1,20 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See http://swift.org/LICENSE.txt for license information + See http://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +import Utils +import GameUtils +import GameLib + +Utils.echoModule() + +GameLib.userHelp() + +let level = LevelDetector.detect(for: "TestUser") +print(level) diff --git a/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/Utils/FileUtils.swift b/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/Utils/FileUtils.swift new file mode 100644 index 00000000000..7dfbf02100c --- /dev/null +++ b/Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/Utils/FileUtils.swift @@ -0,0 +1,13 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See http://swift.org/LICENSE.txt for license information + See http://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +public func echoModule() { + print("Utils") +} diff --git a/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift new file mode 100644 index 00000000000..c0bd262bc14 --- /dev/null +++ b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift @@ -0,0 +1,14 @@ +// swift-tools-version:5.5 +import PackageDescription + +let package = Package( + name: "UtilsPkg", + products: [ + .library(name: "Utils", targets: ["Utils"]), + .library(name: "Lib", targets: ["Lib", "Utils"]) + ], + targets: [ + .target(name: "Utils", dependencies: []), + .target(name: "Lib", dependencies: ["Utils"]) + ] +) diff --git a/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Lib/Library.swift b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Lib/Library.swift new file mode 100644 index 00000000000..d22e816119f --- /dev/null +++ b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Lib/Library.swift @@ -0,0 +1,5 @@ +import Utils + +public func userHelp() { + _ = Utils.LevelDetector.detect(for: "someUser") +} \ No newline at end of file diff --git a/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Utils/FileUtils.swift b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Utils/FileUtils.swift new file mode 100644 index 00000000000..a8e3a9ae04d --- /dev/null +++ b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Utils/FileUtils.swift @@ -0,0 +1,15 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See http://swift.org/LICENSE.txt for license information + See http://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +public struct LevelDetector: Equatable { + public static func detect(for user: String) -> Int { + return user.count + } +} diff --git a/Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Package.swift b/Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Package.swift new file mode 100644 index 00000000000..938249b1a2c --- /dev/null +++ b/Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Package.swift @@ -0,0 +1,20 @@ +// swift-tools-version:5.7 +import PackageDescription + +let package = Package( + name: "AppPkg", + dependencies: [ + .package(path: "../XPkg"), + ], + targets: [ + .executableTarget( + name: "App", + dependencies: [ + .product(name: "X", + package: "XPkg", + moduleAliases: ["Utils": "XUtils", "X": "XNew"] + ) + ]), + ] +) + diff --git a/Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Sources/App/main.swift b/Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Sources/App/main.swift new file mode 100644 index 00000000000..54c32cfbe63 --- /dev/null +++ b/Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Sources/App/main.swift @@ -0,0 +1,5 @@ +import XNew +import XUtils + +utilsInX() +XNew.funcInX() diff --git a/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Package.swift b/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Package.swift new file mode 100644 index 00000000000..1ba8dd6b1e9 --- /dev/null +++ b/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Package.swift @@ -0,0 +1,18 @@ +// swift-tools-version:5.7 +// The swift-tools-version declares the minimum version of Swift required to build this package. + +import PackageDescription + +let package = Package( + name: "XPkg", + products: [ + .library(name: "X", targets: ["X"]), + ], + targets: [ + .target(name: "X", + dependencies: [ + "Utils", + ]), + .target(name: "Utils", dependencies: []) + ] +) diff --git a/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/Utils/File.swift b/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/Utils/File.swift new file mode 100644 index 00000000000..9e6c16340c4 --- /dev/null +++ b/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/Utils/File.swift @@ -0,0 +1,3 @@ +public func utilsInX() { + print("utils in X") +} diff --git a/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/X/File.swift b/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/X/File.swift new file mode 100644 index 00000000000..33866191685 --- /dev/null +++ b/Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/X/File.swift @@ -0,0 +1,6 @@ +import Utils + +public func funcInX() { + print("func in X") + utilsInX() +} diff --git a/Tests/FunctionalTests/ModuleAliasingFixtureTests.swift b/Tests/FunctionalTests/ModuleAliasingFixtureTests.swift index 77c180cfc58..5b7d19cb94b 100644 --- a/Tests/FunctionalTests/ModuleAliasingFixtureTests.swift +++ b/Tests/FunctionalTests/ModuleAliasingFixtureTests.swift @@ -42,6 +42,19 @@ final class ModuleAliasingFixtureTests: XCTestCase { } } + func testModuleDirectDeps3() async throws { + try await fixture(name: "ModuleAliasing/DirectDeps3") { fixturePath in + let pkgPath = fixturePath.appending(components: "AppPkg") + let buildPath = pkgPath.appending(components: ".build", try UserToolchain.default.targetTriple.platformBuildPathComponent, "debug") + await XCTAssertBuilds(pkgPath, extraArgs: ["--vv"]) + XCTAssertFileExists(buildPath.appending(components: executableName("App"))) + XCTAssertFileExists(buildPath.appending(components: "Modules", "GameUtils.swiftmodule")) + XCTAssertFileExists(buildPath.appending(components: "Modules", "GameLib.swiftmodule")) + XCTAssertFileExists(buildPath.appending(components: "Modules", "Utils.swiftmodule")) + _ = try await SwiftPM.Build.execute(packagePath: pkgPath) + } + } + func testModuleNestedDeps1() async throws { try await fixture(name: "ModuleAliasing/NestedDeps1") { fixturePath in let pkgPath = fixturePath.appending(components: "AppPkg") @@ -71,4 +84,14 @@ final class ModuleAliasingFixtureTests: XCTestCase { _ = try await SwiftPM.Build.execute(packagePath: pkgPath) } } + + func testModuleNestedDeps3() async throws { + try await fixture(name: "ModuleAliasing/NestedDeps3") { fixturePath in + let pkgPath = fixturePath.appending(components: "AppPkg") + let buildPath = pkgPath.appending(components: ".build", try UserToolchain.default.targetTriple.platformBuildPathComponent, "debug") + await XCTAssertBuilds(pkgPath, extraArgs: ["--vv"]) + XCTAssertFileExists(buildPath.appending(components: executableName("App"))) + _ = try await SwiftPM.Build.execute(packagePath: pkgPath) + } + } } From 686320e748534d438fc4c2d99a0d68854f1a3527 Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Mon, 21 Jul 2025 15:35:43 -0400 Subject: [PATCH 2/2] Fill in the module alias even if there are unaliased product modules Avoid adding aliases to root package product modules since they establish the physical name of the upstream modules --- .../DirectDeps3/UtilsPkg/Package.swift | 1 - Sources/PackageGraph/ModuleAliasTracker.swift | 85 ++++++++++++------- .../BuildTests/ModuleAliasingBuildTests.swift | 15 ++-- 3 files changed, 62 insertions(+), 39 deletions(-) diff --git a/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift index c0bd262bc14..4aa421e3ea4 100644 --- a/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift +++ b/Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift @@ -4,7 +4,6 @@ import PackageDescription let package = Package( name: "UtilsPkg", products: [ - .library(name: "Utils", targets: ["Utils"]), .library(name: "Lib", targets: ["Lib", "Utils"]) ], targets: [ diff --git a/Sources/PackageGraph/ModuleAliasTracker.swift b/Sources/PackageGraph/ModuleAliasTracker.swift index bb080fb424a..cb17646686e 100644 --- a/Sources/PackageGraph/ModuleAliasTracker.swift +++ b/Sources/PackageGraph/ModuleAliasTracker.swift @@ -16,22 +16,39 @@ import Basics // This is a helper class that tracks module aliases in a package dependency graph // and handles overriding upstream aliases where aliases themselves conflict. struct ModuleAliasTracker { + /// Map product identifier to a list of aliases that are associated through product dependencies on it. fileprivate var aliasMap = [String: [ModuleAliasModel]]() + + /// Map a package and its product identifier to a list of aliases that are associated through product dependencies on it. fileprivate var idToAliasMap = [PackageIdentity: [String: [ModuleAliasModel]]]() + + /// Map from a package + product identifier to the modules that are included in a product + ones that are one level dependency away var idToProductToAllModules = [PackageIdentity: [String: [Module]]]() + + /// Map from a product identifier to the modules that are directly included in the product declaration. var productToDirectModules = [String: [Module]]() + + /// Map from a product identifier to the modules that are included in a product + ones that are one level dependency way var productToAllModules = [String: [Module]]() + + /// Map parent product identifier to any product identifier that it depends on transitively. var parentToChildProducts = [String: [String]]() + + // Parent/child relationship DAG through product dependencies var parentToChildIDs = [PackageIdentity: [PackageIdentity]]() var childToParentID = [PackageIdentity: PackageIdentity]() + var appliedAliases = Set() init() {} + + /// * Analyze the dependencies of the modules from origin package and build a DAG of packages. + /// * Record the module aliases mutating func addModuleAliases(modules: [Module], package: PackageIdentity) throws { let moduleDependencies = modules.flatMap(\.dependencies) for dep in moduleDependencies { - if case let .product(productRef, _) = dep, - let productPkg = productRef.package { + if case let .product(productRef, _) = dep { + if let productPkg = productRef.package { let productPkgID = PackageIdentity.plain(productPkg) // Track dependency package ID chain addPackageIDChain(parent: package, child: productPkgID) @@ -43,6 +60,7 @@ struct ModuleAliasTracker { originPackage: productPkgID, consumingPackage: package) } + } } } } @@ -93,27 +111,15 @@ struct ModuleAliasTracker { } } + // FIXME this isn't all modules in a product. This is just the dependencies that are one level away from the modules that compose the product and the composing modules themselves. var allModulesInProduct = moduleDeps.compactMap(\.module) allModulesInProduct.append(contentsOf: product.modules) idToProductToAllModules[package, default: [:]][product.identity] = allModulesInProduct + productToDirectModules[product.identity] = product.modules productToAllModules[product.identity] = allModulesInProduct } - func validateAndApplyAliases(product: Product, - package: PackageIdentity, - observabilityScope: ObservabilityScope) throws { - guard let modules = idToProductToAllModules[package]?[product.identity] else { return } - let modulesWithAliases = modules.filter{ $0.moduleAliases != nil } - for moduleWithAlias in modulesWithAliases { - if moduleWithAlias.sources.containsNonSwiftFiles { - let aliasesMsg = moduleWithAlias.moduleAliases?.map{"'\($0.key)' as '\($0.value)'"}.joined(separator: ", ") ?? "" - observabilityScope.emit(warning: "target '\(moduleWithAlias.name)' for product '\(product.name)' from package '\(package.description)' has module aliases: [\(aliasesMsg)] but may contain non-Swift sources; there might be a conflict among non-Swift symbols") - } - moduleWithAlias.applyAlias() - } - } - mutating func propagateAliases(observabilityScope: ObservabilityScope) { // First get the root package ID var pkgID = childToParentID.first?.key @@ -128,13 +134,14 @@ struct ModuleAliasTracker { if let productToAllModules = idToProductToAllModules[rootPkg] { // First, propagate aliases upstream for productID in productToAllModules.keys { + // Alias buffer tracks the most downstream alias for each original module name var aliasBuffer = [String: ModuleAliasModel]() propagate(productID: productID, observabilityScope: observabilityScope, aliasBuffer: &aliasBuffer) } // Then, merge or override upstream aliases downwards for productID in productToAllModules.keys { - merge(productID: productID, observabilityScope: observabilityScope) + merge(productID: productID, observabilityScope: observabilityScope, root: true) } } // Finally, fill in aliases for modules in products that are in the @@ -198,14 +205,15 @@ struct ModuleAliasTracker { } } - // Merge all the upstream aliases and override them if necessary - mutating func merge(productID: String, observabilityScope: ObservabilityScope) { + // Merge all the upstream aliases and override them if necessary (depth-first). + mutating func merge(productID: String, observabilityScope: ObservabilityScope, root: Bool) { guard let children = parentToChildProducts[productID] else { return } for childID in children { merge(productID: childID, - observabilityScope: observabilityScope) + observabilityScope: observabilityScope, + root: false) } if let curDirectModules = productToDirectModules[productID] { @@ -232,10 +240,10 @@ struct ModuleAliasTracker { let depProdModuleAliases = depProdModule.moduleAliases ?? [:] for (key, val) in depProdModuleAliases { var shouldAddAliases = false - if depProdModule.name == key { - shouldAddAliases = true - } else if !depProductModules.map({$0.name}).contains(key) { - shouldAddAliases = true + if depProdModule.name == key && !root { + shouldAddAliases = true + } else if !depProductModules.map({$0.name}).contains(key) && !root { + shouldAddAliases = true } if shouldAddAliases { if depProductAliases[key]?.contains(val) ?? false { @@ -246,6 +254,7 @@ struct ModuleAliasTracker { } } } + chainModuleAliases(modules: curDirectModules, checkedModules: relevantModules, moduleAliases: moduleAliases, @@ -263,14 +272,11 @@ struct ModuleAliasTracker { mutating func fillInRest(package: PackageIdentity) { if let productToModules = idToProductToAllModules[package] { for (_, productModules) in productToModules { - let unAliased = productModules.contains { $0.moduleAliases == nil } - if unAliased { - for module in productModules { - let depAliases = module.recursiveDependentModules.compactMap{$0.moduleAliases}.flatMap{$0} - for (key, alias) in depAliases { - appliedAliases.insert(key) - module.addModuleAlias(for: key, as: alias) - } + for module in productModules { + let depAliases = module.recursiveDependentModules.compactMap{$0.moduleAliases}.flatMap{$0} + for (key, alias) in depAliases { + appliedAliases.insert(key) + module.addModuleAlias(for: key, as: alias) } } } @@ -281,6 +287,20 @@ struct ModuleAliasTracker { } } + func validateAndApplyAliases(product: Product, + package: PackageIdentity, + observabilityScope: ObservabilityScope) throws { + guard let modules = idToProductToAllModules[package]?[product.identity] else { return } + let modulesWithAliases = modules.filter{ $0.moduleAliases != nil } + for moduleWithAlias in modulesWithAliases { + if moduleWithAlias.sources.containsNonSwiftFiles { + let aliasesMsg = moduleWithAlias.moduleAliases?.map{"'\($0.key)' as '\($0.value)'"}.joined(separator: ", ") ?? "" + observabilityScope.emit(warning: "target '\(moduleWithAlias.name)' for product '\(product.name)' from package '\(package.description)' has module aliases: [\(aliasesMsg)] but may contain non-Swift sources; there might be a conflict among non-Swift symbols") + } + moduleWithAlias.applyAlias() + } + } + func diagnoseUnappliedAliases(observabilityScope: ObservabilityScope) { for aliasList in aliasMap.values { for productAlias in aliasList { @@ -301,6 +321,7 @@ struct ModuleAliasTracker { observabilityScope: ObservabilityScope ) { guard !modules.isEmpty else { return } + var aliasDict = [String: String]() var prechainAliasDict = [String: [String]]() var directRefAliasDict = [String: [String]]() diff --git a/Tests/BuildTests/ModuleAliasingBuildTests.swift b/Tests/BuildTests/ModuleAliasingBuildTests.swift index 32974186802..19d10784143 100644 --- a/Tests/BuildTests/ModuleAliasingBuildTests.swift +++ b/Tests/BuildTests/ModuleAliasingBuildTests.swift @@ -1806,9 +1806,11 @@ final class ModuleAliasingBuildTests: XCTestCase { result.checkProductsCount(1) result.checkTargetsCount(9) + // The root-level package is the most down-stream, so it sets the upstream module's name via the + // aliases. It shouldn't have an alias itself since it references the module by the alias name. XCTAssertTrue( result.targetMap.values - .contains { $0.module.name == "A" && $0.module.moduleAliases?["Utils"] == "XUtils" } + .contains { $0.module.name == "A" && $0.module.moduleAliases == nil} ) XCTAssertTrue( result.targetMap.values @@ -2021,12 +2023,12 @@ final class ModuleAliasingBuildTests: XCTestCase { result.checkProductsCount(1) result.checkTargetsCount(8) - + // The root-level package is the most down-stream, so it sets the upstream module's name via the + // aliases. It shouldn't have an alias itself since it references the module by the alias name. XCTAssertTrue( result.targetMap.values .contains { - $0.module.name == "A" && $0.module.moduleAliases?["Log"] == "XLog" && $0.module.moduleAliases? - .count == 1 + $0.module.name == "A" && $0.module.moduleAliases == nil } ) XCTAssertTrue( @@ -2240,11 +2242,12 @@ final class ModuleAliasingBuildTests: XCTestCase { result.checkProductsCount(1) result.checkTargetsCount(8) + // The root-level package is the most down-stream, so it sets the upstream module's name via the + // aliases. It shouldn't have an alias itself since it references the module by the alias name. XCTAssertTrue( result.targetMap.values .contains { - $0.module.name == "A" && $0.module.moduleAliases?["Utils"] == "XUtils" && $0.module - .moduleAliases?["Log"] == "XLog" && $0.module.moduleAliases?.count == 2 + $0.module.name == "A" && $0.module.moduleAliases == nil } ) XCTAssertTrue(