-
Notifications
You must be signed in to change notification settings - Fork 967
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
Proposal to convert the main codebase to Scala #600
Conversation
…cess modifiers on 6 other classes
* remove any deprecations and warnings * upgrade lift-json to the last version for 2.10 and support the latest version for 2.11 and 2.12 * now will compile with listed versions in build file
There were reasons for the Java implementation historically, I don’t know which still hold (I actually don’t remember the full rationale). In any case there are probably also a fair number of people using the library in plain Java apps these days. Reimplementing in Scala I guess it is in theory possible to keep the ABI, but a new scala library dependency would be added even for Java apps. I think if the goal were to replace the implementation of this library, the Play and Akka teams would be among those who need convincing. cc @2m @patriknw @viktorklang @jroper though I’m not sure of the current best contact. I would not unilaterally take this path without their buy in. A less disruptive approach might be to try to share a test suite across distinct Java and Scala implementations that would be kept in sync by that suite. But I’m not sure how brutal the sbt wrangling would be to get to that. The current test suite of course requires providing a matching ABI and isn’t a TCK-style generic HOCON conformance suite. I should caution that this library is under-maintained. The good thing is it works and has good tests. But, my work on it is basically limited to trying to respond to but not resolve or work on issues. I’m just trying to preserve institutional knowledge. (My day job at the moment is all about getting libraries like this maintained, but I’m so busy trying to create a fully general solution to maintenance that I can’t do this particular maintenance 🙃) |
Since this is a breaking change, and since it is so widely depended on by so many libraries, the only sane way to do this is to change the group id and package name. Otherwise, the entire ecosystem that uses Typesafe config, including parts of the ecosystem that are in maintenance mode or aren't even maintained any more but are still used, are going to have to do a big lockstep upgrade all at once, which is going to be hugely disruptive. For a library that itself is under-maintained, that's just not feasible, it's not like sbt where there was an existing large community of people that could embark on the massive task of upgrading the hundreds of downstream dependent projects, no such ecosystem for Typesafe config exists, it will be a mess. But, if we change the package name, then both the old and new can coexist on the classpath, and all the downstream dependent libraries can upgrade at the leisure (or not, up to them). Larger projects like Play and Akka can even support both for a time to aid the transition. We can even continue to maintain the Java version on a branch for Java libraries that don't want to bring in Scala. The most obvious choice for a package name, at least from my perspective, is And given that the change is going to be breaking, I'd say that Scala should be embraced in its fullest. For example, Scala doesn't really need enums, because we have pattern matching with case classes, so we should just use those. So we can also use |
All that said - my gut feel is if it ain't broke, don't fix. Lightbend doesn't have any need for this to be implemented in Scala, and in fact it's going to mean more unanticipated work for us because we'll have to upgrade and come up with a migration plan. So unless one of the tech leads speaks up to disagree, it's not likely that Lightbend is going to be contributing any effort towards this. @havocp are you considering doing the work to review this? It's not going to be a small task. Without either an existing maintainer such as @havocp or someone at Lightbend who is willing to review this work and ensure the approach make sense, that the Java API is workable, and everything is up to scratch, it's not likely to be merged. |
@jvirtanen opps,they will depend on scala libraries which is currently not that small 😂 |
@jroper Seems like it could be a separate project with the sharing of tests. |
Whether it's a separate project or not is not so much a technical question, as it is of what message we want to send and what expectations we want to set. Do we want to send the message that we expect that people will eventually "upgrade" to the Scala version? If so, it should be the same project, it can be made version 2, etc. Otherwise, if we expect most people to stay with the Java version, then it should be a separate project. Personally, I'm in favour of viewing this as a new version of the same project, since it's already under maintained, as @havocp said, adding a new project will just spread the existing resources that maintain it even thinner. My concern is that if it's made an alternative project, then it'll just go there and die. Lightbend is not going to maintain it, we're not even giving the Java version the love it needs, and we're not likely to switch to a different version for no increased value. By making it a new version, it secures the future of it, because Lightbend will most likely upgrade all our projects (over time) to it, and so we'll do at least a minimum of maintenance on it over time. If however it's viewed as an alternative port, with its own project, then it needs an owner, and that owner almost definitely won't be Lightbend. |
Needs inputs from scalacenter too. |
@jroper no I can’t really review this (considering only JVM users, if I had time I’d love to catch up on dozens of other non-critical-but-real issues people have filed / PR’d here rather than push a giant reset button in effect and probably make a variety of new bugs). It would probably also compromise my ability to do any maintenance at all if the code were all replaced - not that I’m doing a lot, but I try to help a little and reply to issues, since I know most of the tricky bits. But I also understand if people are trying to clear out Java deps to allow Scala JS / Native projects (that’s the motivation right? has been my assumption). I don’t want to block something if you all want to do it. Also I’d like to find a path where the native/JS folks have a viable way to help themselves. I have no idea how popular non-JVM targets are right now or how artifact publishing works for them or anything else about those. I appreciate the full-featured port concept, a certainly better path than alternatives with only half the spec implemented and a bunch of regressions. But I wouldn’t want to swap out this implementation with that unless there were some credible plan to handle the resulting ripples of work. Something alongside (used by native/JS only) would definitely be less disruptive I would think, and if adequate maintenance is eventually funded, it could be consolidated. If the goal were simply a Scala API there are a bunch of wrappers already (see README), so those have been designed many times. Anyway the library is in “it’s solid, it works, only a few conservative changes” mode - because that’s all the maintenance resources permit - if we wanted to make non-conservative changes that affect the JVM case I can’t commit to planning that or to managing any resulting fallout. If there are ways to add a Scala version to the build and if it breaks only the non-JVM cases are affected, and the non-JVM community keeps it working; I wouldn’t be opposed fo having it in the source tree and sharing some tests, as a way to keep it in sync. But I’m not sure how viable that is. |
I am very excited about this port, thank you @ekrich for your hard work. Scalafmt and Scalafix use HOCON and I would love to run those applications on JS + Native. The lack of a high-fidelity HOCON Scala implementation has been a blocker for this to happen so far. Migrating For JVM-only users this change is arguably a regression so I share the hesitation with @havocp and @jroper to merge this port into the main repo, especially given the already limited resources to maintain the Java HOCON implementation. My recommendation would be to setup a separate repo for this Scala port and release for JVM+JS+Native. This port can live under your personal fork @ekrich, a scalahocon/scalahocon organization or something else. I am happy to help out @ekrich as I'm eager to provide a native |
So, first things first, wow Eric! What an undertaking! Well done.
I really hope we do find a way forward, as it would be a massive shame to discard your efforts here. (In future I'd advise discussing this in advance unless you're happy to risk wasted effort.)
As it stands this PR:
The problem underlying this PR is the fact that:
So I would propose a different strategy than those discussed above. What if we re-wrote this library in Scala but didn't depend on the scala library? I've never actually done this but I've been thinking about it for a long time. It would mean:
But ideally it would achieve:
The one sticking point I can think of at this time is being able to write Java enums in pure Scala. I'm not sure how to do that, and we might need to use (or write) a custom scala compiler plugin to achieve that. Anyone know of anything available?
I think that can be solved with Scala The Akka Team uses it in Akka or Akka-HTTP or something.
From reading your commits I think you state you broke this to make the code more Scala-friendly. So we could also retain this source compatibility.
I'm not sure what you mean here. I looked quickly and it seemed you introduced Scala's
As a secondary topic related here, it might be very useful if the test suite here were published in some conformance form such that other implementations could validate themselves against it. @jroper (just an aside)
👨🎨 s/config/HOCON/ please, mate 😁 (com.lightbend.hocon, org.scalahocon, or something) |
This is really huge 🥇 @ekrich ! To have a portable HOCON library I see one major blocking, as the current Typesafe Config implementation is heavily tangled with JVM, i.e. even if we manage to have a line-by-line port in Scala it won't easily link on the alternative platforms(that is the goal of this initiative). Concepts like The most straight forward step is, IMHO,to have, first, a separate |
Something to keep in mind, the Config is plain data - a JSON object plus ConfigFactory is a convention about where to load configs from that enables multiple libraries in one app to share the same config file. Many of those conventions are JVM-specific. The raw HOCON parser has a very limited public API (ConfigDocument) that people who truly want a HOCON parser tend fo find disappointing because it doesn’t expose format details. This lib mostly keeps fhe actual parser API private right now. One of the more popular feature requests is to expose more of it. Kind of a side topic but. These are conceptually separable. |
There is a bit too much response here for me to address all the comments but I'd like to address a few of the issues.
Overall, I agree with the assessment that we will probably need to be a separate repo although I know it is always better to not have a fork. This is especially true if we have limited resources to help. The path forward for Java folks would be less that ideal in the current form. Support for Scala folks would be less than ideal as mentioned above. @dwijnand This could be written without the Scala dependency but then we have no benefits of Scala and the API can't serve both Java and Scala both at a high level. I used @jroper I totally understand about resources, risk and plans. I do believe if we could make this a port that supports Scala on the JVM, Scala Native, and Scala.JS at a high level then perhaps it would be worth it for Lightbend, Akka, Play, etc. to adopt it in the future. Also, when this library was build and used originally, cross compiling and such was much more difficult than it is now thanks to the sbt team and community. @havocp Thanks you for this library and as I said earlier that I don't think getting this far would have been possible without the excellent test coverage - tests in Scala - thank-you! I'm hoping That I can ask questions about features etc. as we move forward in some way or the other. I would like to get the Scala Center's take where portable Scala code should reside. Certainly inputs from both @densh and @sjrd would be nice as well. I would also need a bunch of support from all the interested parties and those that have offered support such as @olafurpg, @gabro, @andreaTP, and @hepin1989 etc. I think at a minimum, having a HOCON test suite that could support both should be goal. I think it is pretty clear that merging this code is less than ideal but I also want to let others weigh in before any final plan is made. In the meantime I can continue on a different branch working towards something that can be cross platform. |
Taking into account what has been said so far, my recommendation would be:
This will allow several things:
Downsides are:
|
I totally agree 👍 |
@sjrd agree, I think adding a separate deliverable to this repo is a promising way to keep things in sync without disrupting JVM users. It would be good to add maintainers to this repo at the same time though! |
What do you mean no benefits? The benefit of it being in Scala is that it's cross-built into Scala.js and Scala Native. And what do you mean by "the API can't serve both Java and Scala both at a high level"? I don't see how it can't serve as the same library. Of course you can always wrap the library, but in the pursuit of adding Scala.js and Scala Native support I would keep out the desire to make it more "Scala-like". |
@sjrd's idea is more easily implementable than mine and is better than the idea of a fork. Maybe with Dotty we could eventually reconcile the two implementations back into one. |
Some random thoughts --
|
I really appreciate all the input from everyone. The current config library is stable and serves it purpose well for Java and Scala customers alike and is deployed widely. Any changes or anything that could destabilize the project would be very detrimental to downstream projects. The project needs minimal support and introducing this code into the repository would be disruptive and since there is little need for this change and it would require resources, it is not desired. The port, although a faithful reproduction of the current library, cannot serve the Java customer at the level of the current library for the following reasons.
For the above reasons, we have decided to not pursue this PR. We will pursue a Scala, cross platform library with as much compatibility as possible. We will also cooperate on any issues that may affect both the Java and the Scala version. Again we appreciate all the comments and will cc the stakeholders. |
Thank you for sharing the decision @ekrich.
Can you update the issue here once there's something public to follow? Also, it would be probably interesting to at least copy/adapt at least the test suite, in order to ensure API compatibility. Thank you again for you work! |
@gabro Sure no problem. We plan to use the test suite. Thanks for your kind comments as well. |
Best would be to have a language-agnostic test suite. There are hocon
implementations in unrelated languages.
…On Wed, Dec 5, 2018, 10:45 AM Eric K Richardson ***@***.***> wrote:
@gabro <https://github.com/gabro> Sure no problem. We plan to use the
test suite. Thanks for your kind comments as well.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#600 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGAUCWGdHGCr98SXxYriLnKrE_Eh6N5ks5u1-osgaJpZM4Y8gwl>
.
|
Eric has made https://github.com/ekrich/sconfig and published a release |
@ekrich may suggest you PR a change to the README in this repo, to call attention to your efforts? |
Proposal to convert the main codebase to Scala
Purpose
The initial idea was that lightbend/config is a dependency of scalafmt so if we change the API to Scala we could cross compile the library to Scala Native and then scalafmt could be compiled to Scala Native as well.
Process
The port was started with
Config
as that is the core interface in the library and givenConfig
is an interface it could easily be converted to atrait
and the process could move on. But true to form this interface has a Javavararg
s argument so this was a difficult start nonetheless. The port was done file by file until complete with the exception of the Java Optional annotation. Some process points are as follows:java.util
asju
andjava.lang
asjl
to highlight the code for better readabilityscala.collection.JavaConverters
for explicit conversionsDeficiencies
The biggest issue is that creating Java enums in Scala is at best half baked. They work fine in Scala but are verbose and maintain the API faithfully but are not as useful in Java. Individually they can be used by adding the parentheses but cannot participate in switch statements without reverting to
name()
orordinal()
so if statements would be the best approach. They also are not marked as enums in byte code so they can not be used in andEnumMap
but the implementation used aTreeMap
which worked fine to preserve order.The API has been maintained faithfully but the code will not be source compatible for either Java or Scala mostly do the parens in Scala and the enums in Java. The changes to adopt would be very easy to do however.
The
Optional
annotation and reflection based code would need to be evaluated in order to make something appropriate for Scala Native and Scala.JS. There is portable-scala-reflect but that would introduce a dependency and the library has none. Also, Scala Native does not support this reflection API yet.Some access modifiers have been altered from the conversion. Some have been restored as they were commented out but some were left public in order to make the Java code using the Scala compile. The sheer bulk of code changed made perfection very difficult. Auditing the code could reintroduce tighter control where needed in the implementation code where the permissions were changed.
Conclusions and path forward
Certainly this was a significant amount of work so it should be in the community’s best interest to find a path forward if possible. One potential is to use this code as a path forward for Scala 2.11+ and above and only maintain bug fixes for the current version for current users of 2.10 through 2.12. This would allow a more flexible upgrade to newer versions for those customers on 2.11+ already.
One large benefit is that future developers can now code in Scala which is perhaps one of the biggest points for the maintainers.
Hopefully we can find a useful conclusion to this work that is completed to date.