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

Module #1249

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

Module #1249

wants to merge 43 commits into from

Conversation

Thrameos
Copy link
Contributor

@Thrameos Thrameos commented Dec 2, 2024

This is a placeholder for module support. This is built off the noagent PR, so it will need to be rebased once that is included.

Key points:

  • Add a modulepath argument to startJVM to support modules.
  • Add a modules argument to allow the user to add modules
  • Adds a module-info to org.jpype
  • Starts the JVM with org.jpype on the module path and adds org.jpype to the list of initial modules.
  • After this change the class loader and context are under org.jpype rather than unnamed module.
  • The reflector used to launch from Python is currently under unnamed module. I am not sure yet if this causes problems.

Needs:

  • This needs to be tested extensively under non-ascii paths. We are going to hit all the same problems that we did with classpaths with the broken application loader and broken System.load command.
  • Need to test with javaFX to make sure we now satisfy the requirements for modules.
  • If the reflector MUST be in a named module we will have to rework the late loader. (required for JDBC and forName bugs)
  • There are questions as to how to support other module arguments "opens", "patch" etc though I believe these are not as high usage as the"adds".
  • Currently modules is connecting to "add-modules" which may cause problems with documentation. Though

Review will defer until after the 1.5.1 mess is dealt with.

raise ValueError("Unable to find ascii temp directory.")
shutil.copyfile(support_lib, path)
support_lib = path
tmp = path

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'tmp' is unnecessary as it is
redefined
before this value is used.
@@ -67,6 +67,15 @@
raise unittest.SkipTest("numpy required")
return f

def requireAscii(func):
def f(self):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns Note test

Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
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.

1 participant