RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

sundararajan.athijegannathan
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

sundararajan.athijegannathan
Oops. Wrong bug link.

Correct one: https://bugs.openjdk.java.net/browse/JDK-8220700

-Sundar

On 20/08/19 5:17 pm, [hidden email] wrote:
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

Mandy Chung
+1

What we really want is to replace this stop-gap shell script some day
with jlink to generate the native launcher [1].

Mandy
[1] JDK-8182555

On 8/20/19 4:52 AM, [hidden email] wrote:

> Oops. Wrong bug link.
>
> Correct one: https://bugs.openjdk.java.net/browse/JDK-8220700
>
> -Sundar
>
> On 20/08/19 5:17 pm, [hidden email] wrote:
>> Please review.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8182555
>>
>> Webrev: http://cr.openjdk.java.net/~sundar/8220700/webrev.00/
>>
>> Thanks,
>>
>> -Sundar
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

Alan Bateman
On 20/08/2019 17:25, Mandy Chung wrote:
> +1
>
> What we really want is to replace this stop-gap shell script some day
> with jlink to generate the native launcher [1].
I agree. One other thing for stop-gap is a test as we could easily
regress without any of the existing tests catching it.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

sundararajan.athijegannathan
Updated to add a test:

http://cr.openjdk.java.net/~sundar/8220700/webrev.01

PS. There are launcher tests in BasicTest.java and arguments are passed
even - but args are only printed. The new test passes args and checks
the return value that is computed using the args.

Thanks,

-Sundar

On 20/08/19 10:07 pm, Alan Bateman wrote:
> On 20/08/2019 17:25, Mandy Chung wrote:
>> +1
>>
>> What we really want is to replace this stop-gap shell script some day
>> with jlink to generate the native launcher [1].
> I agree. One other thing for stop-gap is a test as we could easily
> regress without any of the existing tests catching it.
>
> -Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

Severin Gehwolf
Hi,

On Wed, 2019-08-21 at 12:23 +0530, [hidden email] wrote:
> Updated to add a test:
>
> http://cr.openjdk.java.net/~sundar/8220700/webrev.01
>
> PS. There are launcher tests in BasicTest.java and arguments are passed
> even - but args are only printed. The new test passes args and checks
> the return value that is computed using the args.

While that's true it's not a regression test for this particular bug,
is it? It's not failing prior this patch! I think what you'd need is
something like this on top:

http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8220700-test-additions.patch

This fails prior 8220700 and passes after.

Thanks,
Severin

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

sundararajan.athijegannathan
In reply to this post by sundararajan.athijegannathan
Thanks Severin for your test improvement suggestion!

Updated: http://cr.openjdk.java.net/~sundar/8220700/webrev.02/index.html

PS. This email is in response to
https://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-August/014273.html 
For some reason, I didn't get Severin's email. Checked it by list archive!

Thanks,

-Sundar

On 21/08/19 12:23 pm, [hidden email] wrote:

> Updated to add a test:
>
> http://cr.openjdk.java.net/~sundar/8220700/webrev.01
>
> PS. There are launcher tests in BasicTest.java and arguments are
> passed even - but args are only printed. The new test passes args and
> checks the return value that is computed using the args.
>
> Thanks,
>
> -Sundar
>
> On 20/08/19 10:07 pm, Alan Bateman wrote:
>> On 20/08/2019 17:25, Mandy Chung wrote:
>>> +1
>>>
>>> What we really want is to replace this stop-gap shell script some
>>> day with jlink to generate the native launcher [1].
>> I agree. One other thing for stop-gap is a test as we could easily
>> regress without any of the existing tests catching it.
>>
>> -Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

Alan Bateman
On 21/08/2019 12:33, [hidden email] wrote:
> Thanks Severin for your test improvement suggestion!
>
> Updated: http://cr.openjdk.java.net/~sundar/8220700/webrev.02/index.html
This looks okay but would be nice if Adder had a comment to make it
clear that it totals the arguments up to the first "--" arg.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

sundararajan.athijegannathan
Hi Alan,

Thanks for your review. I'll add a comment in Adder and push.

Thanks,

-Sundar

On 21/08/19 5:28 pm, Alan Bateman wrote:
> On 21/08/2019 12:33, [hidden email] wrote:
>> Thanks Severin for your test improvement suggestion!
>>
>> Updated: http://cr.openjdk.java.net/~sundar/8220700/webrev.02/index.html
> This looks okay but would be nice if Adder had a comment to make it
> clear that it totals the arguments up to the first "--" arg.
>
> -Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

Severin Gehwolf
In reply to this post by sundararajan.athijegannathan
On Wed, 2019-08-21 at 17:03 +0530, [hidden email] wrote:
> Thanks Severin for your test improvement suggestion!
>
> Updated: http://cr.openjdk.java.net/~sundar/8220700/webrev.02/index.html

Looks good.

Thanks,
Severin

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8220700: jlink generated launcher script needs quoting to avoid parameter expansion

Mandy Chung
In reply to this post by sundararajan.athijegannathan


On 8/21/19 4:33 AM, [hidden email] wrote:
> Thanks Severin for your test improvement suggestion!
>
> Updated: http://cr.openjdk.java.net/~sundar/8220700/webrev.02/index.html
>

The test case is good.

Mandy