WIP: 8154714: JShell API: addExports support [for Java 9.X]
ShinyaYoshida
bitterfoxc at gmail.com
Wed Sep 28 07:37:12 UTC 2016
Thank you!
I've pushed:
http://hg.openjdk.java.net/jdk9/dev/langtools/rev/721c5727816d
2016-09-28 11:01 GMT+09:00 Robert Field <robert.field at oracle.com>:
>
> On 09/27/16 18:02, ShinyaYoshida wrote:
>
> Robert,
> Thank you!!
>
> I'll push but I have a question.
> Who should I put to reviewed-by?
>
>
> Reviewed-by: jlahoda, rfield
>
> Robert and Jan are enough?
>
>
> Yep
>
> Don't I need to put the internal reviewers?
>
>
> Nope
>
> Thanks,
> Robert
>
>
> Thanks,
> shinyafox(Shinya Yoshida)
>
>
> 2016-09-28 1:00 GMT+09:00 Robert Field <robert.field at oracle.com>:
>
>> [audience reduced]
>>
>> shinyafox,
>>
>> This passed internal review.
>>
>> You are good to push!
>>
>> Thanks!
>> Robert
>>
>> On 09/26/16 13:20, ShinyaYoshida wrote:
>>
>> Hi Robert,
>> Thank you for asking for the internal review.
>>
>> I've updated and replaced the previous webrev!
>> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.11.langtools/
>>
>> Regards,
>> shinyafox(Shinya Yoshida)
>>
>> 2016-09-26 10:10 GMT-07:00 Robert Field <robert.field at oracle.com>:
>>
>>> That's great!
>>>
>>> I've submitted this to internal review of the new command-line options.
>>> We should get that in less than a week.
>>>
>>> One nit, I've noticed that --help doesn't list the new -X option.
>>> Borrowing from "java -help" this should be:
>>>
>>> -X Print help on non-standard options
>>>
>>>
>>> Thanks!
>>>
>>> -Robert
>>>
>>>
>>>
>>> On 09/23/16 23:49, ShinyaYoshida wrote:
>>>
>>> Hi Robert,
>>>
>>>
>>> 2016-09-23 0:06 GMT-07:00 Robert Field <robert.field at oracle.com>:
>>>
>>>>
>>>> On 09/22/16 12:51, ShinyaYoshida wrote:
>>>>
>>>> Hi Robert and Jan,
>>>> I've updated the webrev to add -C option and --add-exports option(as a
>>>> non-standard option):
>>>> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.1
>>>> 0.langtools/
>>>>
>>>> Could you review and give me a comment?
>>>>
>>>>
>>>> Wow! This is so much simpler an approach!
>>>>
>>>> A couple little things and this is good...
>>>>
>>>> In JShellTool, should --add-exports have .withRequiredArg() rather than
>>>> .withOptionalArg()?
>>>>
>>> Yeah! That's what I want to use!!
>>> I've fixed
>>>
>>>
>>>>
>>>> I think
>>>>
>>>> 645 if (options.has("X")) {
>>>> 646 printUsageX();
>>>> 647 }
>>>>
>>>> Should be up below "help", and, like "help", should abort by returning
>>>> null.
>>>>
>>> Absolutely yes!!
>>>
>>> New version is here:
>>> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.11.langtools/
>>>
>>> Regards,
>>> shinyafox(ShinyaYoshida)
>>>
>>>
>>>>
>>>> Your comment looks fine.
>>>>
>>>> Test and resource file look good.
>>>>
>>>> Thanks,
>>>> Robert
>>>>
>>>>
>>>>
>>>>
>>>> Another issue is found until implementing:
>>>> https://bugs.openjdk.java.net/browse/JDK-8166581
>>>>
>>>> Could you confirm and address this?
>>>>
>>>> Regards,
>>>> shinyafox(Shinya Yoshida)@JavaOne 20016
>>>>
>>>> 2016-09-13 9:26 GMT-07:00 Robert Field <robert.field at oracle.com>:
>>>>
>>>>>
>>>>> On 09/12/16 17:55, ShinyaYoshida wrote:
>>>>>
>>>>> Hi Jan and all,
>>>>> I've confirmed your snippet.
>>>>> Certainly it have a security problem and it should be avoided.
>>>>>
>>>>> I'll update my patch to add a new command-line option -C which give
>>>>> the option to compiler.
>>>>>
>>>>> In addition to, I'd like to add a new command-line option
>>>>> --add-exports <module>/<package> which is a short-circuit for
>>>>> "-J--add-exports <module>/<package>=ALL-UNNAMED -C--add-exports
>>>>> <module>/<package>=ALL-UNNAMED."
>>>>> java and javac are also have --add-exports option, so it seems to me
>>>>> that it is reasonable for jshell to have --add-exports option.
>>>>> What do you think about this?
>>>>>
>>>>> Kind Regards,
>>>>> shinyafox(Shinya Yoshida)
>>>>>
>>>>>
>>>>> OK, great.
>>>>>
>>>>> I started to write that I didn't think you needed or wanted a -C<opt>
>>>>> flag. But on further thought, that allows all kinds of flexibility that we
>>>>> may not want to provide item by item -- for example:
>>>>> --processor-module-path, --module-source-path, ....
>>>>> There is a question: which compiles should these flags get applied to,
>>>>> but it seems they would just be ignored where not needed, so probably all
>>>>> of them.
>>>>>
>>>>> I notice --add-exports is on the -X help for both java and javac -- it
>>>>> is non-standard. Seems wrong for it to be a non-standard option for those
>>>>> two and a standard option for jshell.
>>>>>
>>>>> I think it is a question for Jigsaw people as to what module options:
>>>>>
>>>>> * should be directly jshell command-line options
>>>>> * should be non-standard -X command-line options (if jshell has -X
>>>>> options)
>>>>> * obscure enough that users should use -R<opt> and -C<opt>
>>>>>
>>>>> -Robert
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 2016-09-13 9:38 GMT+09:00 Robert Field <robert.field at oracle.com>:
>>>>>
>>>>>> If our approach to security is "do nothing a user could not do with
>>>>>> their own use of javac/java" (which we have yet to have validated) then if
>>>>>> the SharedSecrets use in JShell requires the change to the module.info.java
>>>>>> in java.base to add jdk.jshell, then that would be a violation.
>>>>>>
>>>>>> If this changeset gave access only to snippets executing via the
>>>>>> JShell API maybe that would be OK. But it, as your exploit verifies, could
>>>>>> easily be used by anyone to change module boundaries.
>>>>>>
>>>>>> Maybe there would be a hack that would allow only proper JShell use,
>>>>>> but I don't see what that would be, and that sounds dangerous and
>>>>>> unappealing.
>>>>>>
>>>>>> Command-line option only would match the original request and other
>>>>>> tools. The implementation would closely match the implementation of '-R',
>>>>>> being on the command-line of JShellTool and the API in JShell.Builder.
>>>>>> Alas, this is mostly a rewrite, only some code in TaskFactory,
>>>>>> ToolBasicTest, and maybe KullaTesting would be reusable.
>>>>>>
>>>>>> Bummer!
>>>>>>
>>>>>> But thanks for catching this Jan!
>>>>>>
>>>>>> -Robert
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 09/12/16 14:12, Jan Lahoda wrote:
>>>>>>
>>>>>>> (Basically, the use of SharedSecrets in jdk.jshell seems highly
>>>>>>> suspicious to me.)
>>>>>>>
>>>>>>> Jan
>>>>>>>
>>>>>>> On 12.9.2016 23:05, Jan Lahoda wrote:
>>>>>>>
>>>>>>>> On 12.9.2016 20:06, Robert Field wrote:
>>>>>>>>
>>>>>>>>> The jshell tool is built on the JShell API. Command-line options,
>>>>>>>>> like
>>>>>>>>> -R, pass to the remote process via the JShell API. -R would allow
>>>>>>>>> --patch-module or --add-exports. Having addExports functionality
>>>>>>>>>
>>>>>>>>
>>>>>>>> I am not much worried about --add-exports added using -R, that
>>>>>>>> seems OK
>>>>>>>> to me. This only affects a newly spawned JVM, and the API client
>>>>>>>> could
>>>>>>>> spawn it even without JShell, so not a problem IMO.
>>>>>>>>
>>>>>>>> What I am worried about is that using the most recent proposed patch
>>>>>>>> (lwebrev.04.langtools), one can use the DirectExecutionControl to
>>>>>>>> change
>>>>>>>> module boundaries in the _currently running VM_. Please see the
>>>>>>>> attached
>>>>>>>> class to see what I mean - I didn't even have to create JShell as
>>>>>>>> such,
>>>>>>>> simply using DirectExecutionControl was enough. It probably does not
>>>>>>>> even need some special privileges to achieve the result.
>>>>>>>>
>>>>>>>> Jan
>>>>>>>>
>>>>>>>> anywhere is jshell means having it in the JShell API.
>>>>>>>>>
>>>>>>>>> I believe this cycles us, to some degree, back to the security
>>>>>>>>> discussion. What is our level of concern about a launched process
>>>>>>>>> running pure JDK code?
>>>>>>>>>
>>>>>>>>> -Robert
>>>>>>>>>
>>>>>>>>> On 09/12/16 09:57, Jan Lahoda wrote:
>>>>>>>>>
>>>>>>>>>> [adding Jon]
>>>>>>>>>>
>>>>>>>>>> On 12.9.2016 17:31, Robert Field wrote:
>>>>>>>>>>
>>>>>>>>>>> Both javac and java have a --add-exports option, so it isn't
>>>>>>>>>>> opening
>>>>>>>>>>> anything that isn't already open.
>>>>>>>>>>> I don't see how command-line options are any different that API
>>>>>>>>>>> in
>>>>>>>>>>> terms
>>>>>>>>>>> of exposure.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I think there's a huge difference between command line and an
>>>>>>>>>> API, in
>>>>>>>>>> at least two aspects:
>>>>>>>>>> a) the person that controls the command line, has a lot of power
>>>>>>>>>> in
>>>>>>>>>> any case (they could do -Xbootclasspath/p: in 8 or --patch-module
>>>>>>>>>> now
>>>>>>>>>> to get around the boundaries). API users are much more restricted
>>>>>>>>>> in
>>>>>>>>>> what they can do.
>>>>>>>>>> b) affecting the command line options is generally very hard -
>>>>>>>>>> imagine
>>>>>>>>>> library maintainers - how can they affect the command line
>>>>>>>>>> setting?
>>>>>>>>>> The only thing they can do is to write some documentation, but
>>>>>>>>>> they
>>>>>>>>>> cannot directly affect the command line. But they can call an API
>>>>>>>>>> (and
>>>>>>>>>> they can call it even reflectively).
>>>>>>>>>>
>>>>>>>>>> I believe it is generally undesirable to create APIs that allow to
>>>>>>>>>> create holes in module boundaries. If you think that's something
>>>>>>>>>> JShell needs, I think we need to take that to the Jigsaw team.
>>>>>>>>>>
>>>>>>>>>> Jan
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> -Robert
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 09/11/16 13:17, Jan Lahoda wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I apologize for not following this sooner.
>>>>>>>>>>>>
>>>>>>>>>>>> Looking at this, it seems to me the change would effectively
>>>>>>>>>>>> allow to
>>>>>>>>>>>> overcome the module boundary limitations using JShell - I don't
>>>>>>>>>>>> think
>>>>>>>>>>>> that's something that would be acceptable. We could take this
>>>>>>>>>>>> to the
>>>>>>>>>>>> Jigsaw team, but I don't think they would be happy about this.
>>>>>>>>>>>>
>>>>>>>>>>>> I wonder what exactly is the usecase here. Would it be
>>>>>>>>>>>> sufficient if
>>>>>>>>>>>> the jshell tool as such would allow command line options to
>>>>>>>>>>>> break
>>>>>>>>>>>> through the boundaries? We already have -R, which should allow
>>>>>>>>>>>> to make
>>>>>>>>>>>> holes to the boundaries when running the snippets, so maybe we
>>>>>>>>>>>> just
>>>>>>>>>>>> need to add something for compile-time?
>>>>>>>>>>>>
>>>>>>>>>>>> Jan
>>>>>>>>>>>>
>>>>>>>>>>>> On 11.9.2016 02:12, ShinyaYoshida wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>> Please apply this webrev:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.0
>>>>>>>>>>>>> 0.jdk/
>>>>>>>>>>>>> <http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.
>>>>>>>>>>>>> 00.jdk/>
>>>>>>>>>>>>> Changing module-info is needed.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2016-09-10 7:51 GMT+09:00 Robert Field <
>>>>>>>>>>>>> robert.field at oracle.com
>>>>>>>>>>>>> <mailto:robert.field at oracle.com>>:
>>>>>>>>>>>>>
>>>>>>>>>>>>> [audience reduced]
>>>>>>>>>>>>>
>>>>>>>>>>>>> BTW shinyafox,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Building "make images" with this patch and the latest
>>>>>>>>>>>>> jdk9/dev
>>>>>>>>>>>>> bits
>>>>>>>>>>>>> fails. Probably need to change module set-up --
>>>>>>>>>>>>>
>>>>>>>>>>>>> /w/y/dev/langtools/src/jdk.jshell/share/classes/jdk/jshell/e
>>>>>>>>>>>>> xecution/DirectExecutionControl.java:33:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> error: package jdk.internal.misc does not exist
>>>>>>>>>>>>> import jdk.internal.misc.JavaLangReflectModuleAccess;
>>>>>>>>>>>>> ^
>>>>>>>>>>>>> /w/y/dev/langtools/src/jdk.jshell/share/classes/jdk/jshell/e
>>>>>>>>>>>>> xecution/DirectExecutionControl.java:34:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> error: package jdk.internal.misc does not exist
>>>>>>>>>>>>> import jdk.internal.misc.SharedSecrets;
>>>>>>>>>>>>> ^
>>>>>>>>>>>>> /w/y/dev/langtools/src/jdk.jshell/share/classes/jdk/jshell/e
>>>>>>>>>>>>> xecution/DirectExecutionControl.java:155:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> error: cannot find symbol
>>>>>>>>>>>>> JavaLangReflectModuleAccess moduleAccess =
>>>>>>>>>>>>> SharedSecrets.getJavaLangReflectModuleAccess();
>>>>>>>>>>>>> ^
>>>>>>>>>>>>> symbol: class JavaLangReflectModuleAccess
>>>>>>>>>>>>> location: class DirectExecutionControl
>>>>>>>>>>>>> /w/y/dev/langtools/src/jdk.jshell/share/classes/jdk/jshell/e
>>>>>>>>>>>>> xecution/DirectExecutionControl.java:155:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> error: cannot find symbol
>>>>>>>>>>>>> JavaLangReflectModuleAccess moduleAccess =
>>>>>>>>>>>>> SharedSecrets.getJavaLangReflectModuleAccess();
>>>>>>>>>>>>> ^
>>>>>>>>>>>>> symbol: variable SharedSecrets
>>>>>>>>>>>>> location: class DirectExecutionControl
>>>>>>>>>>>>> 4 errors
>>>>>>>>>>>>>
>>>>>>>>>>>>> -Robert
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 09/08/16 07:14, ShinyaYoshida wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>> Thank you for your pointing out.
>>>>>>>>>>>>>> I've understood.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Here is new webrev:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.0
>>>>>>>>>>>>>> 4.langtools/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8154714/webre
>>>>>>>>>>>>>> v.04.langtools/>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In this version,
>>>>>>>>>>>>>> - ExecutionControl#addExports throws BadArgumentException
>>>>>>>>>>>>>> which is
>>>>>>>>>>>>>> extends ExecutionControlException if module or package
>>>>>>>>>>>>>> are not
>>>>>>>>>>>>>> found.
>>>>>>>>>>>>>> - JShell#addExports throws IllegalArgumentException when
>>>>>>>>>>>>>> BadArgumentException is occured
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I've notice that /classpath and addToClasspath says
>>>>>>>>>>>>>> nothing when
>>>>>>>>>>>>>> it doesn't exist.
>>>>>>>>>>>>>> I think it should be reported, so I've created a issue:
>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8165650
>>>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8165650>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> What do you think?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please review me again!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2016-09-08 4:25 GMT+09:00 Robert Field <
>>>>>>>>>>>>>> robert.field at oracle.com
>>>>>>>>>>>>>> <mailto:robert.field at oracle.com>>:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks for the update.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> l10n.properties --
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Oops, typo: the help.set.exports entry got pasted
>>>>>>>>>>>>>> again
>>>>>>>>>>>>>> instead of help.retain.exports
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Sorry I was unclear about the exceptions...
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On the execution side --
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ExecutionControl.InternalException is only for
>>>>>>>>>>>>>> unexpected
>>>>>>>>>>>>>> failures.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In this case you are dealing an expected exception
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> occurs
>>>>>>>>>>>>>> when the user gives the wrong input. None of the
>>>>>>>>>>>>>> existing
>>>>>>>>>>>>>> ExecutionControlException subclasses are good
>>>>>>>>>>>>>> candidates,
>>>>>>>>>>>>>> so a
>>>>>>>>>>>>>> new ExecutionControlException subclass is needed. I
>>>>>>>>>>>>>> was
>>>>>>>>>>>>>> suggesting "BadArgumentException" for this exception.
>>>>>>>>>>>>>> None of
>>>>>>>>>>>>>> ExecutionControlException subclasses are seen at the
>>>>>>>>>>>>>> JShell
>>>>>>>>>>>>>> level -- this should be the case for this new one as
>>>>>>>>>>>>>> well.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I don't what kind of exceptions are thrown by
>>>>>>>>>>>>>> moduleAccess.addExportsToAllUnnamed(target, pack),
>>>>>>>>>>>>>> but that
>>>>>>>>>>>>>> message text or some other appropriate descriptive
>>>>>>>>>>>>>> message
>>>>>>>>>>>>>> text needs to be placed in the BadArgumentException
>>>>>>>>>>>>>> message so
>>>>>>>>>>>>>> it can be seen all the way up (including the
>>>>>>>>>>>>>> JShellTool).
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On the JShell class --
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Incorrect arguments on JShell API methods generate
>>>>>>>>>>>>>> the Java
>>>>>>>>>>>>>> standard java.lang.IllegalArgumentException (see, for
>>>>>>>>>>>>>> example,
>>>>>>>>>>>>>> drop() or status() ) -- so, this is also the
>>>>>>>>>>>>>> appropriate
>>>>>>>>>>>>>> exception for JShell.addExports(). Thus,
>>>>>>>>>>>>>> JShell.addExports()
>>>>>>>>>>>>>> catchs the BadArgumentException, turning it into an
>>>>>>>>>>>>>> IllegalArgumentException.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Robert
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 09/07/16 06:10, ShinyaYoshida wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>> Thank you for your helping.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I've update webrev according to your comments.
>>>>>>>>>>>>>>> If I'm missing your meaning, I'm sorry. Please point
>>>>>>>>>>>>>>> it.
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~shinyafox/kulla/8154714/webrev.0
>>>>>>>>>>>>>>> 3.langtools/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8154714/webre
>>>>>>>>>>>>>>> v.03.langtools/>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Now,
>>>>>>>>>>>>>>> - JShell#addExports will throw BadArgumentException
>>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>> module or package is wrong, which is check exception.
>>>>>>>>>>>>>>> - JShellTool checks that exception.
>>>>>>>>>>>>>>> - ExecutionControl#addExports throws
>>>>>>>>>>>>>>> InternalException when
>>>>>>>>>>>>>>> something is wrong.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 2016-09-06 6:28 GMT+09:00 Robert Field
>>>>>>>>>>>>>>> <robert.field at oracle.com <mailto:
>>>>>>>>>>>>>>> robert.field at oracle.com>>:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Code looks good. Needs docs I promised (see
>>>>>>>>>>>>>>> below).
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> As to completion issue you created, I see you
>>>>>>>>>>>>>>> already
>>>>>>>>>>>>>>> added discussion and tests. I added your
>>>>>>>>>>>>>>> completion
>>>>>>>>>>>>>>> code.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Below is my suggested doc...
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> /set exports <module> <package>\n\t\
>>>>>>>>>>>>>>> Allow access to some of the unexported
>>>>>>>>>>>>>>> types of a
>>>>>>>>>>>>>>> module\n\n\
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> /retain exports <module> <package>\n\t\
>>>>>>>>>>>>>>> Allow access to some of the unexported
>>>>>>>>>>>>>>> types of a
>>>>>>>>>>>>>>> module\n\n\
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> help.set.exports=\
>>>>>>>>>>>>>>> Set a package to add to those exported to
>>>>>>>>>>>>>>> snippets.\n\
>>>>>>>>>>>>>>> \n\t\
>>>>>>>>>>>>>>> /set exports <module> <package>\n\
>>>>>>>>>>>>>>> \n\
>>>>>>>>>>>>>>> Where <module> is the module from which to export
>>>>>>>>>>>>>>> <package>.\n\
>>>>>>>>>>>>>>> Where <package> is the package to export to
>>>>>>>>>>>>>>> snippets.\n
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> help.retain.exports=\
>>>>>>>>>>>>>>> Retain a package to add to those exported to
>>>>>>>>>>>>>>> snippets.\n\
>>>>>>>>>>>>>>> \n\t\
>>>>>>>>>>>>>>> /retain exports <module> <package>\n\
>>>>>>>>>>>>>>> \n\
>>>>>>>>>>>>>>> Where <module> is the module from which to export
>>>>>>>>>>>>>>> <package>.\n\
>>>>>>>>>>>>>>> Where <package> is the package to export to
>>>>>>>>>>>>>>> snippets.\n
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> [I notice that 'snippet' is used in doc without
>>>>>>>>>>>>>>> being
>>>>>>>>>>>>>>> defined. If, while you are it, you change the
>>>>>>>>>>>>>>> help for
>>>>>>>>>>>>>>> /list to define it:]
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> help.list =\
>>>>>>>>>>>>>>> Show the source of snippets, prefaced with the
>>>>>>>>>>>>>>> snippet id
>>>>>>>>>>>>>>> (a snippet is a chunk of code that the jshell
>>>>>>>>>>>>>>> tool
>>>>>>>>>>>>>>> evaluates -- the code you enter at the prompt is
>>>>>>>>>>>>>>> a
>>>>>>>>>>>>>>> snippet).\n\
>>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> src/jdk.jshell/share/classes/jdk/jshell/JShell.java --
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> /**
>>>>>>>>>>>>>>> * Add a package to those exported to
>>>>>>>>>>>>>>> snippets.
>>>>>>>>>>>>>>> * @param module The module from which to
>>>>>>>>>>>>>>> export
>>>>>>>>>>>>>>> * @param pack The package to export
>>>>>>>>>>>>>>> * @throws NullPointerException if arguments
>>>>>>>>>>>>>>> are
>>>>>>>>>>>>>>> null.
>>>>>>>>>>>>>>> * @throws IllegalStateException if this
>>>>>>>>>>>>>>> {@code
>>>>>>>>>>>>>>> JShell} instance is closed.
>>>>>>>>>>>>>>> * @throws XYZ if ....
>>>>>>>>>>>>>>> */
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> OK, actually this code should change. Like
>>>>>>>>>>>>>>> addToClasspath above it, it should return void.
>>>>>>>>>>>>>>> It
>>>>>>>>>>>>>>> looks
>>>>>>>>>>>>>>> like under normal operation it can fail -- the
>>>>>>>>>>>>>>> failure
>>>>>>>>>>>>>>> should be reported with an exception.
>>>>>>>>>>>>>>> What are the failure modes?
>>>>>>>>>>>>>>> IllegalArgumentException
>>>>>>>>>>>>>>> would be likely choice at the JShell API layer.
>>>>>>>>>>>>>>> But
>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>> the exception needs to be propagated there.
>>>>>>>>>>>>>>> Since none of the existing exceptions is a
>>>>>>>>>>>>>>> match, this
>>>>>>>>>>>>>>> means a new ExecutionControlException. Maybe
>>>>>>>>>>>>>>> something
>>>>>>>>>>>>>>> general, like BadUserArgumentException.
>>>>>>>>>>>>>>> Note: I've also added IllegalStateException --
>>>>>>>>>>>>>>> this can
>>>>>>>>>>>>>>> be thrown when you get
>>>>>>>>>>>>>>> EngineTerminationException.
>>>>>>>>>>>>>>> Also, typo, the second "module cannot be null"
>>>>>>>>>>>>>>> should be
>>>>>>>>>>>>>>> "package cannot be null"
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> src/jdk.jshell/share/classes/jdk/jshell/execution/RemoteCodes.java
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The comment you have is fine. Just remove PLEASE
>>>>>>>>>>>>>>> REVISE
>>>>>>>>>>>>>>> THE COMMENT
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> src/jdk.jshell/share/classes/jdk/jshell/spi/ExecutionControl
>>>>>>>>>>>>>>> .java
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> /**
>>>>>>>>>>>>>>> * Add a package to those exported to
>>>>>>>>>>>>>>> snippets.
>>>>>>>>>>>>>>> * @param module The module from which to
>>>>>>>>>>>>>>> export
>>>>>>>>>>>>>>> * @param pack The package to export
>>>>>>>>>>>>>>> * @throws EngineTerminationException the
>>>>>>>>>>>>>>> execution
>>>>>>>>>>>>>>> engine has terminated
>>>>>>>>>>>>>>> * @throws ....
>>>>>>>>>>>>>>> * @throws InternalException an internal
>>>>>>>>>>>>>>> problem
>>>>>>>>>>>>>>> occurred
>>>>>>>>>>>>>>> */
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Plus add the bad argument exception.
>>>>>>>>>>>>>>> Whatever it is, should be added to
>>>>>>>>>>>>>>> extensionCommand()
>>>>>>>>>>>>>>> exceptions.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Robert
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 09/05/16 08:02, ShinyaYoshida wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>> I've update webrev of langtools:
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~sh
>>>>>>>>>>>>>>>> inyafox/kulla/8154714/webrev.02.langtools/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.02.langtools/>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I added an issue for the completion feature.
>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8165445
>>>>>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8165445>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 2016-09-05 22:49 GMT+09:00 ShinyaYoshida
>>>>>>>>>>>>>>>> <bitterfoxc at gmail.com <mailto:
>>>>>>>>>>>>>>>> bitterfoxc at gmail.com>>:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>> Thank you for the review.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I totally agree with you.
>>>>>>>>>>>>>>>> I also think that availableModules and
>>>>>>>>>>>>>>>> availablePackages(module) are worthless
>>>>>>>>>>>>>>>> excepting
>>>>>>>>>>>>>>>> for the command completion feature.
>>>>>>>>>>>>>>>> I'd like to avoid adding these APIs which
>>>>>>>>>>>>>>>> have not
>>>>>>>>>>>>>>>> been speculated and revised because if API
>>>>>>>>>>>>>>>> is once
>>>>>>>>>>>>>>>> added, it is too difficult to remove than
>>>>>>>>>>>>>>>> adding.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> So currently, I'll remove the completion
>>>>>>>>>>>>>>>> feature
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> /set or /retain exports and make the issue
>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>> JBS to
>>>>>>>>>>>>>>>> add completion feature for them in future.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I'll update the webrev. So please review it
>>>>>>>>>>>>>>>> again
>>>>>>>>>>>>>>>> later.
>>>>>>>>>>>>>>>> When the silently evaluation feature is
>>>>>>>>>>>>>>>> added, I'd
>>>>>>>>>>>>>>>> like to work for the completion feature
>>>>>>>>>>>>>>>> again.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 2016-09-05 15:00 GMT+09:00 Robert Field
>>>>>>>>>>>>>>>> <robert.field at oracle.com
>>>>>>>>>>>>>>>> <mailto:robert.field at oracle.com>>:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 09/03/16 05:07, ShinyaYoshida wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>> I've updated my patch, new webrev is
>>>>>>>>>>>>>>>>> here:
>>>>>>>>>>>>>>>>> webrev.01.langtools:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~sh
>>>>>>>>>>>>>>>>> inyafox/kulla/8154714/webrev.01.langtools/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.01.langtools/>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> webrev.01.jdk:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~sh
>>>>>>>>>>>>>>>>> inyafox/kulla/8154714/webrev.01.jdk/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.01.jdk/>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> (Same as previous)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> In this version,
>>>>>>>>>>>>>>>>> - rename /set add-exports to /set
>>>>>>>>>>>>>>>>> exports
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> OK
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> - add /retain exports <module> <package>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> The /retain implementation looks good.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> - add tests
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Well tested
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> - move EXT_CMD_XXX into RemoteCodes
>>>>>>>>>>>>>>>>> - make RemoteCodes public and move it
>>>>>>>>>>>>>>>>> into
>>>>>>>>>>>>>>>>> jdk.jshell.execution.code which is
>>>>>>>>>>>>>>>>> module-private
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> So, here, in addition to addExports,
>>>>>>>>>>>>>>>> there are
>>>>>>>>>>>>>>>> two more methods added to the API
>>>>>>>>>>>>>>>> availableModules() and
>>>>>>>>>>>>>>>> availablePackage(String
>>>>>>>>>>>>>>>> module).
>>>>>>>>>>>>>>>> I see how in rare cases they could be
>>>>>>>>>>>>>>>> useful.
>>>>>>>>>>>>>>>> And the rare case that causes them to be
>>>>>>>>>>>>>>>> present
>>>>>>>>>>>>>>>> is for command completion in the tool.
>>>>>>>>>>>>>>>> What are the options:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> (1) Add them to the API. If it is in
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> API
>>>>>>>>>>>>>>>> then it is a first-class form of
>>>>>>>>>>>>>>>> functionality
>>>>>>>>>>>>>>>> and should be treated as such in
>>>>>>>>>>>>>>>> execution
>>>>>>>>>>>>>>>> support (as a command)
>>>>>>>>>>>>>>>> (2) Not support them and not support
>>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>>> completion for /set exports. Command
>>>>>>>>>>>>>>>> completion
>>>>>>>>>>>>>>>> is not supported in much more common
>>>>>>>>>>>>>>>> cases.
>>>>>>>>>>>>>>>> Then we could support command
>>>>>>>>>>>>>>>> completion in
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> future (see (3)).
>>>>>>>>>>>>>>>> (3) What these commands do is simply
>>>>>>>>>>>>>>>> execute
>>>>>>>>>>>>>>>> an expression on the remote side. But we
>>>>>>>>>>>>>>>> already have a mechanism to execute
>>>>>>>>>>>>>>>> expressions
>>>>>>>>>>>>>>>> on the remote side: Snippets.
>>>>>>>>>>>>>>>> That leaves a problem: we
>>>>>>>>>>>>>>>> wouldn't
>>>>>>>>>>>>>>>> want
>>>>>>>>>>>>>>>> these Snippets to be seen by the user,
>>>>>>>>>>>>>>>> but we
>>>>>>>>>>>>>>>> have a solution for that the tool uses
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> idGenerator.
>>>>>>>>>>>>>>>> Another problem is that, by
>>>>>>>>>>>>>>>> default
>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>> with no current way around the default,
>>>>>>>>>>>>>>>> expressions generate temp-variables. My
>>>>>>>>>>>>>>>> intention was to make this configurable
>>>>>>>>>>>>>>>> -- see
>>>>>>>>>>>>>>>> Eval.shouldGenTempVar(...).
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> One goal I've been using in the design
>>>>>>>>>>>>>>>> of the
>>>>>>>>>>>>>>>> API is to keep it as small and simple as
>>>>>>>>>>>>>>>> possible. This improves
>>>>>>>>>>>>>>>> maintainability, but
>>>>>>>>>>>>>>>> much more importantly makes the API
>>>>>>>>>>>>>>>> easier to
>>>>>>>>>>>>>>>> learn and use.
>>>>>>>>>>>>>>>> So, for something to warrant inclusion
>>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>> should
>>>>>>>>>>>>>>>> be generally useful and have no other
>>>>>>>>>>>>>>>> way of
>>>>>>>>>>>>>>>> being achieved -- neither is true of
>>>>>>>>>>>>>>>> these two
>>>>>>>>>>>>>>>> queries.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> My suggestion, particularly in light of
>>>>>>>>>>>>>>>> how
>>>>>>>>>>>>>>>> late
>>>>>>>>>>>>>>>> in the release this is, would be (2).
>>>>>>>>>>>>>>>> Thoughts?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> -Robert
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 2016-09-03 18:15 GMT+09:00
>>>>>>>>>>>>>>>>> ShinyaYoshida
>>>>>>>>>>>>>>>>> <bitterfoxc at gmail.com
>>>>>>>>>>>>>>>>> <mailto:bitterfoxc at gmail.com>>:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>>> Thank you for reviewing.
>>>>>>>>>>>>>>>>> I'll update my patch later.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 2016-09-02 11:38 GMT+09:00 Robert
>>>>>>>>>>>>>>>>> Field
>>>>>>>>>>>>>>>>> <robert.field at oracle.com
>>>>>>>>>>>>>>>>> <mailto:robert.field at oracle.com>>:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks shiyafox! This is a
>>>>>>>>>>>>>>>>> significant
>>>>>>>>>>>>>>>>> piece of code!
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> As to your questions --
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> > I wonder if you could work
>>>>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>>>>> documentation of the API and
>>>>>>>>>>>>>>>>> the tool
>>>>>>>>>>>>>>>>> instead?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Yes, I will do API and tool
>>>>>>>>>>>>>>>>> docs.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thank you!
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> > Subcommand of /set is
>>>>>>>>>>>>>>>>> reasonable?
>>>>>>>>>>>>>>>>> Similar command, /classpath,
>>>>>>>>>>>>>>>>> to make
>>>>>>>>>>>>>>>>> users available to use new
>>>>>>>>>>>>>>>>> packages is
>>>>>>>>>>>>>>>>> not a subcommand of /set.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Yes, I think /set exports is
>>>>>>>>>>>>>>>>> best.
>>>>>>>>>>>>>>>>> And, yes, /classpath is
>>>>>>>>>>>>>>>>> inconsistent,
>>>>>>>>>>>>>>>>> it should be /set classpath,
>>>>>>>>>>>>>>>>> that can
>>>>>>>>>>>>>>>>> be a separate task, unless you
>>>>>>>>>>>>>>>>> want to
>>>>>>>>>>>>>>>>> roll that in.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I see, I've created the issue to
>>>>>>>>>>>>>>>>> move
>>>>>>>>>>>>>>>>> /classpath to /set classpath:
>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8165405
>>>>>>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8165405>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> > Should we need retain
>>>>>>>>>>>>>>>>> feature to
>>>>>>>>>>>>>>>>> this?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> All /set have a /retain
>>>>>>>>>>>>>>>>> counterpart --
>>>>>>>>>>>>>>>>> we should maintain that
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Ok, I'll add /retain exports.
>>>>>>>>>>>>>>>>> I've also create the issue for
>>>>>>>>>>>>>>>>> retaining
>>>>>>>>>>>>>>>>> classpath:
>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8165406
>>>>>>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8165406>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> As to the code review: Looks
>>>>>>>>>>>>>>>>> good to
>>>>>>>>>>>>>>>>> me, just minor
>>>>>>>>>>>>>>>>> questions/issues --
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> JShellTool.java:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * As discussed /set
>>>>>>>>>>>>>>>>> add-exports =>
>>>>>>>>>>>>>>>>> /set
>>>>>>>>>>>>>>>>> exports
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * Nit: a comment got moved out
>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>> place
>>>>>>>>>>>>>>>>> line 1194(old)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> JShell.java:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * EXT_CMD_AVAILABLE_MODULES and
>>>>>>>>>>>>>>>>> EXT_CMD_AVAILABLE_PACKAGES, these
>>>>>>>>>>>>>>>>> should be referenced via
>>>>>>>>>>>>>>>>> RemoteCodes.java rather than be
>>>>>>>>>>>>>>>>> coped.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Absolutely! I'd like to move these
>>>>>>>>>>>>>>>>> into
>>>>>>>>>>>>>>>>> RemoteCodes but it is
>>>>>>>>>>>>>>>>> package-private
>>>>>>>>>>>>>>>>> class
>>>>>>>>>>>>>>>>> and cannot be referred from JShell.
>>>>>>>>>>>>>>>>> Can I make RemoteCodes public and
>>>>>>>>>>>>>>>>> move it
>>>>>>>>>>>>>>>>> to another module-private package
>>>>>>>>>>>>>>>>> like
>>>>>>>>>>>>>>>>> jdk.jshell.execution.code?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * Which leads me to another
>>>>>>>>>>>>>>>>> question:
>>>>>>>>>>>>>>>>> why is addExports a command
>>>>>>>>>>>>>>>>> and these
>>>>>>>>>>>>>>>>> two are extensions. It seems
>>>>>>>>>>>>>>>>> fine,
>>>>>>>>>>>>>>>>> just
>>>>>>>>>>>>>>>>> curious. I notice that
>>>>>>>>>>>>>>>>> extensions
>>>>>>>>>>>>>>>>> are a
>>>>>>>>>>>>>>>>> lot less code -- ;-)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Exporting module and package is
>>>>>>>>>>>>>>>>> similar to
>>>>>>>>>>>>>>>>> adding classpath to platform, so I
>>>>>>>>>>>>>>>>> made
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> addExports a command like
>>>>>>>>>>>>>>>>> setClasspath.
>>>>>>>>>>>>>>>>> And it should be added as API
>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>> it is
>>>>>>>>>>>>>>>>> very basic feature.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> TaskFactory.java:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * Question: the "/" separator
>>>>>>>>>>>>>>>>> between
>>>>>>>>>>>>>>>>> module and package is that
>>>>>>>>>>>>>>>>> platform
>>>>>>>>>>>>>>>>> independent?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> It seems to me that
>>>>>>>>>>>>>>>>> "{module}/{path}"
>>>>>>>>>>>>>>>>> format for --add-exports option of
>>>>>>>>>>>>>>>>> javac is
>>>>>>>>>>>>>>>>> platform independent.
>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> DefaultLoaderDelegate.java:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> * Jan, can you look at this
>>>>>>>>>>>>>>>>> code?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> -Robert
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 09/01/16 05:36,
>>>>>>>>>>>>>>>>> ShinyaYoshida
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>> I've implemented this
>>>>>>>>>>>>>>>>> feature to
>>>>>>>>>>>>>>>>> JShell API and Tool with
>>>>>>>>>>>>>>>>> full
>>>>>>>>>>>>>>>>> completion:
>>>>>>>>>>>>>>>>> Bugs:
>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8154714
>>>>>>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8154714>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> jdk:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~sh
>>>>>>>>>>>>>>>>> inyafox/kulla/8154714/webrev.00.jdk/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.00.jdk/>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.00.jdk/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.00.jdk/>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> langtools:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~sh
>>>>>>>>>>>>>>>>> inyafox/kulla/8154714/webrev.00.langtools/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.00.langtools/>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.00.langtools/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7
>>>>>>>>>>>>>>>>> Eshinyafox/kulla/8154714/webrev.00.langtools/>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> This is missing the train
>>>>>>>>>>>>>>>>> of Java
>>>>>>>>>>>>>>>>> 9.0 but it could catch the
>>>>>>>>>>>>>>>>> train of
>>>>>>>>>>>>>>>>> Java 9.1 or later.
>>>>>>>>>>>>>>>>> I want to get your
>>>>>>>>>>>>>>>>> comments for
>>>>>>>>>>>>>>>>> this.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> jshell> /set add-exports
>>>>>>>>>>>>>>>>> jdk.jconsole
>>>>>>>>>>>>>>>>> sun.tools.jconsole
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> jshell> import
>>>>>>>>>>>>>>>>> sun.tools.jconsole.*
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> jshell>
>>>>>>>>>>>>>>>>> LocalVirtualMachine.getAllVirtualMachines().forEach((k,
>>>>>>>>>>>>>>>>> v) -> System.out.println(k
>>>>>>>>>>>>>>>>> + ":"
>>>>>>>>>>>>>>>>> + v))
>>>>>>>>>>>>>>>>> 10778:jdk.jshell/jdk.internal.jshell.tool.JShellTool
>>>>>>>>>>>>>>>>> 10826:jdk.jshell.execution.RemoteExecutionControl
>>>>>>>>>>>>>>>>> 35548
>>>>>>>>>>>>>>>>> 28447:org.netbeans.Main --cachedir
>>>>>>>>>>>>>>>>> /home/bitter_fox/.cache/netbeans/8.0.1
>>>>>>>>>>>>>>>>> --userdir
>>>>>>>>>>>>>>>>> /home/bitter_fox/.netbeans/8.0.1
>>>>>>>>>>>>>>>>> --branding nb
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Currently, there is no
>>>>>>>>>>>>>>>>> testing, so
>>>>>>>>>>>>>>>>> it will be required to add
>>>>>>>>>>>>>>>>> test.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>> shinyafox(Shinya Yoshida)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
More information about the kulla-dev
mailing list