ligurio / elle-cli

The command-line frontend to transactional consistency checkers for black-box databases
Other
27 stars 5 forks source link

Default to headless mode #50

Closed mprimi closed 2 years ago

mprimi commented 2 years ago

elle-cli should probably run by default in (Java) headless mode, since it's a command-line application without a GUI.

Currently, it does not and this creates a couple of inconveniences:

MacOS: when Elle-cli is running there is an application (e.g. shows in alt-tab list of running applications). And it steals focus from the currently active application. So if you have it running in background on a bunch of traces, it's impossible to do anything else, because it keeps going foreground.

Linux (Ubuntu 20.04 with java-default-headless installed):

Caused by: java.awt.HeadlessException:
No X11 DISPLAY variable was set, but this program performed an operation which requires it.
    at java.desktop/sun.awt.HeadlessToolkit.getMenuShortcutKeyMask(HeadlessToolkit.java:135)
    at rhizome.viz__init.load(Unknown Source)
    at rhizome.viz__init.<clinit>(Unknown Source)
    ... 126 more

There is a simple workaround that fixes both cases:

java -Djava.awt.headless=true -jar ${ELLE_CLI_JAR}

But it would be nice if Elle-cli was headless-ready out of the box.

More info: https://www.baeldung.com/java-headless-mode

ligurio commented 2 years ago

Thanks for reporting this!

Actually -Djava.awt.headless=true is passed in project.clj, see https://github.com/ligurio/elle-cli/blob/7970c5938baefa2669f1dc98bb2a497f008e950b/project.clj#L7. Seems it is not enough.

Regarding a problem with running on Ubuntu with installed java-default-headless - it is recommended to set DISPLAY, see https://www.ibm.com/support/pages/launcheradminsh-script-generates-no-x11-display-variable-was-set-program-performed-operation-which-requires-it-error I do this in CI, see https://github.com/ligurio/elle-cli/blob/7970c5938baefa2669f1dc98bb2a497f008e950b/.github/workflows/test.yaml#L59-L63

But it would be nice if elle-cli was headless-ready out of the box.

Agree. I left the issue unresolved.

mprimi commented 2 years ago

Actually -Djava.awt.headless=true is passed in project.clj, see

Ah, interesting. I am guessing the property gets lost in translation when creating the UberJar (or maybe it's set, but too late during lifecycle)?

Setting display seems like a workaround. It should not be necessary if headless mode was working as expected.

mprimi commented 2 years ago

Setting display seems like a workaround. It should not be necessary if headless mode was working as expected.

Confirmed this, see these PR tests passing on ubuntu after removing DISPLAY: https://github.com/ligurio/elle-cli/pull/51

So my theory is: setting :jvm-opts works for lein run and similar, but the headless property is lost when creating the uberjar.

And here's evidence this is correct:

  1. Add a print in the first statement of main:
(defn -main
  [& args]
  (println "HEADLESS? " (System/getProperty "java.awt.headless"))
  (try
  1. Run with lein:
    $ lein run
    Compiling elle_cli.cli
    HEADLESS?  true
    elle-cli - command-line transactional safety checker.

So far so good, headless mode property is passed, as expected. However:

  1. Build and run the uberjar:
    $ lein uberjar
    (...)
    $ java -jar target/elle-cli-0.1.3-standalone.jar
    HEADLESS?  nil
    elle-cli - command-line transactional safety checker.

So it does look like those :jvm-opts are not carried over to the uberjar. In this case you do want "java.awt.headless" to be always set in the uberjar.

I'm not familiar with lein or Clojure, but I'm sure there's multiple ways to achieve that (bundle a property file as resource, for example)

Once that's done, you could remove any hack you had to do in testing, because the uberjar will always run in headless mode (which is what you want for a pure CLI tool, I think).

ligurio commented 2 years ago

Thanks for research!

Once that's done, you could remove any hack you had to do in testing

I'll try to fix it.

mprimi commented 2 years ago

Thank you for the quick turnaround on this! @ligurio