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

Simplify startJVM and support PathLike types for classpath and jvmpath arguments #1062

Merged
merged 2 commits into from
Apr 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions jpype/_classpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#
# *****************************************************************************
import os as _os
import typing

import _jpype

__all__ = ['addClassPath', 'getClassPath']
Expand All @@ -24,7 +26,7 @@
_SEP = _os.path.pathsep


def addClassPath(path1):
def addClassPath(path1: typing.Union[str, _os.PathLike]) -> None:
""" Add a path to the Java class path
Classpath items can be a java, a directory, or a
Expand Down Expand Up @@ -66,7 +68,7 @@ def addClassPath(path1):
_CLASSPATHS.append(path1)


def getClassPath(env=True):
def getClassPath(env: bool = True) -> str:
""" Get the full Java class path.
Includes user added paths and the environment CLASSPATH.
Expand All @@ -79,7 +81,7 @@ def getClassPath(env=True):
global _CLASSPATHS
global _SEP

# Merge the evironment path
# Merge the environment path
classPath = list(_CLASSPATHS)
envPath = _os.environ.get("CLASSPATH")
if env and envPath:
Expand Down
128 changes: 76 additions & 52 deletions jpype/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@
# See NOTICE file for details.
#
# *****************************************************************************
import sys
from __future__ import annotations
pelson marked this conversation as resolved.
Show resolved Hide resolved

import atexit
import os
import sys
import typing

import _jpype
from . import types as _jtypes
from . import _classpath
Expand Down Expand Up @@ -53,6 +58,10 @@ class JVMNotRunning(RuntimeError):
pass


if typing.TYPE_CHECKING:
_PathOrStr = typing.Union[str, os.PathLike]


# See http://scottlobdell.me/2015/04/decorators-arguments-python/
def deprecated(*args):
""" Marks a function a deprecated when used as decorator.
Expand Down Expand Up @@ -96,23 +105,48 @@ def isJVMStarted():
return _jpype.isStarted()


def _hasClassPath(args):
def _hasClassPath(args: typing.Tuple[_PathOrStr, ...]) -> bool:
for i in args:
if i.startswith('-Djava.class.path'):
if isinstance(i, str) and i.startswith('-Djava.class.path'):
return True
return False


def _handleClassPath(clsList):
def _handleClassPath(
classpath: typing.Union[
_PathOrStr,
typing.Tuple[_PathOrStr, ...]
marscher marked this conversation as resolved.
Show resolved Hide resolved
],
) -> str:
"""
Return a classpath which represents the given tuple of classpath specifications
"""
out = []
for s in clsList:
if not isinstance(s, str):
raise TypeError("Classpath elements must be strings")
if s.endswith('*'):

if isinstance(classpath, (str, os.PathLike)):
classpath = (classpath,)
try:
# Convert anything iterable into a tuple.
classpath = tuple(classpath)
pelson marked this conversation as resolved.
Show resolved Hide resolved
except TypeError:
raise TypeError("Unknown class path element")

for element in classpath:
try:
pth = os.fspath(element)
except TypeError as err:
raise TypeError("Classpath elements must be strings or Path-like") from err

if isinstance(pth, bytes):
marscher marked this conversation as resolved.
Show resolved Hide resolved
# In the future we may allow this to support Paths which are undecodable.
# https://docs.python.org/3/howto/unicode.html#unicode-filenames.
raise TypeError("Classpath elements must be strings or Path-like")

if pth.endswith('*'):
import glob
out.extend(glob.glob(s + '.jar'))
pelson marked this conversation as resolved.
Show resolved Hide resolved
out.extend(glob.glob(pth + '.jar'))
else:
out.append(s)
out.append(pth)
return _classpath._SEP.join(out)


Expand All @@ -123,7 +157,14 @@ def interactive():
return bool(getattr(sys, 'ps1', sys.flags.interactive))


def startJVM(*args, **kwargs):
def startJVM(
pelson marked this conversation as resolved.
Show resolved Hide resolved
*jvmargs: str,
jvmpath: typing.Optional[_PathOrStr] = None,
classpath: typing.Optional[_PathOrStr] = None,
ignoreUnrecognized: bool = False,
convertStrings: bool = False,
interrupt: bool = not interactive(),
) -> None:
"""
Starts a Java Virtual Machine. Without options it will start
the JVM with the default classpath and jvmpath.
Expand All @@ -132,14 +173,14 @@ def startJVM(*args, **kwargs):
The default jvmpath is determined by ``jpype.getDefaultJVMPath()``.

