From 0401a2ae55077cfd1f4c0acd43ae0a1a56ab21ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Fo=C5=99t?= Date: Thu, 2 Jan 2025 16:07:24 +0100 Subject: [PATCH] Fix resolve failing when package from registry is referenced by name (#8166) ### Motivation: When running `swift package --replace-scm-with-registry --default-registry-url {our-registry-url} resolve`, the command fails when depending on packages that reference their transitive dependencies by name. I created a [demo](https://github.com/fortmarek/spm-registry-by-name-references) where you can reproduce the issue by running `swift package --replace-scm-with-registry --default-registry-url {our-registry-url} resolve`. You should see the following error: ``` error: 'cpisciotta.xcbeautify': unknown dependency 'Colorizer' in target 'XcbeautifyLib'; valid dependencies are: 'swift-argument-parser' (from 'https://github.com/apple/swift-argument-parser.git'), 'getGuaka.Colorizer', 'MaxDesiatov.XMLCoder' error: 'cpisciotta.xcbeautify': unknown dependency 'XMLCoder' in target 'XcbeautifyLib'; valid dependencies are: 'swift-argument-parser' (from 'https://github.com/apple/swift-argument-parser.git'), 'getGuaka.Colorizer', 'MaxDesiatov.XMLCoder' ``` The issue boils down to: ```swift // Root Package.swift import PackageDescription let package = Package( name: "package-test", dependencies: [ // We're depending on `xcbeautify` from our root `Package.swift` .package(url: "https://github.com/cpisciotta/xcbeautify", exact: "2.13.0") ], targets: [ .target(name: "package-test"), ] ) // Package.swift at https://github.com/cpisciotta/xcbeautify/blob/main/Package.swift import PackageDescription let package = Package( name: "xcbeautify", products: [ .library(name: "XcbeautifyLib", targets: ["XcbeautifyLib"]), ], dependencies: [ .package( url: "https://github.com/getGuaka/Colorizer.git", .upToNextMinor(from: "0.2.1") ), .package( url: "https://github.com/MaxDesiatov/XMLCoder.git", .upToNextMinor(from: "0.17.1") ), ], targets: [ .target( name: "XcbeautifyLib", dependencies: [ // Transitive package products referenced by name "Colorizer", "XMLCoder", ] ), ] ) ``` The issue is that when swizzling the package product names to their identity counterparts, only dependencies referenced via `.product(...)` are swizzled. Indeed, when we change the `XcbeautifyLib`'s dependencies to: ```swift .product(name: "Colorizer", package: "Colorizer"), .product(name: "XMLCoder", package: "XMLCoder"), ``` the resolution succeeds. ### Modifications: To fix the above issue, we swizzle also dependencies referenced by name _iff_ the product name is the same as the package name the root depends on. This aligns with how `byName` is treated in other parts of the codebase, such as [here](https://github.com/swiftlang/swift-package-manager/blob/main/Sources/PackageModel/Manifest/Manifest.swift#L407). ### Result: `swift package --replace-scm-with-registry --default-registry-url {our-registry-url} resolve` succeeds when run in the [repro sample](https://github.com/fortmarek/spm-registry-by-name-references). --------- Co-authored-by: Max Desiatov --- Sources/Workspace/Workspace+Registry.swift | 27 ++++++-- Tests/WorkspaceTests/WorkspaceTests.swift | 79 ++++++++++++++++++++++ 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/Sources/Workspace/Workspace+Registry.swift b/Sources/Workspace/Workspace+Registry.swift index 8a022a9540a..0fc2a25c494 100644 --- a/Sources/Workspace/Workspace+Registry.swift +++ b/Sources/Workspace/Workspace+Registry.swift @@ -257,11 +257,17 @@ extension Workspace { var modifiedDependencies = [TargetDescription.Dependency]() for dependency in target.dependencies { var modifiedDependency = dependency - if case .product(let name, let packageName, let moduleAliases, let condition) = dependency, - let packageName - { - // makes sure we use the updated package name for target based dependencies - if let modifiedPackageName = targetDependencyPackageNameTransformations[packageName] { + switch dependency { + case .product( + name: let name, + package: let packageName, + moduleAliases: let moduleAliases, + condition: let condition + ): + if let packageName, + // makes sure we use the updated package name for target based dependencies + let modifiedPackageName = targetDependencyPackageNameTransformations[packageName] + { modifiedDependency = .product( name: name, package: modifiedPackageName, @@ -269,6 +275,17 @@ extension Workspace { condition: condition ) } + case .byName(name: let packageName, condition: let condition): + if let modifiedPackageName = targetDependencyPackageNameTransformations[packageName] { + modifiedDependency = .product( + name: packageName, + package: modifiedPackageName, + moduleAliases: [:], + condition: condition + ) + } + case .target: + break } modifiedDependencies.append(modifiedDependency) } diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift index 47b92a56c3c..457deeb04a6 100644 --- a/Tests/WorkspaceTests/WorkspaceTests.swift +++ b/Tests/WorkspaceTests/WorkspaceTests.swift @@ -12869,6 +12869,85 @@ final class WorkspaceTests: XCTestCase { ["did load manifest for registry package: org.baz (identity: org.baz)"] ) } + + func testTransitiveResolutionFromRegistryWithByNameDependencies() async throws { + let sandbox = AbsolutePath("/tmp/ws/") + let fs = InMemoryFileSystem() + + let workspace = try await MockWorkspace( + sandbox: sandbox, + fileSystem: fs, + roots: [ + MockPackage( + name: "Root", + path: "root", + targets: [ + MockTarget(name: "RootTarget", dependencies: [ + .product(name: "FooProduct", package: "foo"), + ]), + ], + products: [], + dependencies: [ + .sourceControl(url: "https://git/org/foo", requirement: .upToNextMajor(from: "1.0.0")), + ], + toolsVersion: .v5_6 + ), + ], + packages: [ + MockPackage( + name: "Bar", + identity: "org.bar", + alternativeURLs: ["https://git/org/Bar"], + targets: [ + MockTarget(name: "BarTarget"), + ], + products: [ + MockProduct(name: "Bar", modules: ["BarTarget"]), + ], + versions: ["1.0.0", "1.1.0"] + ), + MockPackage( + name: "FooPackage", + identity: "org.foo", + alternativeURLs: ["https://git/org/foo"], + targets: [ + MockTarget(name: "FooTarget", dependencies: [ + "Bar", + ]), + ], + products: [ + MockProduct(name: "FooProduct", modules: ["FooTarget"]), + ], + dependencies: [ + .sourceControl(url: "https://git/org/Bar", requirement: .upToNextMajor(from: "1.0.0")), + ], + versions: ["1.0.0", "1.1.0", "1.2.0"] + ), + ] + ) + + workspace.sourceControlToRegistryDependencyTransformation = .swizzle + + try await workspace.checkPackageGraph(roots: ["root"]) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTester(graph) { result in + result.check(roots: "Root") + result.check(packages: "org.bar", "org.foo", "Root") + result.check(modules: "FooTarget", "BarTarget", "RootTarget") + result.checkTarget("RootTarget") { result in + result.check(dependencies: "FooProduct") + } + result.checkTarget("FooTarget") { result in + result.check(dependencies: "Bar") + } + } + } + + workspace.checkManagedDependencies { result in + result.check(dependency: "org.foo", at: .registryDownload("1.2.0")) + result.check(dependency: "org.bar", at: .registryDownload("1.1.0")) + } + } // no dups func testResolutionMixedRegistryAndSourceControl1() async throws {