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

Update jline to 3.23.0 #986

Closed
wants to merge 7 commits into from

Conversation

scala-steward
Copy link
Contributor

@scala-steward scala-steward commented Mar 10, 2023

About this PR

📦 Updates org.jline:jline from 3.22.0 to 3.23.0

Usage

Please merge!

I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.

If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.

Configure Scala Steward for your repository with a .scala-steward.conf file.

Have a fantastic day writing Scala!

⚙ Adjust future updates

Add this to your .scala-steward.conf file to ignore future updates of this dependency:

updates.ignore = [ { groupId = "org.jline", artifactId = "jline" } ]

Or, add this to slow down future updates of this dependency:

dependencyOverrides = [{
  pullRequests = { frequency = "30 days" },
  dependency = { groupId = "org.jline", artifactId = "jline" }
}]
labels: library-update, early-semver-minor, semver-spec-minor, commit-count:1

@tuxji
Copy link
Contributor

tuxji commented Mar 10, 2023

For whoever is troubleshooting, this error is reproducible when the branch is locally checked out, but it puzzles me why one jline class cannot find a method of another jline class at runtime. A new method was added to a jline class and another jline class called it. The new method should be callable since both classes are packaged in the same jar, yet the call is failing for some reason I haven't been able to find:

java.lang.NoSuchMethodError: 'org.jline.utils.AttributedString org.jline.utils.AttributedString.fromAnsi(java.lang.String, java.util.List, java.lang.String, java.lang.String)'
	at org.jline.reader.impl.LineReaderImpl.fromAnsi(LineReaderImpl.java:4188)

If you check the diffs for terminal/src/main/java/org/jline/utils/AttributedString.java in jline/jline3@jline-parent-3.22.0...jline-parent-3.23.0, you will see that method was added to that class and the other class was modified to call that new method as well. I have downloaded the jline-3.23.0 jar, confirmed that both classes are in the jar, and verified that the new method is present in the jline class's public methods:

interran@GH3WPL13E:~/jline$ javap -public org/jline/utils/AttributedString.class
Compiled from "AttributedString.java"
public class org.jline.utils.AttributedString extends org.jline.utils.AttributedCharSequence {
  ...
  public static org.jline.utils.AttributedString fromAnsi(java.lang.String, java.util.List<java.lang.Integer>, java.lang.String, java.lang.String);
  ...
}

Good luck.

@stevedlawrence
Copy link
Member

I was looking at this and was equally stumped. But I found that manually running the CLI debugger worked, which makes me wonder if this is an SBT classloader issue? SBT ships and uses an older version of jline. The stack trace shows that we are stepping into the new jline code that you've pointed out, but maybe when it looks up the AttributedString class its somehow finding the older one that SBT is using?

Or maybe Jline is doing something funky with classloaders? Like its caching something that that it found with SBT, and then reusing the old cached when when called from Daffodil?

I'm not sure what the right solution would be to fix this though. We could probably change or CLI debuger tests to fork which should avoid the SBT classpath entirely, but I think that will increase our build times quite a bit....

@stevedlawrence
Copy link
Member

Confirmed the above is what's happening. I added this to the CLIDebuggerRunner before it calls reader.get.readLine:

          val class1 = classOf[org.jline.utils.AttributedString]
          val jar1 = this.getClass.getResource('/' + class1.getName.replace('.', '/') + ".class")
          System.err.println(s"$jar1")

          val class2 = classOf[org.jline.reader.impl.LineReaderImpl]
          val jar2 = this.getClass.getResource('/' + class2.getName.replace('.', '/') + ".class")
          System.err.println(s"$jar2")

And the output is:

jar:file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jline-terminal-3.19.0.jar!/org/jline/utils/AttributedString.class
jar:file:<removed>/daffodil.git/lib_managed/jars/org.jline/jline/jline-3.23.0.jar!/org/jline/reader/impl/LineReaderImpl.class

So for some reason the AttributedString class we find is from what sbt uses, and the LineReaderImpl we find is what we actually depend on. I still don't know of a good fix, but this proably confrims that it's not a jline thing since we are probably using the sbt classloader at this point. This is likely a long time bug that we just haven't noticed because the classes we incorrectly get from sbt are close enough to what our jline version uses.

@tuxji
Copy link
Contributor

tuxji commented Mar 10, 2023

If sbt's own jars can interfere with Daffodil's classpath while running unit tests, I am concerned because I have found several places where we could have more problems as well.