Parameters:
*args (Optional, str[]): Arguments to give to the JVM.
The first argument may be the path the JVM.
*jvmargs (Optional, str[]): Arguments to give to the JVM.
Copy link
Member

Choose a reason for hiding this comment

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

this is an API breaking change, which I'd like to avoid (without a prior deprecation).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see the breaking change part, could you elaborate?

Perhaps you missed the fact that it was proceeded with *, and therefore not a named argument in any way? There is no way that the name of the variable arguments component leaks out into the signature - I have changed it here entirely for readability reasons (since it really is just jvmargs that you can pass through).

Copy link
Member

Choose a reason for hiding this comment

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

absolutely, eventually I should just rest with my broken bone and leave this stuff to @Thrameos. Sorry.

Copy link
Contributor Author

@pelson pelson May 2, 2023

Choose a reason for hiding this comment

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

I think there may be a problem with this after-all. It was previously allowed to call startJVM(None)

There is a PR incoming.

The first argument may be the path to the JVM.

Keyword Arguments:
jvmpath (str): Path to the jvm library file,
jvmpath (str, PathLike): Path to the jvm library file,
Typically one of (``libjvm.so``, ``jvm.dll``, ...)
Using None will apply the default jvmpath.
classpath (str,[str]): Set the classpath for the JVM.
classpath (str, PathLike, [str, PathLike]): Set the classpath for the JVM.
This will override any classpath supplied in the arguments
list. A value of None will give no classpath to JVM.
ignoreUnrecognized (bool): Option to ignore
Expand All @@ -158,8 +199,7 @@ def startJVM(*args, **kwargs):

Raises:
OSError: if the JVM cannot be started or is already running.
TypeError: if an invalid keyword argument is supplied
or a keyword argument conflicts with the arguments.
TypeError: if a keyword argument conflicts with the positional arguments.

