Skip to content

Commit

Permalink
Fix resolve failing when package from registry is referenced by name (s…
Browse files Browse the repository at this point in the history 
…wiftlang#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 <[email protected]>
  • Loading branch information
@fortmarek @MaxDesiatov
fortmarek and MaxDesiatov authored Jan 2, 2025
1 parent adf64d4 commit 0401a2a
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 5 deletions.
27 changes: 22 additions & 5 deletions Sources/Workspace/Workspace+Registry.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,18 +257,35 @@ 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,
moduleAliases: moduleAliases,
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)
}
Expand Down
79 changes: 79 additions & 0 deletions Tests/WorkspaceTests/WorkspaceTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 0401a2a

Please sign in to comment.