RFR 8054474: Add --state-dir=bar to sjavac

[Andreas Lundblad]

On Fri, Aug 08, 2014 at 09:43:55AM +0200, Fredrik Öhrström wrote:
> See update:
> http://cr.openjdk.java.net/~ohrstrom/webrev-8054474-statedir-v2/

This looks strange:

+        String idOpt = Util.extractStringOption("id", javacService.serverSettings());
         final String id = idOpt;

Why two variables?


Perhaps the following variables could have more descriptive names.

+        String s = options.getServerConf();
+        String se = (s!=null)? s : "";
+        String i = Util.extractStringOption("id", se);


Regarding

    settings = (se.equals("")) ? "id="+id+",portfile="+portfile : se;

Do we have to bundle up the settings in a *String*? I'd prefea more typesafe datastructure.

-- Andreas


> 
> 
> 2014-08-07 3:30 GMT+02:00 Jonathan Gibbons <jonathan.gibbons at oracle.com>:
> 
> >
> > On 08/06/2014 03:48 PM, Fredrik Öhrström wrote:
> >
> >> Currently sjavac defaults to store the sjavac_state file in the
> >> destination dir along the generated classes,
> >> this can be inconvenient in some build systems, because you have to avoid
> >> the state file when creating a jar archive. It is convenient to be able to
> >> relocate javac_state to a state dir. This directory will also hold a
> >> default portfile javac_server and the log files for the server, if the
> >> server conf is not supplied.
> >>
> >> The state dir defaults to the destination dir.
> >>
> >> http://cr.openjdk.java.net/~ohrstrom/webrev-8054474-statedir/ <
> >> http://cr.openjdk.java.net/%7Eohrstrom/webrev-8054474-statedir/>
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8054474
> >>
> >> //Fredrik
> >>
> >
> > Fredrik,
> >
> > JavacState.java
> >
> >  140         javacStateFilename = stateDir.getPath()+File.
> > separator+"javac_state";
> >  141         javacState = new File(javacStateFilename);
> >
> > Try not to use "string addition to compute filenames.   In this case it is
> > better to write
> >
> >     javacState = new File(stateDIr, "javac_state");
> >
> > I know Andreas is not a "R"eviewer, but as he is also working on the
> > sjavac code,
> > I would like him to comment on these changes as well.
> >
> > -- Jon
> >