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

[java] Don't crash if none of the parsers can be initialized #288

Merged
merged 3 commits into from
Aug 21, 2024

Conversation

alexander-yakushev
Copy link
Member

@alexander-yakushev alexander-yakushev commented Aug 20, 2024

Context here: https://clojurians.slack.com/archives/C17JYSA3H/p1724172844138059

On the JRE, loading CIDER (and hence Orchard) produces an exception like this:

error in process sentinel: Could not start nREPL server: Downloading: cider/cider-nrepl/0.45.0/cider-nrepl-0.45.0.pom from clojars
Downloading: cider/orchard/0.22.0/orchard-0.22.0.pom from clojars
Downloading: mx/cider/logjam/0.2.0/logjam-0.2.0.pom from clojars
Downloading: cider/orchard/0.22.0/orchard-0.22.0.jar from clojars
Downloading: mx/cider/logjam/0.2.0/logjam-0.2.0.jar from clojars
Downloading: cider/cider-nrepl/0.45.0/cider-nrepl-0.45.0.jar from clojars
Execution error (IllegalArgumentException) at orchard.misc/require-and-resolve (misc.clj:161).
No such namespace: orchard.java.parser-utils

@alexander-yakushev alexander-yakushev force-pushed the parser-crash branch 4 times, most recently from fb643a2 to 08bf5f5 Compare August 20, 2024 18:45
@alexander-yakushev
Copy link
Member Author

alexander-yakushev commented Aug 20, 2024

The culprit was not the source-info triggering parser loads, but the def module-info line requiring parser-utils. Given that module-info was loaded a bit awkwardly already (and there was a comment affirming it), I've moved it into a separate namespace that can be safely loaded on any JDK after 8.

Another sidenote is the usage of "JDK9" in the names. Obviously, we won't change the existing ones for no reason, but I suggest we use JDK11 as the LTS version that we actually support.

@bbatsov
Copy link
Member

bbatsov commented Aug 21, 2024

I'm fine with updating the references to JDK 9 - they are there just because it introduced the module system (that probably no one is using still) and all the complexity that came with it. I think the LTS concept was added shortly afterward, but we forgot to update the references accordingly.

@@ -0,0 +1,7 @@
(ns orchard.java.jdk11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt there will be anything else here that's not related to modules, so this can also be something like orchard.java.modules. I'm not very fond of linking ns names to specific versions of something. But I don't feel strongly about this.

@alexander-yakushev alexander-yakushev changed the title [java] Don't crash if any of the parsers can't be initialized [java] Don't crash if none of the parsers can be initialized Aug 21, 2024
@alexander-yakushev alexander-yakushev force-pushed the parser-crash branch 3 times, most recently from dc50180 to e9ab24b Compare August 21, 2024 14:38
@alexander-yakushev
Copy link
Member Author

Replaced all mentions of JDK9.

@bbatsov bbatsov merged commit c813437 into master Aug 21, 2024
24 checks passed
@bbatsov bbatsov deleted the parser-crash branch August 21, 2024 19:57
@bbatsov
Copy link
Member

bbatsov commented Aug 21, 2024

This might be a good time for a new Orchard release.

@theronic
Copy link

theronic commented Nov 3, 2024

Which version of cider-nrepl includes this fix, please?

@alexander-yakushev
Copy link
Member Author

The latest one certainly does.

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