I compared each jar in sbt's boot classpath with each jar in Daffodil's lib classpath and I found:

  • 4 jars with identical versions right now, but which could change later
  1. <daffodil.>/com.typesafe.config-1.4.2.jar
    <sbt-1.8.2>/config-1.4.2.jar

  2. <daffodil.>/org.scala-lang.modules.scala-xml_2.12-2.1.0.jar
    <sbt-1.8.2>/scala-xml_2.12-2.1.0.jar

  3. <daffodil.>/org.scala-lang.scala-library-2.12.17.jar
    <sbt-1.8.2>/scala-library-2.12.17.jar

  4. <daffodil.>/org.scala-lang.scala-reflect-2.12.17.jar
    <sbt-1.8.2>/scala-reflect-2.12.17.jar

  • 4 jars with different versions right now, which could be causing more problems
  1. <daffodil.>/org.fusesource.jansi.jansi-2.4.0.jar
    <sbt-1.8.2>/jansi-2.1.0.jar

  2. <daffodil.>/org.jline.jline-3.22.0.jar
    <sbt-1.8.2>/jline-builtins-3.19.0.jar
    <sbt-1.8.2>/jline-reader-3.19.0.jar
    <sbt-1.8.2>/jline-style-3.19.0.jar
    <sbt-1.8.2>/jline-terminal-3.19.0.jar
    <sbt-1.8.2>/jline-terminal-jansi-3.19.0.jar
    <sbt-1.8.2>/jline-terminal-jna-3.19.0.jar

Note that our jline jar combines classes from ALL jline jars in one place -
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-builtins/
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-reader/
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-remote-ssh/
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-remote-telnet/
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-style/
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-terminal/
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-terminal-jansi/
drwxr-xr-x 0 17-Jan-2023 16:55:04 META-INF/maven/org.jline/jline-terminal-jna/

  1. <daffodil.>/org.scala-lang.modules.scala-parser-combinators_2.12-2.2.0.jar
    <sbt-1.8.2>/scala-parser-combinators_2.12-1.1.2.jar

  2. <daffodil.>/org.slf4j.slf4j-api-2.0.6.jar
    <sbt-1.8.2>/slf4j-api-1.7.36.jar

@stevedlawrence
Copy link
Member

Yeah, this could be bad...

Some more information, if I output class1.getClassLoader and class2.getClassLoader we get this for AttributedString

JLineLoader(
  file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jna-5.12.0.jar,
  file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jline-2.14.7-sbt-a1b0ffbb8f64bb820f4f84a0c07a0c0964507493.jar,
  file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jline-terminal-jna-3.19.0.jar,
  file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jline-terminal-3.19.0.jar,
  file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jansi-2.1.0.jar,
  file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jna-platform-5.12.0.jar,
  file:<removed>/.sbt/boot/scala-2.12.17/org.scala-sbt/sbt/1.8.2/jline-terminal-jansi-3.19.0.jar
)

And this for LineReaderImpl

sbt.internal.LayeredClassLoader@6933e5a6

JLineLoader is some sbt thing. Fortunately it doesn't have all those other jars you mentioned, so this is potentially only an issue with these specific jars (not confirmed though). Still not great, but maybe limited to just jline related things. Not sure if it's possible to disable the JLineLoader though.

@tuxji
Copy link
Contributor

tuxji commented Mar 10, 2023

I checked sbt's options (sbt --help) in case an option might disable the JLineLoader. I tried both sbt --batch daffodil-cli/test and sbt --batch --no-colors daffodil-cli/test but I still got the same exceptions. I noticed that sbt also has a --sbt-boot <path> option so I copied ~/.sbt/boot to ~/sbtboot and replaced the jansi and jline jars with Daffodil's jansi and jline jars from its lib_managed directories. Replacing the individual jline jars with the all-in-one jline jar crashed sbt's JLineLoader class:

interran@GH3WPL13E:~/apache/daffodil-tuxji$ sbt --sbt-boot ~/sbtboot daffodil-cli/test
        ... 
Caused by: java.lang.NullPointerException
        at java.base/java.util.ArrayDeque.addLast(ArrayDeque.java:304)
        at java.base/java.util.ArrayDeque.add(ArrayDeque.java:495)
        at java.base/jdk.internal.loader.URLClassPath.<init>(URLClassPath.java:155)
        at java.base/jdk.internal.loader.URLClassPath.<init>(URLClassPath.java:172)
        at java.base/java.net.URLClassLoader.<init>(URLClassLoader.java:120)
        at sbt.internal.JLineLoader.<init>(JLineLoader.java:15)
        ... 

Adding the individual jline 2.23.0 jars gets us past JLineLoader, but still crashes sbt later when it tries to call a now-missing jline class:

interran@GH3WPL13E:~/apache/daffodil-tuxji$ sbt --sbt-boot ~/sbtboot daffodil-cli/test
java.lang.NoClassDefFoundError: org/jline/terminal/impl/jansi/JansiSupportImpl
        at sbt.internal.util.JLine3$.<init>(JLine3.scala:52)
        ... 

So sbt would need source changes before the newer jline jars could be dropped into its boot path. The whole idea was just to see if changing the boot path would even work, not use it as a workaround anyway.

@stevedlawrence
Copy link
Member

Yeah, seems like a real bug in sbt. I've opened a ticket here sbt/sbt#7177, hoping they will have a solution...

@scala-steward
Copy link
Contributor Author

Superseded by #1103.

@scala-steward scala-steward deleted the update/jline-3.23.0 branch October 26, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants