scala / scala-dev

Scala 2 team issues. Not for user-facing bugs or directly actionable user-facing improvements. For build/test/infra and for longer-term planning and idea tracking. Our bug tracker is at https://github.com/scala/bug/issues
Apache License 2.0
130 stars 14 forks source link

JLine 3: finalize jna vs. jansi choice, including legalities #677

Closed SethTisue closed 4 years ago

SethTisue commented 4 years ago

in my limited searching so far, it seems like it may not even matter? but I'm suspicious that if I dug further I'd discover differences. I don't even really understand what the choice is about

the doc on this is at https://github.com/jline/jline3#jansi-vs-jna

I have a note that said @adriaanm said to use jansi, but I don't know why

initially I thought I could just take Adriaan's conclusion as a given and not bother digging into it, but now I've found that Dotty went with jna, and throws it somewhat into question for me since I think it could help us a lot to use the Dotty code and the Dotty behavior as a reference, and it would be nice to have fewer possible causes for behavior differences

regardless of where we end up, we need to make sure to update our license/legal stuff (I should check the legal stuff for JLine itself, at the same time I do jansi)

if jna involves native code that could be a source of trouble (harder/impossible to shade, increased possibility of JLine 2 / 3 conflict with sbt still on 2, ...). but maybe jna can fall back to using no native code? (does the Dotty one use native code?)

what does Ammonite do?

there is a bit of prior discussion at https://github.com/scala/scala/pull/8036#discussion_r299040225

adriaanm commented 4 years ago

one aspect is licensing

adriaanm commented 4 years ago

Coursier and sbt both use jansi. Ammonite uses jna.

adriaanm commented 4 years ago

Speaking of Ammonite, they upgraded to jline 3 in this PR: https://github.com/lihaoyi/Ammonite/pull/775 (no discussion of JNA versus JANSI, so I assume it's for no particular reason).

My gut feeling (after having skimmed Jline's usage of JNA and JANSI), is that jansi is preferable because it's a more focused solution.

smarter commented 4 years ago

I had the same question recently, as far as I can tell it really doesn't matter and I'd be happy to align Dotty with what scalac chooses.

SethTisue commented 4 years ago

about licensing, at https://github.com/scala/scala/pull/7645#issuecomment-474957755 @smarter wrote:

JNA can be used as Apache2: https://github.com/java-native-access/jna/blob/16aac5fcac290ac05804da222e779e2dba365aa5/LICENSE#L1-L6

adriaanm commented 4 years ago

Let's decide to use Jansi on the basis that sbt and coursier use it, and that Guillaume is happy to align with us. Switching between the two is a small change, so, worst case, we can fall back to JNA later.

SethTisue commented 4 years ago

ok it's settled, and I just need to make sure to take care of the licensing (including Whitesource) and this will be done.

SethTisue commented 4 years ago

when I take care of this, I might as well also update src/intellij at the same time

SethTisue commented 4 years ago

I added commits to https://github.com/scala/scala/pull/8036 that update the legal stuff and IntelliJ

SethTisue commented 4 years ago

We discussed this again today at a team meeting.

sbt uses JLine 2+Jansi, and @eed3si9n has found that on Windows, sbt console doesn't work because of the conflicting Jansi versions. (And modifying sbt to somehow work around this might be possible, but it isn't obvious how, and anyway, it would require sbt users on Windows to upgrade their sbt in order to use 2.13.2, which isn't great.)

So Eugene has this PR which switches Scala over to jna: https://github.com/som-snytt/scala/pull/5

The conclusion at the team meeting: let's try jna next. If it works better on sbt+Windows, then great. Ammonite uses jna, Dotty uses jna, that should make us optimistic it will work. If the experiment doesn't pan out, we'll reconsider.

SethTisue commented 4 years ago

We also discussed (both in the team meeting, and on https://github.com/som-snytt/scala/pull/5) whether sbt ought to fork for its console. In the long run perhaps it should, but consoleProject can't fork, and anyway if switching to jna resolves any immediate difficulties, that's both easier and safer than messing with how console works.

SethTisue commented 4 years ago

PR updated