refactor: Separated JDK management code into its own module #1874

quintesse · 2024-11-27T13:12:42Z

src/main/java/dev/jbang/util/JavaUtil.java

maxandersen · 2025-02-09T08:05:40Z

small thing: how come the package is called .jvm when everything else is named JDK+something ?

build.gradle

maxandersen · 2025-02-09T08:12:18Z

src/main/java/dev/jbang/util/JBangFormatter.java

+ * This JUL Formatter is used to format JUL log messages in such a way that they
+ * look similar to JBang log messages.
+ */
+public class JBangFormatter extends Formatter {


hmm - originally I explicitly kept "ux output" separate from "logging output" ...is there a reason why we want to redirect all output to JUL ?

Well, you might not want all of the logging output to be confused with UX output, I can understand that.

But there are two things that come to mind:

our output mechanism sure does look like logging (having levels like warn, inifo, error, debug, etc)

to make the new module really independent from JBang I needed a way to generate output that didn't use JBang's system and I didn't want to introduce some new interface

So that's why I decided to switch the module's output to use plain logging but then the output would look different from before, so that's why I decided to capture the logging output and make it look like JBang output.

Now, I thought it would be nice to do the same for anything that could be outputting logging info, but that's just something I decided in the moment and we could easily change that to only work for the module and nothing else.

maxandersen · 2025-02-09T08:13:43Z

src/main/resources/logging.properties

@@ -0,0 +1,4 @@
+handlers=java.util.logging.ConsoleHandler


why do we need this now? (not against it - just want to understand the rationale)

It's just to make any logging appear as if it comes directly from JBang (with our normal "[jbang]" prefix for every line), which was needed to make the logging output from the new module appear exactly the same as it did before.

ok but:
a) what makes it end up in stderr not stdout?
b) doesn't this meddling with jul logging make it harder to embed?

a) what do you mean? all JBang messages go to stderr, right? (at least any of the xxxMsg() methods, which is what the JUL logging replaces here)

b) also not sure what you mean here. Using JUL logging is pretty normal, isn't it? You could use it just fine even with logging completely disabled. There's no essential output being generated using JUL.

quintesse · 2025-02-09T13:24:43Z

small thing: how come the package is called .jvm when everything else is named JDK+something ?

Historical reasons, because that was the tool is called that I want to use this module for. But it can of course be anything we want.

quintesse · 2025-03-05T16:23:47Z

Yay, all checks green! I've marked the PR as final now that devkitman has been published and all the tests pass.

build.gradle

quintesse · 2025-03-05T16:26:06Z

build.gradle

-	implementation 'com.offbytwo:docopt:0.6.0.20150202'
+	implementation 'dev.jbang:devkitman:0.1.2'
+
+//	implementation 'com.offbytwo:docopt:0.6.0.20150202'


This probably shouldn't be here, but it seemed unused to I commented it out and it slipped into the PR. If you want I can remove it from the PR.

lol - thats a ancient leftover from very early jbang...never needed. lets just nuke it.

quintesse · 2025-03-05T16:27:13Z

