Wrong name mangling. V8Utils => v8utils instead of v8.utils #3493

Closed
Gonzih opened this Issue Nov 24, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@Gonzih

Gonzih commented Nov 24, 2015

Hello,

I found small issue with ruby => java package name mangling.
Here is an example:

Java::ComEclipsesourceV8Utils::V8Executor
NameError: cannot load Java class com.eclipsesource.v8utils.V8Executor
from org/jruby/javasupport/JavaClass.java:204:in `for_name'

V8Executor is located inside com.eclipsesource.v8.utils package.

Thanks!

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 24, 2015

Member

I suspect there's just a bad regex looking for [a-z][A-Z] to split rather than including other valid package characters in the first part.

Member

headius commented Nov 24, 2015

I suspect there's just a bad regex looking for [a-z][A-Z] to split rather than including other valid package characters in the first part.

@Gonzih

This comment has been minimized.

Show comment
Hide comment
@Gonzih

Gonzih Nov 24, 2015

Also have similar problem that might be related:

java_import com.eclipsesource.v8.V8
NameError: missing class name (`com.eclipsesource.v8.V8')
               get_proxy_or_package_under_package at org/jruby/javasupport/JavaUtilities.java:54

Is this related or is this a different issue entirely?

Gonzih commented Nov 24, 2015

Also have similar problem that might be related:

java_import com.eclipsesource.v8.V8
NameError: missing class name (`com.eclipsesource.v8.V8')
               get_proxy_or_package_under_package at org/jruby/javasupport/JavaUtilities.java:54

Is this related or is this a different issue entirely?

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 24, 2015

Member

Is that the correct class + package for that V8 class? The dotted form is what we recommend users use to access Java classes, and I don't see a reason this would fail.

Member

headius commented Nov 24, 2015

Is that the correct class + package for that V8 class? The dotted form is what we recommend users use to access Java classes, and I don't see a reason this would fail.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 24, 2015

Member

I have a trivial fix for the main issue:

diff --git a/core/src/main/java/org/jruby/javasupport/Java.java b/core/src/main/java/org/jruby/javasupport/Java.java
index 9ba8ba4..8108217 100644
--- a/core/src/main/java/org/jruby/javasupport/Java.java
+++ b/core/src/main/java/org/jruby/javasupport/Java.java
@@ -766,7 +766,7 @@ public class Java implements Library {
         return packageModule;
     }

-    private static final Pattern CAMEL_CASE_PACKAGE_SPLITTER = Pattern.compile("([a-z][0-9]*)([A-Z])");
+    private static final Pattern CAMEL_CASE_PACKAGE_SPLITTER = Pattern.compile("([a-z0-9]+)([A-Z])");

     private static RubyModule getPackageModule(final Ruby runtime, final String name) {
         final RubyModule javaModule = runtime.getJavaSupport().getJavaModule();
Member

headius commented Nov 24, 2015

I have a trivial fix for the main issue:

diff --git a/core/src/main/java/org/jruby/javasupport/Java.java b/core/src/main/java/org/jruby/javasupport/Java.java
index 9ba8ba4..8108217 100644
--- a/core/src/main/java/org/jruby/javasupport/Java.java
+++ b/core/src/main/java/org/jruby/javasupport/Java.java
@@ -766,7 +766,7 @@ public class Java implements Library {
         return packageModule;
     }

-    private static final Pattern CAMEL_CASE_PACKAGE_SPLITTER = Pattern.compile("([a-z][0-9]*)([A-Z])");
+    private static final Pattern CAMEL_CASE_PACKAGE_SPLITTER = Pattern.compile("([a-z0-9]+)([A-Z])");

     private static RubyModule getPackageModule(final Ruby runtime, final String name) {
         final RubyModule javaModule = runtime.getJavaSupport().getJavaModule();
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 24, 2015

Member

What version of JRuby are you using? We could fix this in both 1.7 and 9 but we'd prefer to just fix it in 9 since it may change the way existing code resolves packages (even if they do happen to be using the WRONG way, we don't want to break people's code).

Member

headius commented Nov 24, 2015

What version of JRuby are you using? We could fix this in both 1.7 and 9 but we'd prefer to just fix it in 9 since it may change the way existing code resolves packages (even if they do happen to be using the WRONG way, we don't want to break people's code).

@Gonzih

This comment has been minimized.

Show comment
Hide comment
@Gonzih

Gonzih Nov 24, 2015

I'm using 9.0.4.0, thanks for the fix! I will look at the java import issue closer.

Gonzih commented Nov 24, 2015

I'm using 9.0.4.0, thanks for the fix! I will look at the java import issue closer.

kares added a commit that referenced this issue Nov 25, 2015

Merge branch 'master' into ruby-2.3
* master: (29 commits)
  [travis-ci] revert jdk 9 testing from 2fa04aa
  Eliminate Block.Type arg in BlockBody yield/call signatures
  Pass Block instead of Binding in BlockBody.yield/call
  Minor: fix typo in name of runtime helper instr method name
  Update to jnr-unixsocket 0.10.
  Improve CamelCase package splitting. Fixes #3493.
  Update to jnr-unixsocket 0.10-SNAPSHOT for forFD.
  [Truffle] j+tr: support for passing any extra ruby options
  each_object(cls.singleton_class) should not walk special classes.
  This test has been moved to RubySpec.
  [Truffle] Typo.
  [Truffle] Tag failing regex specs.
  [Truffle] Tag failing Time.at spec.
  The bin200 distribution has grown in size.
  [Truffle] Fixed two typos of the same word on the same line of code.
  [Truffle] Simplified the fix in 5446cc2.
  [Truffle] Fixed an NPE when a SharedMethodInfo has a null argumentsDescriptors.
  [Truffle] Fixed the interpreter_path when the root JRuby distribution directory is not named "jruby."
  Fixes #3483. define_method with empty body throws RuntimeError in interpreter
  Test main on Java 9
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment