You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ant.apache.org by bu...@apache.org on 2015/08/22 11:57:56 UTC

[Bug 58271] New: script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

            Bug ID: 58271
           Summary: script task implementation should use reflection API
                    to find if javax.script.ScriptEngineManager is
                    available
           Product: Ant
           Version: 1.9.5
          Hardware: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Optional Tasks
          Assignee: notifications@ant.apache.org
          Reporter: a.sundararajan@gmail.com

Based on reading 1.9.5 sources:

http://grepcode.com/file/repo1.maven.org/maven2/org.apache.ant/ant/1.9.5/org/apache/tools/ant/util/ScriptRunnerCreator.java#ScriptRunnerCreator.createRunner%28java.lang.String%2Cjava.lang.String%2Cjava.lang.String%29

http://grepcode.com/file/repo1.maven.org/maven2/org.apache.ant/ant/1.9.5/org/apache/tools/ant/util/LoaderUtils.java#LoaderUtils.classNameToResource%28java.lang.String%29

I find that <script> task implementation uses ClassLoader.getResource is used
to locate a .class resource URL. Based on whether that returns null or not,
ScriptRunnerBase is created. But per documentation of ClassLoader.getResource
(http://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#getResource-java.lang.String-),
getResource is meant for only audio/text etc. -  / A resource is some data
(images, audio, text, etc) that can be accessed by class code in a way that is
independent of the location of the code./ Never talks about ".class" 'file' as
a resource. Besides, reflection API may be used to check if
javax.script.ScriptEngineManager class is available or not in the JDK.
ClassLoader.getResource on .class resource can be avoided.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

--- Comment #4 from a.sundararajan@gmail.com ---
(In reply to Stefan Bodewig from comment #3)
> TBH I don't remember why the code was written the way it is and haven't been
> involved with writing it at all - just wanted to avoid fixing code for
> cosmetic reasons that wasn't broken.
> 
> Looking at the code my guess is the special case for BSF that immediately
> before the Class.forName call is required for it to work when using BSF and
> the code needs a way to ensure BSF is actually there before modifying the
> classloader hierarchy.
> 
> The problem we should really talk about is LoaderUtils.classNameToResource
> which is also used inside the JUnit task.  We use it to tell people we've
> found multiple versions of JUnit.  Not only that, we also tell them where
> we've found them.  How would we achieve the same with using reflection alone?
> 
> The LoaderUtils.classExists that uses LoaderUtils.classNameToResource is
> only used inside the BSF classloader hack so shouldn't be a problem if we
> manage to fix the hack.

ClassLoader.getResource behaviour change is *only* about classes in named
modules (like javax.script.ScriptEngineManager and other JDK platform classes
which will be in named modules). The use ClassLoader.getResource for other
classes like junit or anything that is deployed via classpath or located via
some custom class loader - is not be a problem. That should work fine as it
does in the current JDK versions. So, if the use of ClassLoader.getResource for
platform classes can be avoided, ant tasks would be fine.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

--- Comment #6 from a.sundararajan@gmail.com ---
(In reply to Stefan Bodewig from comment #5)
> IIUC we could modify ScriptRunnerCreator so that it still uses
> ClassLoader.getResource if the selected engine is BSF but uses Class.forName
> for the javax.script engine, right?
> 
> This should address the changes Java9 and allow us to fiddle with
> classloaders before loading BSF at the same time.

Yes. That should work.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

--- Comment #2 from a.sundararajan@gmail.com ---
(In reply to Stefan Bodewig from comment #1)
> Are you experiencing any problem with the current approach?

Hi, I am working on jdk9. The behaviour of ClassLoader.getResource is expected
to change in the modular ("jigsaw") jdk9
(http://openjdk.java.net/projects/jigsaw/). Retrieving .class resource URL from
a named module will not work as expected.

Given that there is cleaner alternative - namely reflection API, it is better
to use java reflection to check javax.script.ScriptEngineManager exists or not
in the JDK configured. Is there any particular reason why this
ClassLoader.getResource is preferred? If so, please let me know.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

Stefan Bodewig <bo...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |NEW

--- Comment #3 from Stefan Bodewig <bo...@apache.org> ---
TBH I don't remember why the code was written the way it is and haven't been
involved with writing it at all - just wanted to avoid fixing code for cosmetic
reasons that wasn't broken.

Looking at the code my guess is the special case for BSF that immediately
before the Class.forName call is required for it to work when using BSF and the
code needs a way to ensure BSF is actually there before modifying the
classloader hierarchy.

The problem we should really talk about is LoaderUtils.classNameToResource
which is also used inside the JUnit task.  We use it to tell people we've found
multiple versions of JUnit.  Not only that, we also tell them where we've found
them.  How would we achieve the same with using reflection alone?

The LoaderUtils.classExists that uses LoaderUtils.classNameToResource is only
used inside the BSF classloader hack so shouldn't be a problem if we manage to
fix the hack.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

--- Comment #8 from Björn Kautler <Bj...@Kautler.net> ---
Actually this was not a bug in Ant but in Java.

At least I would interpret it in that way that this error happens with Java 9
build 114, but works fine with the latest released build 156 which is the case.

So if you liked the old code better you now have the choice to change it back.
Excpet of course if the Java bug is, that it works again in the latest build.
:-)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

Stefan Bodewig <bo...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO
                 OS|                            |All

--- Comment #1 from Stefan Bodewig <bo...@apache.org> ---
Are you experiencing any problem with the current approach?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

--- Comment #9 from Stefan Bodewig <bo...@apache.org> ---
Thanks Björn

back then it seemed as if the change would be required for Java9 as the way
Java wrked was about to change. Now it looks the Java0 team has changed their
minds again. I'm not convinced build 156 is the final call :-)

Apart from that it probably is good the old code works again as it is still
used for JUnit. Aftre re-reading this thread it would have failed for JUnit as
well under 9b114 if junit-jar was a named module. Well, in theory the junit
devs could decide to ship junit as a Java0 module in the future.

For Ant 1.10.x I'd be willing to rewrite the whole access of javax.script to
not use reflection at all as we are guaranteed to find it - Ant 1.10 requires
Java8 at runtime.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

Stefan Bodewig <bo...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
   Target Milestone|---                         |1.9.7

--- Comment #7 from Stefan Bodewig <bo...@apache.org> ---
should be fixed with git commit 4ccca1f

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

--- Comment #5 from Stefan Bodewig <bo...@apache.org> ---
IIUC we could modify ScriptRunnerCreator so that it still uses
ClassLoader.getResource if the selected engine is BSF but uses Class.forName
for the javax.script engine, right?

This should address the changes Java9 and allow us to fiddle with classloaders
before loading BSF at the same time.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58271] script task implementation should use reflection API to find if javax.script.ScriptEngineManager is available

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58271

Dominik Stadler <do...@gmx.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dominik.stadler@gmx.at

-- 
You are receiving this mail because:
You are the assignee for the bug.