"""
if _jpype.isStarted():
Expand All @@ -168,51 +208,36 @@ def startJVM(*args, **kwargs):
if _JVM_started:
raise OSError('JVM cannot be restarted')

args = list(args)

# JVM path
jvmpath = None
if args:
if jvmargs:
# jvm is the first argument the first argument is a path or None
if not args[0] or not args[0].startswith('-'):
jvmpath = args.pop(0)
if 'jvmpath' in kwargs:
if jvmpath:
raise TypeError('jvmpath specified twice')
jvmpath = kwargs.pop('jvmpath')
if jvmargs[0] is not None and isinstance(jvmargs[0], str) and not jvmargs[0].startswith('-'):
if jvmpath:
raise TypeError('jvmpath specified twice')
jvmpath = jvmargs[0]
jvmargs = jvmargs[1:]

if not jvmpath:
jvmpath = getDefaultJVMPath()
else:
# Allow the path to be a PathLike.
jvmpath = os.fspath(jvmpath)

extra_jvm_args = tuple()

# Classpath handling
if _hasClassPath(args):
if _hasClassPath(jvmargs):
# Old style, specified in the arguments
if 'classpath' in kwargs:
if classpath is not None:
# Cannot apply both styles, conflict
raise TypeError('classpath specified twice')
classpath = None
elif 'classpath' in kwargs:
# New style, as a keywork
classpath = kwargs.pop('classpath')
else:
# Not speficied at all, use the default classpath
elif classpath is None:
# Not specified at all, use the default classpath.
classpath = _classpath.getClassPath()

# Handle strings and list of strings.
if classpath:
if isinstance(classpath, str):
args.append('-Djava.class.path=%s' % _handleClassPath([classpath]))
elif hasattr(classpath, '__iter__'):
args.append('-Djava.class.path=%s' % _handleClassPath(classpath))
else:
raise TypeError("Unknown class path element")

ignoreUnrecognized = kwargs.pop('ignoreUnrecognized', False)
convertStrings = kwargs.pop('convertStrings', False)
interrupt = kwargs.pop('interrupt', not interactive())

if kwargs:
raise TypeError("startJVM() got an unexpected keyword argument '%s'"
% (','.join([str(i) for i in kwargs])))
extra_jvm_args += (f'-Djava.class.path={_handleClassPath(classpath)}', )

try:
import locale
Expand All @@ -221,7 +246,7 @@ def startJVM(*args, **kwargs):
# Keep the current locale settings, else Java will replace them.
prior = [locale.getlocale(i) for i in categories]
# Start the JVM
_jpype.startup(jvmpath, tuple(args),
_jpype.startup(jvmpath, jvmargs + extra_jvm_args,
ignoreUnrecognized, convertStrings, interrupt)
# Collect required resources for operation
initializeResources()
Expand All @@ -238,8 +263,7 @@ def startJVM(*args, **kwargs):
match = re.search(r"([0-9]+)\.[0-9]+", source)
if match:
version = int(match.group(1)) - 44
raise RuntimeError("%s is older than required Java version %d" % (
jvmpath, version)) from ex
raise RuntimeError(f"{jvmpath} is older than required Java version{version}") from ex
raise


Expand Down
58 changes: 39 additions & 19 deletions test/jpypetest/test_startup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,20 @@
import jpype
import common
import subrun
import functools
import os
from pathlib import Path
import sys
import unittest


def runStartJVM(*args, **kwargs):
jpype.startJVM(*args, **kwargs)


@functools.wraps(jpype.startJVM)
pelson marked this conversation as resolved.
Show resolved Hide resolved
def runStartJVMTest(*args, **kwargs):
jpype.startJVM(*args, **kwargs)
try:
jclass = jpype.JClass('jpype.array.TestArray')
return
except:
pass
raise RuntimeError("Test class not found")
assert jpype.JClass('jpype.array.TestArray') is not None
except Exception as err:
raise RuntimeError("Test class not found") from err


root = os.path.dirname(os.path.abspath(os.path.dirname(__file__)))
Expand All @@ -57,17 +54,18 @@ def testRestart(self):
jpype.shutdownJVM()
jpype.startJVM(convertStrings=False)

def testJVMPathKeyword(self):
runStartJVM(jvmpath=self.jvmpath)

def testInvalidArgsFalse(self):
with self.assertRaises(RuntimeError):
runStartJVM("-for_sure_InVaLiD",
ignoreUnrecognized=False, convertStrings=False)
jpype.startJVM(
"-for_sure_InVaLiD",
ignoreUnrecognized=False, convertStrings=False,
)

def testInvalidArgsTrue(self):
runStartJVM("-for_sure_InVaLiD",
ignoreUnrecognized=True, convertStrings=False)
jpype.startJVM(
"-for_sure_InVaLiD",
ignoreUnrecognized=True, convertStrings=False,
)

def testClasspathArgKeyword(self):
runStartJVMTest(classpath=cp, convertStrings=False)
Expand All @@ -81,6 +79,16 @@ def testClasspathArgListEmpty(self):
def testClasspathArgDef(self):
runStartJVMTest('-Djava.class.path=%s' % cp, convertStrings=False)

def testClasspathArgPath(self):
runStartJVMTest(classpath=Path(cp), convertStrings=False)

def testClasspathArgPathList(self):
runStartJVMTest(classpath=[Path(cp)], convertStrings=False)

def testClasspathArgGlob(self):
jpype.startJVM(classpath=os.path.join(cp, '..', 'jar', 'mrjar*'))
marscher marked this conversation as resolved.
Show resolved Hide resolved
assert jpype.JClass('org.jpype.mrjar.A') is not None

def testClasspathTwice(self):
with self.assertRaises(TypeError):
runStartJVMTest('-Djava.class.path=%s' %
Expand All @@ -90,14 +98,26 @@ def testClasspathBadType(self):
with self.assertRaises(TypeError):
runStartJVMTest(classpath=1, convertStrings=False)

def testPathArg(self):
def testJVMPathArg_Str(self):
runStartJVMTest(self.jvmpath, classpath=cp, convertStrings=False)

def testPathKeyword(self):
path = jpype.getDefaultJVMPath()
def testJVMPathArg_Path(self):
with self.assertRaises(TypeError):
runStartJVMTest([
# Pass a path as the first argument. This isn't supported (this is
# reflected in the type definition), but the fact that it "works"
# gives rise to this test.
Path(self.jvmpath), cp], # type: ignore
marscher marked this conversation as resolved.
Show resolved Hide resolved
convertStrings=False,
)

def testJVMPathKeyword_str(self):
runStartJVMTest(classpath=cp, jvmpath=self.jvmpath,
convertStrings=False)

def testJVMPathKeyword_Path(self):
runStartJVMTest(jvmpath=Path(self.jvmpath), classpath=cp, convertStrings=False)

def testPathTwice(self):
with self.assertRaises(TypeError):
jpype.startJVM(self.jvmpath, jvmpath=self.jvmpath)
Expand Down