RFR: 8169588: [AOT] jaotc --classpath option is confusing

[Rickard Bäckman]

Thanks for the review.

/R

On 01/17, Vladimir Kozlov wrote:
> On 1/13/17 6:33 AM, Rickard Bäckman wrote:
> >On 01/12, Vladimir Kozlov wrote:
> >>Very nice! Thank you, Rickard.
> >>
> >>But I am still prefer by default flags to be specified as reference
> >>examples for users.
> >
> >Not sure what you mean?
> 
> You did exactly what I wanted - you added flags to tests so users
> can look on tests as examples.
> 
> I satisfied with this latest version. Reviewed.
> 
> Thanks,
> Vladimir
> 
> >
> >>
> >>Can you don't change next test?:
> >>
> >>test/compiler/aot/cli/jaotc/CompileModuleTest.java
> >
> >Because the other way still works? But I've changed it now. Just more
> >to type.
> >
> >>
> >>and ModuleSourceProviderTest.java test should specify --module flag?
> >
> >No? There is nothing in those tests that depend on the flags?
> >
> >>
> >>And similar other tests without flags.
> >>
> >>You did not answered my question about -J-classpath -J. :
> >
> >I removed it.
> >
> >>
> >>>>>>And why you need to add -J-classpath -J. ?
> >
> >Had to do it because the launcher didn't set classpath to include . by
> >default. I added code to work around that now.
> >
> >New webrev... still uploading so have patience if it isn't there.
> >
> >http://cr.openjdk.java.net/~rbackman/8169588.4/
> >
> >/R
> >
> >>>>>
> >>>>>That is how the suggestion looked in the bug report. Use the classpath
> >>>>>of the JVM for compilation of specific classes.
> >>>>
> >>>>But I would assume '-classpath .' should be default - only if it is
> >>>>different from ./ current dir you need to specify it.
> >>
> >>Thanks,
> >>Vladimir
> >>
> >>On 1/12/17 4:43 AM, Rickard Bäckman wrote:
> >>>I kept the previous guessing functionality. However if it finds multiple
> >>>solutions it will fail with a message.
> >>>
> >>>It now also works with:
> >>>
> >>>jaotc --jar foo1.jar:foo2.jar --jar foo3.jar --directory
> >>>/tmp/something:/tmp/another --module java.base:jdk.aot --classname
> >>>java.lang.String:java.util.List
> >>>
> >>>I removed the classpath option as it was unused.
> >>>
> >>>http://cr.openjdk.java.net/~rbackman/8169588.3/
> >>>
> >>>On 01/10, Vladimir Kozlov wrote:
> >>>>On 1/10/17 4:25 AM, Rickard Bäckman wrote:
> >>>>>See inline.
> >>>>>
> >>>>>On 01/09, Vladimir Kozlov wrote:
> >>>>>>I think it is confusing.
> >>>>>>
> >>>>>>We should either have flags to distinguish them:
> >>>>>>
> >>>>>>--module java.base
> >>>>>>--class HelloWorld
> >>>>>>--jar foobar
> >>>>>
> >>>>>for --jar it would be foobar.jar though.
> >>>>>and --directory /tmp/foobar
> >>>>>
> >>>>>I'll do that instead if that is what is preferred.
> >>>>>But then new questions...
> >>>>
> >>>>okay
> >>>>
> >>>>>
> >>>>>Do we want users to be able to mix?
> >>>>>
> >>>>>jaotc --module java.base --class HelloWorld
> >>>>
> >>>>yes
> >>>>
> >>>>>
> >>>>>do we allow multiple of the same?
> >>>>>
> >>>>>jaotc --module java.base --module jdk.vm.compiler --jar
> >>>>>/home/someone/foo.jar
> >>>>
> >>>>yes
> >>>>
> >>>>>
> >>>>>do we allow splitting?
> >>>>>
> >>>>>jaotc --module java.base:jdk.vm.compiler --directory
> >>>>>/tmp/foobar:/tmp/foo:/tmp/bar
> >>>>
> >>>>Yes, would be nice to have both: multiply options (--module in
> >>>>previous case) and set of names with one option like here.
> >>>>
> >>>>>
> >>>>>>
> >>>>>>or use file type from whole name:
> >>>>>>
> >>>>>>java.base.jmod
> >>>>>>HelloWorld.class
> >>>>>>foobar.jar
> >>>>>>
> >>>>>>Two other things. I see that you left --classpath flag but use
> >>>>>>-J-classpath -J. in tests.
> >>>>>>How --classpath is used now?
> >>>>>
> >>>>>--classpath isn't used. Probably mismerge. I'll remove it.
> >>>>
> >>>>OK.
> >>>>
> >>>>>
> >>>>>>And why you need to add -J-classpath -J. ?
> >>>>>
> >>>>>That is how the suggestion looked in the bug report. Use the classpath
> >>>>>of the JVM for compilation of specific classes.
> >>>>
> >>>>But I would assume '-classpath .' should be default - only if it is
> >>>>different from ./ current dir you need to specify it.
> >>>>
> >>>>Thanks,
> >>>>Vladimir
> >>>>
> >>>>>
> >>>>>/R
> >>>>>
> >>>>>>
> >>>>>>Thanks,
> >>>>>>Vladimir
> >>>>>>
> >>>>>>On 1/9/17 4:29 AM, Rickard Bäckman wrote:
> >>>>>>>On 01/05, Vladimir Kozlov wrote:
> >>>>>>>
> >>>>>>>Vladimir,
> >>>>>>>
> >>>>>>>I removed the lines.
> >>>>>>>
> >>>>>>>We just check the different possibilities, the first thing that matches
> >>>>>>>is used. The current order is:
> >>>>>>>
> >>>>>>>module
> >>>>>>>class name
> >>>>>>>jar file
> >>>>>>>directory
> >>>>>>>
> >>>>>>>We could always change it so that it prints an error if multiple things
> >>>>>>>matches (example: java.base is a module and a directory in the users
> >>>>>>>current directory).
> >>>>>>>
> >>>>>>>http://cr.openjdk.java.net/~rbackman/8169588.2/
> >>>>>>>
> >>>>>>>/R
> >>>>>>>
> >>>>>>>>Hi Rickard,
> >>>>>>>>
> >>>>>>>>You can remove commented lines Main.java which use ClassCollector.
> >>>>>>>>
> >>>>>>>>I am not sure how you separate module name from directory name without using --module flag. You removed --module from test.
> >>>>>>>>
> >>>>>>>>Thanks,
> >>>>>>>>Vladimir
> >>>>>>>>
> >>>>>>>>On 1/5/17 6:12 AM, Rickard Bäckman wrote:
> >>>>>>>>>Noticed that I forgot to add changes to test cases in the webrev.
> >>>>>>>>>So updated.
> >>>>>>>>>
> >>>>>>>>>http://cr.openjdk.java.net/~rbackman/8169588.1/
> >>>>>>>>>
> >>>>>>>>>/R
> >>>>>>>>>
> >>>>>>>>>On 01/05, Rickard Bäckman wrote:
> >>>>>>>>>>Hi,
> >>>>>>>>>>
> >>>>>>>>>>can I please have this change reviewed?
> >>>>>>>>>>
> >>>>>>>>>>It changes how jaotc handles command line options.
> >>>>>>>>>>With this change it handles:
> >>>>>>>>>>
> >>>>>>>>>>jaotc /tmp/some/directory /tmp/some/directory2/
> >>>>>>>>>>jaotc /tmp/some/file.jar /tmp/foo/bar.jar
> >>>>>>>>>>jaotc java.base
> >>>>>>>>>>
> >>>>>>>>>>You can also use a new flag --search-path
> >>>>>>>>>>
> >>>>>>>>>>jaotc --search-path /tmp/jarfiles:/tmp/jarfiles2 file1.jar file2.jar
> >>>>>>>>>>
> >>>>>>>>>>or mix the kind of things you are trying to compile
> >>>>>>>>>>
> >>>>>>>>>>jaotc /tmp/foo/dir /tmp/bar/foobar.jar java.base
> >>>>>>>>>>
> >>>>>>>>>>https://bugs.openjdk.java.net/browse/JDK-8169588
> >>>>>>>>>>http://cr.openjdk.java.net/~rbackman/8169588/
> >>>>>>>>>>
> >>>>>>>>>>Thanks
> >>>>>>>>>>/R
> >>>/R
> >>>
> >/R
> >