Logging module name clash in Vapor 3

[tanner0101 (Tanner)]

IBM-Swift/LoggerAPI has added a dependency on apple/swift-log in version 1.9.0. This unfortunately creates a clash with an existing module named Logging in vapor/console 3.0.0 and later.

If you use IBM / Kitura packages that depend on LoggerAPI with Vapor 3, you may see the following error while compiling:

error: multiple products named 'Logging' in: Console, swift-log
error: multiple targets named 'Logging' in: Console, swift-log

Solution 1: Rename Vapor's Logging module

I've opened a pull request here with a potential fix to this issue that renames Vapor's Logging module to LoggingKit:

https://github.com/vapor/console-kit/pull/114

This however is a breaking change itself and has the potential to cause projects based on Vapor 3 or community packages to stop compiling.

We can patch these issues quickly as they arise, but it's possible this process would end up negatively affecting more users than the current issue.

Solution 2: Pin to Logger API < 1.9.0

Another solution to this issue is for Vapor 3 projects to pin their LoggerAPI versions to < 1.9.0. This can be done with the following addition to Package.swift:

.package(url: "https://github.com/IBM-Swift/LoggerAPI.git", .upToNextMinor(from: "1.8.0"))

---

There have already been a couple cases in http://vapor.team of people hitting this issue.

We're wondering if trying solution 1, which could potentially solve the issue or cause more breakages, is worth it. Or, if solution 2 will be fine for those who use Kitura packages alongside Vapor 3.

Any feedback you have on this would be great. Thanks!

[calebkleveter]

I'm personally in favor of renaming the Console.Logging module to LoggingKit. That way people that want to use any other logger built on the swift-log package won't run into conflicts either.

[0xTim (Tim)]

Yeah this demonstrates a major weakness in SPM here (tagging @Aciid and @rballard in case there are thoughts on a better way) in that you can't have 2 packages with the same target name.

I'm against renaming it - we use SemVer for a reason. Yes it is a pain and more than likely won't affect most people. But libraries written using Vapor's Logging API will then break, any libraries built on top of those will then break and applications that consume those will be broken, waiting on fixes, which could take time, blocking security updates and new features etc etc

There's also the question of where to draw the line. Let's say I write a package for the SSWG that contains a target called Console which people started using and that broke a number of people's builds - would that result in Vapor renaming it to ConsoleKit?

Slight devils advocate there, but there really isn't a good solution for this one! Good to get all points out before making a decision.

[Aciid (Ankit Aggarwal)]

Unfortunately, this is a Swift language limitation so it's not possible to resolve it in SwiftPM. Prefix your target names!

[gwendal.roue (Gwendal Roué)]

Wow, I learned something important today. But I'm not sure I fully understand the advice. For example I'm not fully sure of the relation between target and module names. Can I prefix a target without renaming a module?

@Aciid, would you please be more specific, maybe even give one or two typical examples?

[0xTim (Tim)]

In your manifest you define a number of targets for your package that you want to expose to the world. The name of these become the namesake of the modules you import in your code. These must all the unique across all dependencies in your code. Whilst you can namespace types and objects in Swift, the modules must be unique

[Max_Howell1 (Max Howell)]

I know you have almost certainly considered it, but in this decentralized system it seems essential to allow target name overlap (by namespacing). Here a dependency added a new dependency and it broke an upstream package. This sort of thing will happen again. Nobody considers adding new (especially private, ie. not exposed in API) dependencies to be breaking changes (despite SemVer’s forum having a thread about how actually this should be considered a major breaker)

Solving it at the build-system layer is the only necessary bit IMO. Like ideally you could namespace imports in your sources, but that’s nice-to-have, this is a real problem.

I guess this would require driver changes, since you’d have to map swift source module names to some kind of prefixed module name to allow overlap. Not a trivial task. But in pursuit of stable graphs, it seems important.

[IanPartridge]

GitHub - Kitura/LoggerAPI: Logger protocol maintainer here...

I'm very sorry that this breakage has occurred. It did not occur to us that adding a dependency in SemVer minor could break people like this.

I can't see anything we could have done differently, beyond doing this in a SemVer major release which is not really practical for us...

We would be very happy if we could namespace our stuff as io.kitura.* but that's not an option...

[anandabits (Matthew Johnson)]

@IanPartridge

I can't see anything we could have done differently, beyond doing this in a SemVer major release which is not really practical for us...

It’s understandable that this happened - I don’t think this issue is one that has received widespread attention yet. But going forward it looks like adding a dependent module with a non-namespaced name must be considered a breaking change. SemVer loses its meaning if people don’t take it seriously. Breaking means breaking, period.

If we aren’t strict about changes that are breaking in practice we lose most of the benefits of SemVer. This is something that needs to be a community-wide discipline.

I also agree that there should be a better solution to module name conflicts especially for private dependencies. It seems like a solvable problem and is important enough that it should receive the attention necessary to solve it.

