Skip to content

Commit ef44c0b

Browse files
committed
Extension loading from classpath #71
moving extra classpath loading outside of the kernel startup and into the JJava.main(..) logic
1 parent 4a051de commit ef44c0b

5 files changed

Lines changed: 22 additions & 54 deletions

File tree

jjava-distro/src/main/java/org/dflib/jjava/distro/JJava.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ public static void main(String[] args) throws Exception {
5555
.extensionsEnabled(Env.extensionsEnabled())
5656
.startupSnippets(Env.startupSnippets())
5757
.compilerOpts(Env.compilerOpts())
58-
.extraClasspath(Env.extraClasspath())
5958
.timeout(Env.timeout())
6059

6160
.lineMagic("load", new LoadCodeMagic("", ".jsh", ".jshell", ".java", ".jjava"))
@@ -72,9 +71,14 @@ public static void main(String[] args) throws Exception {
7271

7372
.build();
7473

74+
// this inits built-in Extensions
7575
kernel.onStartup();
76-
kernel.becomeHandlerForConnection(connection);
7776

77+
// adds custom jars to classpath, loads Extensions from them
78+
kernel.addToClasspath(Env.extraClasspath());
79+
80+
// connect to Jupyter
81+
kernel.becomeHandlerForConnection(connection);
7882
connection.connect();
7983
connection.waitUntilClose();
8084
}

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernel.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ public abstract class BaseKernel {
8080
protected final MagicsRegistry magicsRegistry;
8181
protected final Map<String, Extension> extensions;
8282
protected final boolean extensionsEnabled;
83-
protected final String extraClasspath;
8483
protected final StringStyler errorStyler;
8584
protected final AtomicInteger executionCount;
8685

@@ -96,7 +95,6 @@ protected BaseKernel(
9695
MagicParser magicParser,
9796
MagicsRegistry magicsRegistry,
9897
boolean extensionsEnabled,
99-
String extraClasspath,
10098
StringStyler errorStyler) {
10199

102100
this.name = name;
@@ -113,7 +111,6 @@ protected BaseKernel(
113111
this.magicParser = magicParser;
114112
this.magicsRegistry = magicsRegistry;
115113
this.extensionsEnabled = extensionsEnabled;
116-
this.extraClasspath = extraClasspath;
117114
this.extensions = new ConcurrentHashMap<>();
118115
this.errorStyler = Objects.requireNonNull(errorStyler);
119116

@@ -288,13 +285,6 @@ public DisplayData eval(String expr) {
288285
public void onStartup() {
289286
if (extensionsEnabled) {
290287
installDefaultExtensions();
291-
292-
if (extraClasspath != null) {
293-
// TODO: ClassLoader with "extraClasspath" will inherit system classpath, so maybe if there is an
294-
// "extraClasspath", we don't need to call installDefaultExtensions() ?
295-
296-
installExtensionsFromClasspath(extraClasspath);
297-
}
298288
}
299289
}
300290

jjava-kernel/src/main/java/org/dflib/jjava/kernel/JavaKernel.java

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.dflib.jjava.jupyter.kernel.magic.MagicTranspiler;
2121
import org.dflib.jjava.jupyter.kernel.magic.MagicsRegistry;
2222
import org.dflib.jjava.jupyter.kernel.util.CharPredicate;
23+
import org.dflib.jjava.jupyter.kernel.util.PathsHandler;
2324
import org.dflib.jjava.jupyter.kernel.util.StringStyler;
2425
import org.dflib.jjava.kernel.execution.CodeEvaluator;
2526
import org.dflib.jjava.kernel.execution.CompilationException;
@@ -31,7 +32,6 @@
3132
import java.nio.charset.Charset;
3233
import java.util.ArrayList;
3334
import java.util.List;
34-
import java.util.Objects;
3535
import java.util.Set;
3636
import java.util.regex.Matcher;
3737
import java.util.regex.Pattern;
@@ -75,7 +75,6 @@ protected JavaKernel(
7575
MagicParser magicParser,
7676
MagicsRegistry magicsRegistry,
7777
boolean extensionsEnabled,
78-
String extraClasspath,
7978
StringStyler errorStyler,
8079
JShell jShell,
8180
CodeEvaluator evaluator) {
@@ -92,7 +91,6 @@ protected JavaKernel(
9291
magicParser,
9392
magicsRegistry,
9493
extensionsEnabled,
95-
extraClasspath,
9694
errorStyler);
9795

9896
this.jShell = jShell;
@@ -105,11 +103,14 @@ protected JavaKernel(
105103
* @param classpath one or more filesystem paths separated by {@link java.io.File#pathSeparator}.
106104
*/
107105
public void addToClasspath(String classpath) {
108-
Objects.requireNonNull(classpath, "Null classpath");
109-
jShell.addToClasspath(classpath);
106+
if (classpath == null || classpath.isBlank()) {
107+
return;
108+
}
110109

110+
String classpathResolved = PathsHandler.joinPaths(PathsHandler.splitAndResolveGlobs(classpath));
111+
jShell.addToClasspath(classpathResolved);
111112
if (extensionsEnabled) {
112-
installExtensionsFromClasspath(classpath);
113+
installExtensionsFromClasspath(classpathResolved);
113114
}
114115
}
115116

@@ -379,7 +380,6 @@ public JavaKernel build() {
379380
buildMagicParser(magicTranspiler),
380381
buildMagicsRegistry(),
381382
buildExtensionsEnabled(),
382-
buildExtraClasspath(),
383383
buildErrorStyler(),
384384
jShell,
385385
buildCodeEvaluator(jShell, jShellExecutionControlProvider)
@@ -392,9 +392,5 @@ protected List<HelpLink> buildHelpLinks() {
392392
new HelpLink("JJava homepage", "https://github.com/dflib/jjava")
393393
);
394394
}
395-
396-
private String buildExtraClasspath() {
397-
return extraClasspath;
398-
}
399395
}
400396
}

jjava-kernel/src/main/java/org/dflib/jjava/kernel/JavaKernelBuilder.java

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@
55
import org.dflib.jjava.jupyter.kernel.LanguageInfo;
66
import org.dflib.jjava.jupyter.kernel.magic.MagicParser;
77
import org.dflib.jjava.jupyter.kernel.magic.MagicTranspiler;
8-
import org.dflib.jjava.jupyter.kernel.util.PathsHandler;
98
import org.dflib.jjava.kernel.execution.CodeEvaluator;
109
import org.dflib.jjava.kernel.execution.JJavaExecutionControlProvider;
1110

12-
import java.io.File;
1311
import java.util.ArrayList;
1412
import java.util.HashMap;
1513
import java.util.List;
@@ -28,7 +26,6 @@ public abstract class JavaKernelBuilder<
2826
protected String timeout;
2927
protected final List<String> startupSnippets;
3028
protected final List<String> compilerOpts;
31-
protected String extraClasspath;
3229

3330
protected JavaKernelBuilder() {
3431
this.jShellExecControlID = UUID.randomUUID().toString();
@@ -54,16 +51,6 @@ public B compilerOpts(Iterable<String> opts) {
5451
return (B) this;
5552
}
5653

57-
public B extraClasspath(String classpath) {
58-
if (classpath != null && !classpath.isEmpty()) {
59-
this.extraClasspath = this.extraClasspath != null
60-
? this.extraClasspath + File.pathSeparator + classpath
61-
: classpath;
62-
}
63-
64-
return (B) this;
65-
}
66-
6754
/**
6855
* Sets the kernel communication timeout. The String should be a number with a {@link java.util.concurrent.TimeUnit}
6956
* name. E.g. "30 SECONDS"
@@ -85,24 +72,13 @@ protected JShell buildJShell(JJavaExecutionControlProvider jShellExecControlProv
8572
execControlParams.put(JJavaExecutionControlProvider.TIMEOUT_KEY, timeout);
8673
}
8774

88-
JShell shell = JShell.builder()
75+
return JShell.builder()
8976
.out(System.out)
9077
.err(System.err)
9178
.in(System.in)
9279
.executionEngine(jShellExecControlProvider, execControlParams)
9380
.compilerOptions(compilerOpts.toArray(new String[0]))
9481
.build();
95-
96-
if (extraClasspath != null) {
97-
98-
String extraClasspathResolved = PathsHandler.joinPaths(PathsHandler.splitAndResolveGlobs(extraClasspath));
99-
100-
// TODO: not loading JJava extensions from classpath here.
101-
// Is this the cause of https://github.com/dflib/jjava/issues/71 ?
102-
shell.addToClasspath(extraClasspathResolved);
103-
}
104-
105-
return shell;
10682
}
10783

10884
protected CodeEvaluator buildCodeEvaluator(JShell jShell, JJavaExecutionControlProvider jShellExecControlProvider) {

jjava-kernel/src/test/java/org/dflib/jjava/kernel/ExtensionsLifecycleTest.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,27 +39,29 @@ void extraClasspathExtensions() throws Exception {
3939
String extraClasspath = PathsHandler.joinPaths(PathsHandler.resolveGlobs(jar.toAbsolutePath().toString()));
4040

4141
String extensionClassName = "org.dflib.jjava.kernel.test.TestExtension";
42-
String installationsProperty = extensionInstallationsProperty(extensionClassName);
43-
System.clearProperty(installationsProperty);
42+
String extInstalledProp = extensionInstallationsProperty(extensionClassName);
43+
System.clearProperty(extInstalledProp);
4444

4545
JavaKernel kernel = JavaKernel
4646
.builder()
4747
.name("TestKernel")
48-
.extraClasspath(extraClasspath)
4948
.build();
5049
try {
5150
kernel.onStartup();
52-
assertEquals("1", System.getProperty(installationsProperty), "Custom extension was not installed");
5351
assertNotNull(JavaNotebookStatics.kernel, "Built-in extension was not installed");
52+
assertNull(System.getProperty(extInstalledProp), "Custom extension should not be installed yet");
5453

5554
kernel.addToClasspath(extraClasspath);
56-
assertEquals("1", System.getProperty(installationsProperty));
55+
assertEquals("1", System.getProperty(extInstalledProp), "Custom extension was not installed");
56+
57+
kernel.addToClasspath(extraClasspath);
58+
assertEquals("1", System.getProperty(extInstalledProp), "Custom extension was installed more than once?");
5759
} finally {
5860
kernel.onShutdown(false);
5961
}
6062

6163
assertNull(JavaNotebookStatics.kernel);
62-
assertNull(System.getProperty(installationsProperty));
64+
assertNull(System.getProperty(extInstalledProp));
6365
}
6466

6567
private static String extensionInstallationsProperty(String className) {

0 commit comments

Comments
 (0)