RFR v4 - 8027634: Support @argfiles for java command-line tool

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Thu Aug 13 16:38:09 UTC 2015


<trimmed the cc: list>

I am still not complete, but here are some more comments and specifics.


-static int firstAppArgIndex = -1;

+#define NOT_FOUND  -1;
+static int firstAppArgIndex = NOT_FOUND;

     // Any @argument comes in here is an argument as is.
     // Expansion should had done before checkArg called.

I think you are trying to say
    // All arguments arrive here must be a launcher argument,
    // ie. by now, all argfile expansions must have been performed.

3. void JLI_InitArgProcessing(jboolean isJava, jboolean disableArgFile) {
     // No expansion for relaunch

The launcher might re-exec itself, this might happen under certain
circumstances on *nix, is the above logic to prevent subsequent
expansion on relaunch ? However when a re-exec occurs, the
expanded args will be sent to the second invocation of the launcher.

So what does "relaunch" mean ? in your comment ?


usage of ArrayList vs. Arrays will simplify things.

     // arg file content,  expected options
     static String[] testCaseArrays[][] = {
         { // empty file
             {}, {}

     public List<List<List<String>>> loadCases() {
         List<List<List<String>>> rv = new ArrayList<>();
         for (String[][] testCaseArray : testCaseArrays) {
             List<List<String>> testCase = new ArrayList<>(2);
         // long lines
     public void testSyntax() throws IOException {
         List<List<List<String>>> allCases = loadCases();
         for (List<List<String>> testCase : allCases) {
             verifyParsing(testCase.get(0), testCase.get(1));


use case coverage looks great, still haven't finished with this


>> On Aug 12, 2015, at 7:55 AM, Kumar Srinivasan <kumar.x.srinivasan at oracle.com> wrote:
>> Henry,
>> Generally looks good here are some comments, on my initial
>> pass, I am not fully done with args.c I will look at this some
>> more later today or tomorrow.
>> 4.)
>> expectingNoDash is expectingMain right ? if so I would rename it so.
> Not really, it is expecting a argument without dash, such as -cp or -classpath.
>> TestHelper.java:
>> Why add findInOutput method ? this seems to be identical
>> to "matches" at line 606, which does exactly that.
> To return a string, this is used when trying to match a pattern and get information from that line, I used to get the starting index for an argument in verifyOptions.
> Guess I can remove that now as each use of verifyOptions all starts from 1.
>> ArgFileSyntax.java
>> Nice, but I think the creation and storage of test case can be simplified,
>> basically loadCases(), also please avoid  Xbootclasspath. The trouble
>> with these tests which operate loops, are very painful to debug
>> and isolate a failing case,  on the night before GA.
> I wrote that because I start with testng as a DataProvider, we can change that.
>> Can we do something like this.....
>> Map<String, List<String>> tests = new ArrayList<>();
>> tests.put("testing quotes", Array.asList({{...}, {....}});
>> tests.add("no recurse expansion", Array.asList({{...}, {....}});
>> The execute each test, with a description of what the test is doing.
>> This way if a test fails its easy to zero in on the failing test.
> verifyOutput will print out what line is not matching, so it’s pretty easy to identify which case failed.
> Cheers,
> Henry

More information about the core-libs-dev mailing list