Store the correct package name in PackagePIFBuilder.LinkedPackageBinary metadata

[pmattos]

Motivation:

When using the new PIF builder (i.e., --build-system swiftbuild), the returned [PackagePIFBuilder.LinkedPackageBinary] objects were lacking package dependencies information.

For instance, given this simple sample package:

Package(
    name: "TestLinking",
    products: [
        .library(
            name: "TestLinking",
            type: .static,
            targets: ["TestLinking"]
        )
    ],
    dependencies: [
        .package(url: "https://github.com/apple/swift-collections.git", branch: "1.1.1")
    ],
    targets: [
        .target(
            name: "TestLinking",
            dependencies: [
                .product(name: "Collections", package: "swift-collections")
            ]
        )
    ]
)

This would be the expected build graph (by traversing the returned [PackagePIFBuilder.ModuleOrProduct] array):

TestLinking 
  TestLinking 
    TestLinking.swift 
    Collections 
      Collections
        _RopeModule
          InternalCollectionsUtilities 
        OrderedCollections 
          InternalCollectionsUtilities 
        HeapModule
          InternalCollectionsUtilities 
        HashTreeCollections 
          InternalCollectionsUtilities 
        DequeModule 
          InternalCollectionsUtilities 
        BitCollections 
          InternalCollectionsUtilities

...but this is what we would be getting instead (ie, note that all swift-collections dependencies are missing from the graph):

TestLinking
  TestLinking
    TestLinking.swift

Why this issue wasn't breaking the package build too?

Good question! The sample package built just fine with the new PIF builder (i.e., --build-system swiftbuild) even though PackagePIFBuilder.LinkedPackageBinary encoded the wrong information, as outlined above. This happened because the build is driven exclusively by the PIF, which is correct.

The broken PackagePIFBuilder.LinkedPackageBinary objects are only a side effect of computing the PIF and don't effect the build at all. The bug would only manifest in clients who called PackagePIFBuilder directly and then inspected the resulting build graph from such auxiliary metadata (e.g., Xcode).

Modifications:

The root cause was how we initialized LinkedPackageBinary objects during the PIF construction. For a given package dependency, we were incorrectly reusing the client package as the package name (i.e., it should be the package name where the dependency is coming from instead).

This PR also replaces packageName: String with PackageIdentity for tracking packages in LinkedPackageBinary objects, ensuring unique IDs for packages (and exposing a more typesafe API too).

Result:

This fixes rdar://154408350 (New SwiftPM PIF packages builder breaks previews in Xcode).

[pmattos]

This was the root cause: Note how we wrongly forward the client package name (i.e., package.name) to PackagePIFBuilder.LinkedPackageBinary, binding it to the incorrect package.

[bkhouri]

Can we add automated test to ensure we don't the behaviour does not regress?

[plemarquand]

@swift-ci test

[pmattos]

Can we add automated test to ensure we don't the behaviour does not regress?

@bkhouri We do have a few tests in the Xcode side of things... but nothing in SwiftPM OSS touches PackagePIFBuilder.LinkedPackageBinary that I'm aware of. Maybe let's do this in a future PR?

[pmattos]

@swift-ci test windows

[pmattos]

@swift-ci test

[bkhouri]

Can we add automated test to ensure we don't the behaviour does not regress?

@bkhouri We do have a few tests in the Xcode side of things... but nothing in SwiftPM OSS touches PackagePIFBuilder.LinkedPackageBinary that I'm aware of. Maybe let's do this in a future PR?

@pmattos : Just because tests exists in the Xcode side of this, it does not mean test in SwiftPM should be omitted :)

Submitting tests in a future PR does not guarantee we will not regress the behaviour between now and then. Every change should have an associated tests to validate the behaviour.

We have a framework in place to verify PIF changes. Have a look at https://github.com/swiftlang/swift-package-manager/blob/main/Tests/SwiftBuildSupportTests/PIFBuilderTests.swift, so hopefully writting an automated tests can be trivial.