Re: RFR: 8256308: Send arguments to javac server in a config file [v6]
On Tue, 24 Nov 2020 09:52:34 GMT, Joel Borggrén-Franck
wrote:
>> Magnus Ihse Bursie has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix whitespace issues
>
> src/jdk.compiler/share/classes/com/sun/tools/sjavac/Util.java line 96:
>
>> 94:
>> 95: public static String extractStringOptionLine(String opName, String
>> s, String deflt) {
>> 96: return extractStringOptionWithDelimiter(opName, s, deflt,
>> '\n').strip();
>
> Is '\n' going to be problematic due to differences in line endings on various
> platforms?
The JDK project is limited to unix-style line endings only. No other styles are
supported, and any attempt to use such will break the build (often early on).
-
PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v6]
On Mon, 16 Nov 2020 12:44:08 GMT, Magnus Ihse Bursie wrote:
>> Currently, to use the javac server, a horrendously long command line option
>> is created, looking like this: `--server:portfile=> portfile>:sjavac=`, where the sjavac command has
>> had all spaces replaced by %20. Since Project Jigsaw, the set of module
>> arguments needed is huge to begin with, making this command line
>> incomprehensible after mangling.
>>
>> Apart from making java command lines hard to read (and copy/paste!) by
>> developers, it also makes it hard for scripts to parse. The upcoming winenv
>> rewrite is dependent on being able to differentiate between path names and
>> other arguments, which is not possible in this mess.
>>
>> So, instead, let's write it to a file, without any escaping, and just pass
>> the configuration file name to the server.
>>
>> Note that this will change the behavior of the javac server, but as the
>> source code states this is not a documented or externally supported API no
>> CSR is needed.
>>
>> I also cleaned up some code in SjavacClient, in particular code relating to
>> the passing of arguments. (We never change poolsize or keepalive when we
>> call it.)
>
> Magnus Ihse Bursie has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix whitespace issues
Marked as reviewed by jfranck (Reviewer).
src/jdk.compiler/share/classes/com/sun/tools/sjavac/Util.java line 96:
> 94:
> 95: public static String extractStringOptionLine(String opName, String s,
> String deflt) {
> 96: return extractStringOptionWithDelimiter(opName, s, deflt,
> '\n').strip();
Is '\n' going to be problematic due to differences in line endings on various
platforms?
-
PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v6]
On Tue, 17 Nov 2020 23:00:26 GMT, Jonathan Gibbons wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix whitespace issues > > There seems to be much more here than there needs to be. > > If the issue is just long command lines, then the obvious/conventional > solution would be to use @files, which would be a tiny change to the sjavac > source code, to insert a single call to `CommandLine.parse` to expand any > @file arguments on the command line. > > So, before reading all the various details in this proposed change, is there > any reason why the simple @file solution cannot be used? The main issue is not the long command lines. Getting shorter command lines is nice, but more a side-effect. The real driver here is the awkward encoding of the command line, with spaces replaced by %20. This makes it impossible to parse the --server option to Java and properly replace unix paths with Windows paths, in the way that is needed by the upcoming changes to how we build on Windows. - PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v6]
On Mon, 16 Nov 2020 12:44:08 GMT, Magnus Ihse Bursie wrote: >> Currently, to use the javac server, a horrendously long command line option >> is created, looking like this: `--server:portfile=> portfile>:sjavac=`, where the sjavac command has >> had all spaces replaced by %20. Since Project Jigsaw, the set of module >> arguments needed is huge to begin with, making this command line >> incomprehensible after mangling. >> >> Apart from making java command lines hard to read (and copy/paste!) by >> developers, it also makes it hard for scripts to parse. The upcoming winenv >> rewrite is dependent on being able to differentiate between path names and >> other arguments, which is not possible in this mess. >> >> So, instead, let's write it to a file, without any escaping, and just pass >> the configuration file name to the server. >> >> Note that this will change the behavior of the javac server, but as the >> source code states this is not a documented or externally supported API no >> CSR is needed. >> >> I also cleaned up some code in SjavacClient, in particular code relating to >> the passing of arguments. (We never change poolsize or keepalive when we >> call it.) > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix whitespace issues There seems to be much more here than there needs to be. If the issue is just long command lines, then the obvious/conventional solution would be to use @files, which would be a tiny change to the sjavac source code, to insert a single call to `CommandLine.parse` to expand any @file arguments on the command line. So, before reading all the various details in this proposed change, is there any reason why the simple @file solution cannot be used? - PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v6]
On Mon, 16 Nov 2020 12:44:08 GMT, Magnus Ihse Bursie wrote: >> Currently, to use the javac server, a horrendously long command line option >> is created, looking like this: `--server:portfile=> portfile>:sjavac=`, where the sjavac command has >> had all spaces replaced by %20. Since Project Jigsaw, the set of module >> arguments needed is huge to begin with, making this command line >> incomprehensible after mangling. >> >> Apart from making java command lines hard to read (and copy/paste!) by >> developers, it also makes it hard for scripts to parse. The upcoming winenv >> rewrite is dependent on being able to differentiate between path names and >> other arguments, which is not possible in this mess. >> >> So, instead, let's write it to a file, without any escaping, and just pass >> the configuration file name to the server. >> >> Note that this will change the behavior of the javac server, but as the >> source code states this is not a documented or externally supported API no >> CSR is needed. >> >> I also cleaned up some code in SjavacClient, in particular code relating to >> the passing of arguments. (We never change poolsize or keepalive when we >> call it.) > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix whitespace issues Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v6]
> Currently, to use the javac server, a horrendously long command line option > is created, looking like this: `--server:portfile= portfile>:sjavac=`, where the sjavac command has > had all spaces replaced by %20. Since Project Jigsaw, the set of module > arguments needed is huge to begin with, making this command line > incomprehensible after mangling. > > Apart from making java command lines hard to read (and copy/paste!) by > developers, it also makes it hard for scripts to parse. The upcoming winenv > rewrite is dependent on being able to differentiate between path names and > other arguments, which is not possible in this mess. > > So, instead, let's write it to a file, without any escaping, and just pass > the configuration file name to the server. > > Note that this will change the behavior of the javac server, but as the > source code states this is not a documented or externally supported API no > CSR is needed. > > I also cleaned up some code in SjavacClient, in particular code relating to > the passing of arguments. (We never change poolsize or keepalive when we call > it.) Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix whitespace issues - Changes: - all: https://git.openjdk.java.net/jdk/pull/1195/files - new: https://git.openjdk.java.net/jdk/pull/1195/files/44507d25..3015d5e4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1195=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1195=04-05 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/1195.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1195/head:pull/1195 PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file
On Thu, 12 Nov 2020 22:13:37 GMT, Magnus Ihse Bursie wrote: > Currently, to use the javac server, a horrendously long command line option > is created, looking like this: `--server:portfile= portfile>:sjavac=`, where the sjavac command has > had all spaces replaced by %20. Since Project Jigsaw, the set of module > arguments needed is huge to begin with, making this command line > incomprehensible after mangling. > > Apart from making java command lines hard to read (and copy/paste!) by > developers, it also makes it hard for scripts to parse. The upcoming winenv > rewrite is dependent on being able to differentiate between path names and > other arguments, which is not possible in this mess. > > So, instead, let's write it to a file, without any escaping, and just pass > the configuration file name to the server. > > Note that this will change the behavior of the javac server, but as the > source code states this is not a documented or externally supported API no > CSR is needed. > > I also cleaned up some code in SjavacClient, in particular code relating to > the passing of arguments. (We never change poolsize or keepalive when we call > it.) Looks good in general, just some whitespace issues. - PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v5]
On Fri, 13 Nov 2020 01:08:09 GMT, Magnus Ihse Bursie wrote: >> Currently, to use the javac server, a horrendously long command line option >> is created, looking like this: `--server:portfile=> portfile>:sjavac=`, where the sjavac command has >> had all spaces replaced by %20. Since Project Jigsaw, the set of module >> arguments needed is huge to begin with, making this command line >> incomprehensible after mangling. >> >> Apart from making java command lines hard to read (and copy/paste!) by >> developers, it also makes it hard for scripts to parse. The upcoming winenv >> rewrite is dependent on being able to differentiate between path names and >> other arguments, which is not possible in this mess. >> >> So, instead, let's write it to a file, without any escaping, and just pass >> the configuration file name to the server. >> >> Note that this will change the behavior of the javac server, but as the >> source code states this is not a documented or externally supported API no >> CSR is needed. >> >> I also cleaned up some code in SjavacClient, in particular code relating to >> the passing of arguments. (We never change poolsize or keepalive when we >> call it.) > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > MODULE_SUBDIR must be defined before using make/common/JavaCompilation.gmk line 237: > 235: $1_JAVAC_SERVER_CMD := $$(JAVA_DETACH) $$($1_JAVA_FLAGS) > $$($1_JAVAC) > 236: > 237: $1_CONFIG_VARDEPS :=$$($1_JAVAC_PORT_FILE) $$($1_JAVAC_SERVER_CMD) Space make/common/JavaCompilation.gmk line 239: > 237: $1_CONFIG_VARDEPS :=$$($1_JAVAC_PORT_FILE) $$($1_JAVAC_SERVER_CMD) > 238: $1_CONFIG_VARDEPS_FILE := $$(call DependOnVariable, > $1_CONFIG_VARDEPS, \ > 239: $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1.config_vardeps) Indentation 4 make/common/JavaCompilation.gmk line 246: > 244: $1_ECHO_COMMAND := $(ECHO) > 245: endif > 246: $$($1_JAVAC_SERVER_CONFIG): $$($1_CONFIG_VARDEPS_FILE) Indentation? - PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v5]
> Currently, to use the javac server, a horrendously long command line option > is created, looking like this: `--server:portfile= portfile>:sjavac=`, where the sjavac command has > had all spaces replaced by %20. Since Project Jigsaw, the set of module > arguments needed is huge to begin with, making this command line > incomprehensible after mangling. > > Apart from making java command lines hard to read (and copy/paste!) by > developers, it also makes it hard for scripts to parse. The upcoming winenv > rewrite is dependent on being able to differentiate between path names and > other arguments, which is not possible in this mess. > > So, instead, let's write it to a file, without any escaping, and just pass > the configuration file name to the server. > > Note that this will change the behavior of the javac server, but as the > source code states this is not a documented or externally supported API no > CSR is needed. > > I also cleaned up some code in SjavacClient, in particular code relating to > the passing of arguments. (We never change poolsize or keepalive when we call > it.) Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: MODULE_SUBDIR must be defined before using - Changes: - all: https://git.openjdk.java.net/jdk/pull/1195/files - new: https://git.openjdk.java.net/jdk/pull/1195/files/ba312950..44507d25 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1195=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1195=03-04 Stats: 8 lines in 1 file changed: 4 ins; 4 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/1195.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1195/head:pull/1195 PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v4]
> Currently, to use the javac server, a horrendously long command line option > is created, looking like this: `--server:portfile= portfile>:sjavac=`, where the sjavac command has > had all spaces replaced by %20. Since Project Jigsaw, the set of module > arguments needed is huge to begin with, making this command line > incomprehensible after mangling. > > Apart from making java command lines hard to read (and copy/paste!) by > developers, it also makes it hard for scripts to parse. The upcoming winenv > rewrite is dependent on being able to differentiate between path names and > other arguments, which is not possible in this mess. > > So, instead, let's write it to a file, without any escaping, and just pass > the configuration file name to the server. > > Note that this will change the behavior of the javac server, but as the > source code states this is not a documented or externally supported API no > CSR is needed. > > I also cleaned up some code in SjavacClient, in particular code relating to > the passing of arguments. (We never change poolsize or keepalive when we call > it.) Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Some rules had identical names which still caused races, so move config file - Changes: - all: https://git.openjdk.java.net/jdk/pull/1195/files - new: https://git.openjdk.java.net/jdk/pull/1195/files/c47f7c2a..ba312950 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1195=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1195=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1195.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1195/head:pull/1195 PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v3]
> Currently, to use the javac server, a horrendously long command line option > is created, looking like this: `--server:portfile= portfile>:sjavac=`, where the sjavac command has > had all spaces replaced by %20. Since Project Jigsaw, the set of module > arguments needed is huge to begin with, making this command line > incomprehensible after mangling. > > Apart from making java command lines hard to read (and copy/paste!) by > developers, it also makes it hard for scripts to parse. The upcoming winenv > rewrite is dependent on being able to differentiate between path names and > other arguments, which is not possible in this mess. > > So, instead, let's write it to a file, without any escaping, and just pass > the configuration file name to the server. > > Note that this will change the behavior of the javac server, but as the > source code states this is not a documented or externally supported API no > CSR is needed. > > I also cleaned up some code in SjavacClient, in particular code relating to > the passing of arguments. (We never change poolsize or keepalive when we call > it.) Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Paths on Windows must be converted before being written to the config file - Changes: - all: https://git.openjdk.java.net/jdk/pull/1195/files - new: https://git.openjdk.java.net/jdk/pull/1195/files/0a613f55..c47f7c2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1195=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1195=01-02 Stats: 8 lines in 1 file changed: 5 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/1195.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1195/head:pull/1195 PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v2]
> Currently, to use the javac server, a horrendously long command line option > is created, looking like this: `--server:portfile= portfile>:sjavac=`, where the sjavac command has > had all spaces replaced by %20. Since Project Jigsaw, the set of module > arguments needed is huge to begin with, making this command line > incomprehensible after mangling. > > Apart from making java command lines hard to read (and copy/paste!) by > developers, it also makes it hard for scripts to parse. The upcoming winenv > rewrite is dependent on being able to differentiate between path names and > other arguments, which is not possible in this mess. > > So, instead, let's write it to a file, without any escaping, and just pass > the configuration file name to the server. > > Note that this will change the behavior of the javac server, but as the > source code states this is not a documented or externally supported API no > CSR is needed. > > I also cleaned up some code in SjavacClient, in particular code relating to > the passing of arguments. (We never change poolsize or keepalive when we call > it.) Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix race where multiple compilations overwrite the config file - Changes: - all: https://git.openjdk.java.net/jdk/pull/1195/files - new: https://git.openjdk.java.net/jdk/pull/1195/files/26e5efdd..0a613f55 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1195=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1195=00-01 Stats: 6 lines in 1 file changed: 2 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1195.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1195/head:pull/1195 PR: https://git.openjdk.java.net/jdk/pull/1195
RFR: 8256308: Send arguments to javac server in a config file
Currently, to use the javac server, a horrendously long command line option is created, looking like this: `--server:portfile=:sjavac=`, where the sjavac command has had all spaces replaced by %20. Since Project Jigsaw, the set of module arguments needed is huge to begin with, making this command line incomprehensible after mangling. Apart from making java command lines hard to read (and copy/paste!) by developers, it also makes it hard for scripts to parse. The upcoming winenv rewrite is dependent on being able to differentiate between path names and other arguments, which is not possible in this mess. So, instead, let's write it to a file, without any escaping, and just pass the configuration file name to the server. Note that this will change the behavior of the javac server, but as the source code states this is not a documented or externally supported API no CSR is needed. I also cleaned up some code in SjavacClient, in particular code relating to the passing of arguments. (We never change poolsize or keepalive when we call it.) - Commit messages: - 8256308: Send arguments to javac server in a config file Changes: https://git.openjdk.java.net/jdk/pull/1195/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1195=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256308 Stats: 118 lines in 3 files changed: 48 ins; 33 del; 37 mod Patch: https://git.openjdk.java.net/jdk/pull/1195.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1195/head:pull/1195 PR: https://git.openjdk.java.net/jdk/pull/1195
