Skip to content
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

Split package akka.http.ccompat #2902

Open
lefou opened this issue Jan 20, 2020 · 3 comments
Open

Split package akka.http.ccompat #2902

lefou opened this issue Jan 20, 2020 · 3 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted binary-incompatible discuss Tickets that need some discussion before proceeding help wanted Identifies issues that the core team will likely not have time to work on nice-to-have (low-prio) Tasks which make sense, however are not very high priority, feel free to help out!

Comments

@lefou
Copy link

lefou commented Jan 20, 2020

This is a follow-up to akka/akka#28304 (comment).

The two artifacts akka.parsing and akka.http.core. both in version 10.1.11, contain (disjunct) classes in the Java package akka.http.ccompat. A split package is considered bad practice, even in plain Java world. Split packages are even not possible when working with sealed packages/jars.

As we currently review the OSGi support for akka and akka-http in a separate project (as a consequence of akka/akka#28304), we wanted to ask the purpose of the found split package. If this is "by accident", I'd like to propose one of the following fixes:

  1. Move all classes to the same package into the same artifact
  2. Rename one of the packages to avoid the split-package issue, preferably choosing a package name which shares the artifact name as it's prefix
  3. Keep it as-is. I think this way should be avoided, but could be necessary e.g. when the packages are part of a public API. But even then, maybe 1. will also work.

Related to woq-blended/akka-osgi#1

@jrudolph jrudolph added the 0 - new Ticket is unclear on it's purpose or if it is valid or not label Jan 21, 2020
@jrudolph
Copy link
Member

jrudolph commented Jan 21, 2020

we wanted to ask the purpose of the found split package. If this is "by accident", I'd like to propose one of the following fixes:

It's mostly by accident, the problem is that we often cannot repair it because of binary compatibility constraints.

For this particular instance of the problem, we could rename the one in akka-parsing because it's only internal.

The last time I looked into it, this wasn't the only instance, though. e.g. akka.http.scaladsl.settings is also split but that's public API we don't want to change. Another one is akka.http.scaladsl.server.directives. Before we start doing this we would need a comprehensive list of all these instances to see whether it makes sense at all to start this endeavor. My hunch is that just because of those two, changing the structure will take at least a deprecation cycle until they could be finally moved. That would probably take 1-2 years.

So, the most realistic outcome from our side will be 3.

3\. But even then, maybe 1. will also work.

It does not work for us because we want to keep the current module structure. But maybe it could work for you? You could just mix together all artifacts of akka-http into a single big one. This, of course, defies the goal of modularity (same as your solution 1. above) but since most people use the akka-http-core and akka-http together anyway (and all the other modules are small), it might not matter so much in practice.

@lefou
Copy link
Author

lefou commented Jan 21, 2020

Yeah, we saw these other packages too and will also report it, but as each package needs a possible different solution, I reported it separately. Although I agree that the other (not yet reported) split-package issue are not so easy to fix, I think this one can and should be fixed now by renaming the internal package name in akka-parsing.

@raboof
Copy link
Member

raboof commented Jan 27, 2020

Although I agree that the other (not yet reported) split-package issue are not so easy to fix, I think this one can and should be fixed now by renaming the internal package name in akka-parsing.

I agree we could move those ccompat classes (which should be internal), though indeed other instances of the same problem might not be so straightforward.

@jrudolph jrudolph added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted binary-incompatible discuss Tickets that need some discussion before proceeding help wanted Identifies issues that the core team will likely not have time to work on nice-to-have (low-prio) Tasks which make sense, however are not very high priority, feel free to help out! and removed 0 - new Ticket is unclear on it's purpose or if it is valid or not labels Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted binary-incompatible discuss Tickets that need some discussion before proceeding help wanted Identifies issues that the core team will likely not have time to work on nice-to-have (low-prio) Tasks which make sense, however are not very high priority, feel free to help out!
Projects
None yet
Development

No branches or pull requests

3 participants