Upgrade to jline3

[Manno15]

There is still some work to be done to this pull request but I wanted to get this out there to get some feedback in some areas.

I still need to do more robust testing on the shell and go back to a few areas that I commented out. I will also refactor/optimize areas if possible.

[ctubbsii]

@Manno15 This is a great start. It looks like a beast of a problem, but it looks like you're headed in the right direction! The fact that so much of the API of JLine is subject to change makes me wonder if we want to just create our own internal abstraction... like a single class that wraps all the JLine stuff, and puts it all in one place, and provides more friendly methods to get to what we need. I'm thinking something along the lines of:

  class SomeClassName {
    SomeClassName() {
      // set up JLine terminal and related objects here
      // no JLine types used outside this class
      // if we need anything from JLine, we add a convenience method below, so JLine is entirely accessed through this class
    }
    println(...) { ... }
    readline(...) { ... }
    saveHistory(...) { ... }
  }

  var console = new SomeClassName();
  console.println("something")

[Manno15]

Yeah, that sounds like a great idea @ctubbsii. I will look into to implementing something like that while I continue to fix some of the areas I was unsure on.

[Manno15]

This is getting close to being finished. Some issues I am facing are history file incompatibilities from previous versions (I believe jline3 requires timestamps in their history file) and hitting 'q' while in the help command doesn't exit out of the command until you hit enter after the 'q'.

[Manno15]

I believe this pull request is ready for review. There are a couple of areas where I unsure of the proper route to go on. One is the use of DumbTerminal in ShellIT and ShellServerIT. The other concern is here

[dlmarion]

LGTM, I made a comment about testing some code that you thought could be removed.

[ctubbsii]

Most of this looks great. I am concerned about the getsplits behavior and corresponding tests, though.

[Manno15]

I am running into an issue getting these changes to run with zookeeper 3.6.2 on fluo-uno. I will investigate. Specific error below:

Exception in thread "shell" java.lang.UnsatisfiedLinkError: 'int org.fusesource.jansi.internal.CLibrary.ioctl(int, long, org.fusesource.jansi.internal.CLibrary$WinSize)'
	at org.fusesource.jansi.internal.CLibrary.ioctl(Native Method)
	at org.jline.terminal.impl.jansi.JansiNativePty.getSize(JansiNativePty.java:146)
	at org.jline.terminal.impl.AbstractPosixTerminal.getSize(AbstractPosixTerminal.java:60)
	at org.jline.terminal.Terminal.getBufferSize(Terminal.java:216)
	at org.jline.reader.impl.LineReaderImpl.doDisplay(LineReaderImpl.java:746)
	at org.jline.reader.impl.LineReaderImpl.(LineReaderImpl.java:303)
	at org.jline.reader.LineReaderBuilder.build(LineReaderBuilder.java:115)
	at org.apache.accumulo.shell.Shell.config(Shell.java:275)
	at org.apache.accumulo.shell.Shell.execute(Shell.java:520)
	at org.apache.accumulo.start.Main.lambda$execKeyword$0(Main.java:126)
	at java.base/java.lang.Thread.run(Thread.java:834)

EDIT: My guess is for some reason LineReader is trying to default to using Jansi for the terminal and we don't have the correct libraries for it? I will push a commit that tells the terminal to not allow Jansi so it should default to a different type. I believe the old default type was xterm so not sure why the new zookeeper version updates the default to Jansi.

[Manno15]

@ctubbsii I believe I hit on everyones questions and suggestions. Do you have anything further to add or any concerns?

[ctubbsii]

@ctubbsii I believe I hit on everyones questions and suggestions. Do you have anything further to add or any concerns?

I can take another look today and re-run the ITs.

[ctubbsii]

@ctubbsii I believe I hit on everyones questions and suggestions. Do you have anything further to add or any concerns?

I can take another look today and re-run the ITs.

I re-ran all the ITs, and they all pass, except for a few known flaky ones.

[ctubbsii]

I think this is probably mostly okay to merge now. However, I haven't done any manual testing... only code inspection and running tests. So, we should be careful to observe any behavioral changes to the shell as we continue development/testing on 2.1.

There is one more thing that does need to be done before this can be merged, though:

• the LICENSE file in the assembly tarball needs to be updated (assemble/src/main/resources/LICENSE) based on the upstream license information (there are only slight differences with https://github.com/jline/jline3/blob/jline-parent-3.17.1/LICENSE.txt

By the way, you may wish to consider updating to 3.19.0, in case some things have improved since you started working on this.

[Manno15]

I think this is probably mostly okay to merge now. However, I haven't done any manual testing... only code inspection and running tests. So, we should be careful to observe any behavioral changes to the shell as we continue development/testing on 2.1.

Understandable. Fortunately, most of the commands themselves have stayed the same. The things that have changed the most is tab completion and how the history file is set. Those I have tested thoroughly. The tests have also changed quite a bit and possible improvements could be made to those in future versions of Jline.

Another possible concern is how the terminal and reader will be set up on different machines. We let Jline determine the best terminal type and use the default implementations for other various features that maybe we would want to add later on. This isn't different from what we did in Jline2 so it isn't a big concern but I have already noticed several changes in functionality between ConsoleReader and the terminal/lineReader combo that is used now.

There is one more thing that does need to be done before this can be merged, though:
the LICENSE file in the assembly tarball needs to be updated (assemble/src/main/resources/LICENSE) based on the upstream license information (there are only slight differences with https://github.com/jline/jline3/blob/jline-parent-3.17.1/LICENSE.txt
By the way, you may wish to consider updating to 3.19.0, in case some things have improved since you started working on this

I will add these Monday. I am curious to see what they changed in 3.19.

[Manno15]

I updated the license and upgraded to 3.19.0. There wasn't anything noticeable to the upgrade that would benefit us so no further changes from it. I will merge this today unless there are any objections.