RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries

[Alan Bateman]

On 09/04/2019 10:06, Severin Gehwolf wrote:
> :
>> Hi Severin,
>>
>> This is my initial set of comments.
> Thanks for the review! Latest webrev with the updates:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/09/webrev/
>
>
I went through 09/webrev and just have a few comments, mostly on 
StripNativeDebugSymbolsPlugin.

stripBinary uses ProcessBuilder to launch objdump but it doesn't 
redirect or read from the process streams. This could make diagnosing 
problems difficult so I think it should minimally use 
redirectOutput(INHERIT) and redirectError(INHERIT) so that any 
output/errors can be seen by the jlink user.

At L180 there is a question to the reader on whether arg can be null. It 
could be good to answer that if possible and avoid needing to think 
about that.

At L357 there mix of old and new - I assume you can be replaced with 
inFile.toAbsolutePath().toString();

Can you explain the mocking in DefaultStripDebugPluginTest? I think I've 
missed the reason for this test.

In passing, we try to avoid the @author tag as it's so difficult to 
remove them (even if a test is completely re-worked).

-Alan