build.gradle

 shadowJar {
 	minimize() {
 		//exclude(dependency('org.slf4j:slf4j-api:.*'))
+		exclude(dependency('dev.jbang:devkitman:.*'))


devkitman uses services so this is needed or classes will be missing.

maxandersen · 2025-03-13T23:02:09Z

src/test/java/dev/jbang/BaseTest.java

 	@BeforeAll
 	static void init() throws URISyntaxException, IOException {
+		try {
+			// The default ConsoleHandler for logging doesn't like us changing


yeah so all this is why i refrained from using JUL for jbang output. why is this in the test class? shouldn't it be in main code path too?

But in this module JUL is only used for logging, there is no essential output being generated with it. It's a library, it shouldn't do any essential output by itself, it only returns information which the caller can then decide to output or not. The logging is really only used for logging.
Thing is we do have tests that specifically check for certain log lines, we shouldn't really be doing that but I didn't want to change the tests so I added this code to make sure that the output between monolithic JBang and modular JBang would remain 99.9% the same.

See jbangdev#1857

This reverts commit edf06a1.

* fix: stop using comprimised github action * Revert "refactor: Now using `devkitman` module (#1874)" This reverts commit edf06a1.

See jbangdev#1857

quintesse force-pushed the jdkman_lib branch from afc114d to f57b1c6 Compare November 27, 2024 13:14

quintesse changed the title ~~Separated JDK management code into its own module~~ refactor: Separated JDK management code into its own module Nov 27, 2024

maxandersen reviewed Nov 28, 2024

View reviewed changes

src/main/java/dev/jbang/util/JavaUtil.java Outdated Show resolved Hide resolved

quintesse force-pushed the jdkman_lib branch 2 times, most recently from 8c7ba7e to 4489268 Compare November 28, 2024 19:44

quintesse force-pushed the jdkman_lib branch 2 times, most recently from 2d66e04 to 21e1e5a Compare December 13, 2024 22:05

quintesse force-pushed the jdkman_lib branch 5 times, most recently from 260870f to 0f35aa5 Compare January 13, 2025 17:42

quintesse force-pushed the jdkman_lib branch 7 times, most recently from e0639c0 to f5e2bb3 Compare January 27, 2025 19:56

quintesse force-pushed the jdkman_lib branch 6 times, most recently from 1d133d2 to 43b8906 Compare February 5, 2025 23:04

maxandersen reviewed Feb 9, 2025

View reviewed changes

build.gradle Outdated Show resolved Hide resolved

maxandersen reviewed Feb 9, 2025

View reviewed changes

quintesse force-pushed the jdkman_lib branch from 43b8906 to 756ac41 Compare February 10, 2025 20:47

quintesse force-pushed the jdkman_lib branch 2 times, most recently from efdcb7a to 095ae63 Compare March 5, 2025 13:42

quintesse marked this pull request as ready for review March 5, 2025 13:46

quintesse force-pushed the jdkman_lib branch from 095ae63 to fa3279f Compare March 5, 2025 16:01

quintesse commented Mar 5, 2025

View reviewed changes

build.gradle Outdated Show resolved Hide resolved

quintesse commented Mar 5, 2025

View reviewed changes

quintesse force-pushed the jdkman_lib branch 3 times, most recently from c1b7b49 to cc0c494 Compare March 5, 2025 17:14

maxandersen reviewed Mar 13, 2025

View reviewed changes

quintesse force-pushed the jdkman_lib branch 2 times, most recently from 003435a to 1401ab1 Compare March 14, 2025 18:08

refactor: Now using devkitman module

433b2bb

See jbangdev#1857

quintesse force-pushed the jdkman_lib branch from 1401ab1 to 433b2bb Compare March 14, 2025 18:10

maxandersen approved these changes Mar 15, 2025

View reviewed changes

maxandersen merged commit edf06a1 into jbangdev:main Mar 15, 2025
10 checks passed

maxandersen added a commit to maxandersen/jbang that referenced this pull request Mar 16, 2025

Revert "refactor: Now using devkitman module (jbangdev#1874)"

44c1995

This reverts commit edf06a1.

maxandersen added a commit that referenced this pull request Mar 16, 2025

fix: stop using comprimised github action (#1952)

a504967

* fix: stop using comprimised github action * Revert "refactor: Now using `devkitman` module (#1874)" This reverts commit edf06a1.

quintesse mentioned this pull request Mar 26, 2025

Refactor JDK management code into separate module #1857

Closed

quintesse added a commit to quintesse/jbang that referenced this pull request Mar 28, 2025

refactor: Now using devkitman module (jbangdev#1874)

1c7122e

See jbangdev#1857

quintesse added a commit to quintesse/jbang that referenced this pull request Apr 19, 2025

refactor: Now using devkitman module (jbangdev#1874)

ba9c239

See jbangdev#1857

quintesse added a commit to quintesse/jbang that referenced this pull request Apr 29, 2025

refactor: Now using devkitman module (jbangdev#1874)

8f8e68b

See jbangdev#1857

maxandersen pushed a commit to quintesse/jbang that referenced this pull request May 3, 2025

refactor: Now using devkitman module (jbangdev#1874)

83a96e6

See jbangdev#1857

maxandersen pushed a commit to quintesse/jbang that referenced this pull request May 3, 2025

refactor: Now using devkitman module (jbangdev#1874)

3e5bf1f

See jbangdev#1857

Uh oh!

refactor: Separated JDK management code into its own module #1874

refactor: Separated JDK management code into its own module #1874

Uh oh!

Conversation

quintesse commented Nov 27, 2024

Uh oh!

Uh oh!

maxandersen commented Feb 9, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quintesse commented Feb 9, 2025

Uh oh!

quintesse commented Mar 5, 2025

Uh oh!

Uh oh!

quintesse Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

quintesse Mar 5, 2025 •

edited

Loading