Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-30 Thread mandy chung
I reviewed the delta-webrev (v3). Looks good. Thanks for fixing missing newlines in launcher.properties Mandy On 5/30/18 12:00 PM, Jonathan Gibbons wrote: Please review a minor update to the proposed implementation of JEP 330. The primary change is to disallow the use of the "shebang"

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-30 Thread Jonathan Gibbons
Please review a minor update to the proposed implementation of JEP 330. The primary change is to disallow the use of the "shebang" feature in Java source files (i.e. files whose name ends in ".java") as recently proposed on this list [1]. There is some additional minor cleanup to the

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-11 Thread Jan Lahoda
On 11.5.2018 12:31, Maurizio Cimadamore wrote: Thumbs up - thanks for taking the comments into account. Great job! +1 Jan Maurizio On 04/05/18 22:59, Jonathan Gibbons wrote: Here's an update to the previously proposed patch for JEP 330: Launch Single-File Source-Code Programs. It

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-11 Thread Maurizio Cimadamore
Thumbs up - thanks for taking the comments into account. Great job! Maurizio On 04/05/18 22:59, Jonathan Gibbons wrote: Here's an update to the previously proposed patch for JEP 330: Launch Single-File Source-Code Programs. It includes all review feedback so far. The changes are mostly

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-09 Thread mandy chung
On 5/4/18 2:59 PM, Jonathan Gibbons wrote: Here's an update to the previously proposed patch for JEP 330: Launch Single-File Source-Code Programs. It includes all review feedback so far. The changes are mostly minor, but with the addition of more test cases. The webrev includes a

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-04 Thread Jonathan Gibbons
Here's an update to the previously proposed patch for JEP 330: Launch Single-File Source-Code Programs. It includes all review feedback so far. The changes are mostly minor, but with the addition of more test cases. The webrev includes a delta-webrev for those that just want to see what has

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-25 Thread Jonathan Gibbons
Kumar, Thank you for your feedback; I will incorporate it in the next webrev. -- Jon On 04/25/2018 09:38 AM, Kumar Srinivasan wrote: Hi John, I focused mainly on the native side, looks ok, except for a couple of minor issues. java.c 1320 const char *prop =

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-25 Thread Kumar Srinivasan
Hi John, I focused mainly on the native side, looks ok, except for a couple of minor issues. java.c 1320 const char *prop = "-Djdk.internal.javac.source="; 1321 size_t size = JLI_StrLen(prop) + JLI_StrLen(value) + 1; 1322 char *propValue = (char

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-24 Thread mandy chung
On 4/25/18 8:53 AM, Jonathan Gibbons wrote: On 04/12/2018 10:20 PM, mandy chung wrote: This looks quite good to me.  One small comment on the source launcher Main class: 122 } catch (InvocationTargetException e) { 123 // leave VM to handle the stacktrace, in the

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-24 Thread Jonathan Gibbons
On 04/12/2018 10:20 PM, mandy chung wrote: On 4/13/18 4:15 AM, Jonathan Gibbons wrote: Please review an initial implementation for the feature described in JEP 330: Launch Single-File Source-Code Programs. The work is described in the JEP and CSR, and falls into various parts: * The part

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-13 Thread Maurizio Cimadamore
One small followup question - the test SourceLauncherTest is not covering any shebang cases - is that deliberate? I see that those seem to be covered in the launcher test too, but I wonder if we should have tests for clearly broken stuff, such as '#' '#!' '#!\n' Another small question - I

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-13 Thread Jan Lahoda
The javac part looks OK to me. A nit comment, in: launcher/Main.java/MemoryFileManager#createInMemoryClassFile, there is: return new FilterOutputStream(new ByteArrayOutputStream()) { ... It could I think be written as: return new ByteArrayOutputStream() { @Override public void close()

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-12 Thread mandy chung
On 4/13/18 4:15 AM, Jonathan Gibbons wrote: Please review an initial implementation for the feature described in JEP 330: Launch Single-File Source-Code Programs. The work is described in the JEP and CSR, and falls into various parts:  * The part to handle the new command-line options is in

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-12 Thread Maurizio Cimadamore
Looks great - some initial comments (I can't really comment on the launcher changes): * This logic is efficient: int magic = (in.read() << 8) + in.read(); boolean shebang = magic == (('#' << 8) + '!'); but convoluted to read; perhaps could be improved slightly by making '#' << 8) + '!' a

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-04-12 Thread Magnus Ihse Bursie
On 2018-04-12 22:15, Jonathan Gibbons wrote: Please review an initial implementation for the feature described in JEP 330: Launch Single-File Source-Code Programs. The work is described in the JEP and CSR, and falls into various parts:  * The part to handle the new command-line options is in