Skip to content

Package-level module aliases are not applied #6911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fumoboy007 opened this issue Sep 17, 2023 · 7 comments · Fixed by #6917
Closed

Package-level module aliases are not applied #6911

fumoboy007 opened this issue Sep 17, 2023 · 7 comments · Fixed by #6917
Labels

Comments

@fumoboy007
Copy link

fumoboy007 commented Sep 17, 2023

Description

I have a package that needs to alias a dependency’s module due to conflicting module names.

Currently, I am doing this at the product level like so:

.product(name: "MessagePack", package: "MessagePack", moduleAliases: [
   "MessagePack": "FlightSchoolMessagePack",
]),

This works, but I would like to do the alias at the package level like so:

let flightSchoolMessagePackDependency: PackageDescription.Package.Dependency =
   .package(url: "https://github.com/Flight-School/MessagePack.git", from: "1.2.4")
flightSchoolMessagePackDependency.moduleAliases = ["MessagePack": "FlightSchoolMessagePack"]

let package = Package(
   
   dependencies: [
      flightSchoolMessagePackDependency,
   
)

Expected behavior

I expect the package-level module alias to be applied, similar to the product-level module alias.

Actual behavior

The package manager returns an error:

multiple targets named 'MessagePack' in: 'messagepack', 'msgpack-swift'; consider using the moduleAliases parameter in manifest to provide unique names

Steps to reproduce

  1. Download my package.
  2. Switch to the with-flight-school-dep branch.
  3. Replace the dependency .package(url: "https://github.com/Flight-School/MessagePack.git", from: "1.2.4") with a similar dependency that has the moduleAliases property specified.
  4. Remove the moduleAliases parameter from .product(name: "MessagePack", package: "MessagePack", moduleAliases: …).
  5. swift build

Swift Package Manager version/commit hash

swift-5.8.1-RELEASE

Swift & OS version (output of swift --version ; uname -a)

swift-driver version: 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100)
Target: arm64-apple-macosx13.0
Darwin Darrens-Mac.momahome 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul  5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000 arm64
@fumoboy007 fumoboy007 added the bug label Sep 17, 2023
@fumoboy007
Copy link
Author

@elsh @tomerd Perhaps related to #6913? This one is 100% reproducible though.

@neonichu
Copy link
Contributor

Do I understand correctly that this compiles?

let flightSchoolMessagePackDependency: PackageDescription.Package.Dependency =
   .package(url: "https://github.com/Flight-School/MessagePack.git", from: "1.2.4")
flightSchoolMessagePackDependency.moduleAliases = ["MessagePack": "FlightSchoolMessagePack"]

I would not expect that, IIRC we only defined module aliases at the product dependency level? cc @tomerd

@fumoboy007
Copy link
Author

fumoboy007 commented Sep 18, 2023

@neonichu The package-level property is defined here.

@elsh
Copy link
Contributor

elsh commented Sep 18, 2023

It was meant to be used as a property of a product dependency. It was declared public in Package.Dependency at the time it was added due to the need for intra-module visibility; there are other properties such as .targetItem(name:...) that were made public for the same reason even though they weren't meant to be visible to users. However, as of swift 5.9, a new access level called package is introduced, which limits visibility of such properties, so they can be hidden from users once modified with that access level.

@fumoboy007
Copy link
Author

fumoboy007 commented Sep 18, 2023

Hmm I guess we could/should have prefixed those symbols with an underscore to make that clear.

When are we changing the access level to package? Shall we repurpose this issue to track that work?

@neonichu
Copy link
Contributor

I don't think we can do that until the next release after 5.10, but we should be able to use @spi here in the meantime, I think?

@neonichu
Copy link
Contributor

Actually seems like we can just make it internal. It may have been needed to be public because of how the old serialization code worked that I refactored in 5.9

neonichu added a commit that referenced this issue Sep 18, 2023
These were public for implementation reasons, likely because of how the old serialization code worked which I refactored in 5.9

fixes #6911
neonichu added a commit that referenced this issue Sep 19, 2023
These were public for implementation reasons, likely because of how the old serialization code worked which I refactored in 5.9

fixes #6911
MaxDesiatov pushed a commit that referenced this issue Sep 28, 2023
These were public for implementation reasons, likely because of how the old serialization code worked which I refactored in 5.9

fixes #6911
MaxDesiatov pushed a commit that referenced this issue Sep 28, 2023
These were public for implementation reasons, likely because of how the old serialization code worked which I refactored in 5.9

fixes #6911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants