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

Shell output going to stderr rather than stdout w/ jline-3.22.0 #3446

Closed
keith-ratcliffe opened this issue Jun 2, 2023 · 13 comments
Closed
Assignees
Labels
bug This issue has been verified to be a bug.
Milestone

Comments

@keith-ratcliffe
Copy link
Contributor

Describe the bug
Jline version bump from 3.21.0 to 3.22.0 seems to have changed the default behavior of the shell, such that expected output now goes to stderr rather than stdout

Versions (OS, Maven, Java, and others, as appropriate):

  • Affected version(s) of this project: 2.1.1-SNAPSHOT
  • JLine: 3.22.0
  • OS: CentOS 7.5
  • Java: 11

To Reproduce
Steps to reproduce the behavior (or a link to an example repository that reproduces the problem):

In a 2.1.1-SNAPSHOT with jline 3.22.0, do the following

 accumulo shell -u *** -p *** -e 'tables -np' 1>ashell.stdout 2>ashell.stderr

Observe that the expected output from the tables command goes to ashell.stderr

Expected behavior
Command output should be on stdout

Additional context

  • Downgrading to jline-3.21.0 seems to resolve the issue
  • The issue seems to persist with jine-3.23.0 as well
@keith-ratcliffe keith-ratcliffe added the bug This issue has been verified to be a bug. label Jun 2, 2023
@keith-ratcliffe
Copy link
Contributor Author

This commit is the likely culprit: jline/jline3@d6e84da
Not apparent to me yet how to restore the original behavior using 3.22.0

@dlmarion
Copy link
Contributor

dlmarion commented Jun 2, 2023

Looks like that change was just part of a bulk dependency upgrade in #3207 . We should be able to back that one out.

@ctubbsii
Copy link
Member

ctubbsii commented Jun 2, 2023

Looks like that change was just part of a bulk dependency upgrade in #3207 . We should be able to back that one out.

Before we back it out, though, I think we can try to figure out if there's a fix for it using the newer version, or we're going to be stuck on the older version, and not get the updated bugfixes.

@dlmarion
Copy link
Contributor

dlmarion commented Jun 2, 2023

So, jline/jline3#788 fixes jline/jline3#787. Based on what I'm reading, you only get one output stream now.

@dlmarion
Copy link
Contributor

dlmarion commented Jun 2, 2023

Even their 3.22.0 release notes say

Support for out or err stream for the terminal, fixes jline/jline3#787

@ctubbsii
Copy link
Member

ctubbsii commented Jun 6, 2023

I'm having trouble following the details of those issues, both in their intent and their result. However, a lot of the language is written in terms of "be able to" rather than "should do", so it sounds like they might have broken an existing capability while trying to add one. I feel like we should still be able to choose which output stream we write to from inside the shell. It seems that JLine would want to support that, and if it stopped working, it was probably an unintentional breakage that needs to be brought to their attention. However, as I said, I'm still trying to wrap my head around the new JLine stuff and what the intent and result was for those JLine issues/PRs.

@ctubbsii
Copy link
Member

ctubbsii commented Jun 7, 2023

@keith-ratcliffe wrote:

To Reproduce Steps to reproduce the behavior (or a link to an example repository that reproduces the problem):

In a 2.1.1-SNAPSHOT with jline 3.22.0, do the following

 accumulo shell -u *** -p *** -e 'tables -np' 1>ashell.stdout 2>ashell.stderr

Observe that the expected output from the tables command goes to ashell.stderr

I'm going to try to run this and generate my own output, to see the differences in behavior you're talking about, in order to try to track this down. However, what I observe and what you observed might be different. If you can, please provide example output of your observations, so we can be certain we're reproducing the issue you're reporting. Do you have an example command (and its corresponding output) that will generate output to both stdout and stderr with 3.21.0 for you, but only goes to one of them in 3.22.0?

@ctubbsii ctubbsii self-assigned this Jun 7, 2023
@jschmidt10
Copy link
Contributor

@ctubbsii I don't believe we have a case where both streams were leveraged. We were using the shell with some of our test utilities and parsing stdout for things like gathering table names. With JLine 3.22.0, now that it's defaulting to stderr, our utilities that were parsing the output no longer work without hacking in a 2>&1 everywhere we use the shell's output. So an example of something we would do is:

for table in $(accumulo shell -u *** -p *** -e "tables -np" | grep regression_test_); do
  // do something with your test table
done

@ctubbsii
Copy link
Member

ctubbsii commented Jun 9, 2023

@jschmidt10 That's fine. I was mostly just asking, so I could get a reliable command to reproduce and easily identify the separate messages expect to go to each, but I can try to figure it out with my own experimentation.

@ctubbsii
Copy link
Member

ctubbsii commented Jun 9, 2023

For what it's worth, one alternative would be to use jshell instead of our shell to do scripting. It's much more powerful and conveniently not limited to what we have implemented in our shell:

user@hostname$ $ACCUMULO_HOME/bin/accumulo jshell
Preparing JShell for Apache Accumulo

Use 'client' to interact with Accumulo

|  Welcome to JShell -- Version 11.0.19
|  For an introduction type: /help intro

jshell> client.tableOperations().list().stream().filter(t -> t != null).forEach(System.out::println)
accumulo.metadata
accumulo.root

jshell> 

Can also script it so you can get the output in bash:

user@hostname$ $ACCUMULO_HOME/bin/accumulo jshell -s <(echo 'client.tableOperations().list().stream().filter(t -> t != null).forEach(System.out::println)'; echo '/exit';) | tail -n +5

(the tail -n +5 is because we print some informative stuff when we create the client... could maybe improve that in future to be more silent when jshell is silent)

@jschmidt10
Copy link
Contributor

Thanks @ctubbsii. We have worked around it in the meantime with a 2>&1. We can check out jshell too. Thanks!

@ctubbsii
Copy link
Member

Reverted to 3.21.0 to resolve this for 2.1.1. Created #3485 to consider updating later, after more investigation.

@oneonestar
Copy link

oneonestar commented Jun 16, 2023

image
https://github.com/jline/jline3/blob/ccfb8c54fd9658b85a3d659f3abe85b8b4dcea83/terminal/src/main/java/org/jline/terminal/TerminalBuilder.java#L562
https://github.com/jline/jline3/blob/43c601471e53cca7532674867f826f830465126b/terminal/src/main/java/org/jline/terminal/TerminalBuilder.java#L448
https://github.com/jline/jline3/pull/788/files#diff-8f75c2bcaf44443b94e6a2a293331a90047529aabbc2d0e2bae5811ce22ad692L448
Facing the same issue in another project.
When both output and error are not tty, console becomes null. OutputStream will point to FileDescriptor.err.
The default behavior in the previous version was FileDescriptor.out. I think they might made an unintended incompatible change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue has been verified to be a bug.
Projects
None yet
Development

No branches or pull requests

5 participants