[masters3d]

@IanPartridge

We would be very happy if we could namespace our stuff as io.kitura.* but that's not an option...

Are you referring about the technical limitation of swift?

[IanPartridge]

Yes, all modules are "top-level" currently.

[tanner0101 (Tanner)]

@IanPartridge

I'm very sorry that this breakage has occurred. It did not occur to us that adding a dependency in SemVer minor could break people like this

No worries, not IBM’s fault at all. We predicted this issue when apple/swift-log was in the proposal stage. It’s vapor’s fault for picking an overly generic module name for the logging module.

@Max_Howell1

Solving it at the build-system layer is the only necessary bit IMO. Like ideally you could namespace imports in your sources, but that’s nice-to-have, this is a real problem.

I guess this would require driver changes, since you’d have to map swift source module names to some kind of prefixed module name to allow overlap. Not a trivial task. But in pursuit of stable graphs, it seems important .

Yeah I agree, this is something that needs to be handled by Swift / SPM. It will take some time to get proper name spacing though. Until then, it’s up to the SSWG to try and prevent this as much as possible going forward. And we are working really hard to do that, especially @johannesweiss who has written a lot about this.

The only unfortunate part is that fixing past mistakes is difficult. Vapor 3 should not have used the overly general name Logging. Hindsight is 20/20 of course. In the next major version of Vapor, we’ve dropped our Logging module altogether in favor of the Apple one.

We should continue the more broad discussion of namespacing here:

Namespacing of packages/modules, especially regarding SwiftNIO

First of all, I apologise for bringing this up only now but I believe this is very important for the long-term health ecosystem and it's not too late yet. What's the issue? In a given Swift(PM) application, 3 things about a package need to be unique across the whole application: it's package basename (for example swift-nio-http2) it's package name (let package = Package(name: "swift-nio", ...) in Package.swift) all module names Arguably, that can lead to tricky issues when combining multipl…

Or in a new thread.

This thread should be dedicated specifically to the Logging module clash between Vapor 3 and apple/swift-log, and known workarounds / solutions.

[bzamayo (Benjamin Mayo)]

As a related aside, is the Vapor team aware of the newly announced Apple CryptoKit? Won't this clash with Vapor's CryptoKit package? Apple's framework isn't available on Linux so the problem is somewhat lessened I guess.

[tanner0101 (Tanner)]

@bzamayo yup, I'm aware of that one. I think we'll need to rename vapor/crypto-kit. Ideally Vapor would dynamically depend on vapor/crypto-kit only on Linux, using Apple's CryptoKit when on Darwin platforms, but that is not possible with SPM currently (without hacks).

[Aciid (Ankit Aggarwal)]

@gwendal.roue

@Aciid, would you please be more specific, maybe even give one or two typical examples?

@Max_Howell1

Solving it at the build-system layer is the only necessary bit IMO. Like ideally you could namespace imports in your sources, but that’s nice-to-have, this is a real problem.

In Swift, symbols are mangled with their module names and each SwiftPM target produces a single Swift module with (almost) the same name as the target. If two packages have targets with the same name, there is nothing the build system can do to prevent the module and symbol collisions today. This is true even for transitive targets. Let's say the compiler had some ability that allowed "renaming" a module, we'd still have runtime implications (e.g., anything that constructs a class from a string).

What I am trying to say is that we need to identify and resolve language-level problems first. The build system/package manager will most likely be an "easier" part of the problem.

[Blejwi (Damian)]

Are there any plans on releasing this change? Due to this conflict we cannot use swift-log in our applications.
I'm sure that swift-log will become more and more popular and such scenarios may occur more often.

[tonyarnold]

What was the resolution here for Vapor 3.x? I'm seeing this error after integrating GitHub - vapor-community/VaporMonitoring: Monitoring for Vapor into my project.

The workaround proposed of locking LoggerAPI to versions earlier than 1.9.0 does work, but seems like a problem waiting to happen.

[Joe_Smith]

To my knowledge folks are heads-down working on Vapor 4 (which should work with swift-log and swift-metrics out of the box), so a fix would need to come from elsewhere in the community.

Use the latest & greatest SwiftPrometheus, and utilize Middleware by Yasumoto · Pull Request #5 · vapor-community/VaporMonitoring · GitHub changes the dependency of SwiftPrometheus to the 0.x.x series of tags, which has the explicit intention of being NIO1-friendly. I think @MrLotU we should make sure that pre-1.0 release is also compatible with Vapor 3.

@tonyarnold are you running into that issue on PR #5?

[tonyarnold]

To be honest, I haven't tried the branch yet - I wasn't sure if it was going to be merged, so I didn't want to head down the garden path (given how changed the configuration/implementation is). I'll give it a whirl.

Update: Yep, the new branch fixes the name clash. It raises other problems, but I'll bring those up on the PR directly. Thanks, @Joe_Smith!

[Joe_Smith]

Sweet! Looking forward to making it better