RFR: Fix jtreg test java/lang/ProcessBuilder/Basic.java

[Mikael Vidstedt]

Axel,

Thanks for looking into and fixing this! A few questions and nits:

childproc.c:

The whole exec dance is fascinating to decode, but AFAICT your change is correct. Feel free to leave the code as-is, but just for completeness I’ll mention that if I read the existing code correctly, removing the first if-clause completely should also work (that said, there could well be something subtle I’m missing).

Can I suggest the following comment:

"ENOEXEC indicates that the file header was not recognized. The musl C library does not implement the fallback to /bin/sh for that case, so fall through to the code below which implements that fallback using execve_with_shell_fallback."


Basic.java:

* There are a couple of additional reference to /bin/false and /bin/true, lines 374 and 1997 in the new version, was that deliberate or should they also be updated?

* Missing space after BusyBox on line 616

* The formatting of the initialization of the "boolean is” on lines 619-620 seem off

* Can you please add a comment somewhere about why BusyBox needs special treatment - basically what you said in your initial email?

* Why was the change to EnglishUnix needed? That is, what is the difference in the message?

* There’s an extra space after the added check on line 696

Cheers,
Mikael

> On Mar 14, 2018, at 1:00 AM, Siebenborn, Axel <axel.siebenborn at sap.com> wrote:
> 
> Good catch Volker! 
> I forgot the link to the webrev:
> I'm a bit out of practice.
> 
> http://cr.openjdk.java.net/~asiebenborn/jtreg_ProcessBuilder/webrev/
> 
> Thanks, 
> Axel
> 
> 
> 
> -----Original Message-----
> From: Volker Simonis [mailto:volker.simonis at gmail.com] 
> Sent: Dienstag, 13. März 2018 19:39
> To: Siebenborn, Axel <axel.siebenborn at sap.com>
> Cc: portola-dev at openjdk.java.net
> Subject: Re: RFR: Fix jtreg test java/lang/ProcessBuilder/Basic.java
> 
> You probably also wanted to post this URL to your webrev :)
> 
> http://cr.openjdk.java.net/~asiebenborn/jtreg_ProcessBuilder/webrev/
> 
> In general, how should we proceed with fixes for portola? Just post
> webrevs or do you expect us to open bugs in JBS for every issue?
> 
> Thanks,
> Volker
> 
> 
> On Tue, Mar 13, 2018 at 12:28 PM, Siebenborn, Axel
> <axel.siebenborn at sap.com> wrote:
>> Hi,
>> 
>> I'm currently looking into jtreg failures,  that occur with the portola jdk.
>> 
>> A first issue I tackled, was the failing test 'java/lang/ProcessBuilder/Basic.java'
>> 
>> The test copies the executable '/bin/true' and '/bin/false'. However, on alpine linux, these are just links to /bin/busybox.
>> Copying the busbox executable into a file with a different name won't result in the expected return codes.
>> 
>> Another problem with this test is a bug in musl, that probably won't be fixed [1]. 'execvp' has not the fallback to /bin/sh.
>> 
>> Regards,
>> Axel
>> 
>> [1] http://www.openwall.com/lists/musl/2018/03/09/2
>> 
>>