Fix registry package swizzling when package name casing differs #8194
Conversation
fortmarek
requested review from
bnbarham,
MaxDesiatov,
jakepetroules,
francescomikulis,
xedin,
dschaefer2,
shawnhyam,
bripeticca,
plemarquand,
owenv and
bkhouri
as code owners
|
@swift-ci test |
| @@ -220,7 +220,7 @@ extension Workspace { | |||
| info: "swizzling '\(dependency.locationString)' with registry dependency '\(registryIdentity)'." | |||
| ) | |||
| targetDependencyPackageNameTransformations[dependency | |||
| .nameForModuleDependencyResolutionOnly] = registryIdentity.description | |||
| .nameForModuleDependencyResolutionOnly.lowercased()] = registryIdentity.description | |||
There was a problem hiding this comment.
Because package names/identifiers are case insensitive I think it makes more sense to update the .registry case in nameForModuleDependencyResolutionOnly to return a lowercased name so we're consistent everywhere going forward.
There was a problem hiding this comment.
I did that in the latest commit 👍
However, .lowercased() still needs to be kept as dependency is not necessarily the registry case as that's done the line below. This could also be rewritten to:
modifiedDependency = .registry(
identity: registryIdentity,
requirement: requirement,
productFilter: settings.productFilter,
traits: settings.traits
)
targetDependencyPackageNameTransformations[modifiedDependency
.nameForModuleDependencyResolutionOnly] = registryIdentity.descriptionLet me know what you prefer, I don't have a clear preference here.
There was a problem hiding this comment.
I think your proposed rewrite here to create the modifiedDependency first makes more sense, since it doesn't duplicate the lowercasing logic in nameForModuleDependencyResolutionOnly
|
@swift-ci please test |
Fix registry package swizzling when package name casing differs
Motivation:
When swizzling dependencies when running
swift package --replace-scm-with-registry resolve, package references that have different casing don't get swizzled.You can find an example of this scenario in the Firebase SDK that boils down to:
The
.product(name: "FBLPromises", package: "Promises")should be swizzled to use thegoogle.promisesidentifier, but because of the different casing, it's not.Modifications:
When creating the map for packages to be swizzled, make the key lowercase. When we access the map, we again use a lowercase variant of the package name, so we don't unexpectedly miss an entry because of a different casing.
Result:
Packages with different casing are swizzled when using the
swift package --replace-scm-with-registry resolvecommand.