Re: RFR: 8178867: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java failed to clean up files

[Alan Bateman]

On 10/04/2018 20:03, Andrey Nazarov wrote:



On 10 Apr 2018, at 11:47, Alan Bateman  wrote:

On 10/04/2018 19:44, Andrey Nazarov wrote:

Anyone?


On 6 Apr 2018, at 17:10, Andrey Nazarov  wrote:

Hi,

Please review fix in Jlink test. The fix is to close the Stream which works 
with a file system.

Review: http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.01 

JBS: https://bugs.openjdk.java.net/browse/JDK-8178867


If you want, you can get rid of temporary javaFiles list and use 
.forEach(args::add) instead.

Yes. It looks cleaner. 
http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.03
I agree with Paul that it would be nice to replace new 
ArrayList<>(List.of("-d", ... )) too but what you have is okay.


-Alan

Re: RFR: 8178867: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java failed to clean up files

[Paul Sandoz]

> On Apr 10, 2018, at 11:47 AM, Alan Bateman  wrote:
> 
> On 10/04/2018 19:44, Andrey Nazarov wrote:
>> Anyone?
>> 
>>> On 6 Apr 2018, at 17:10, Andrey Nazarov  wrote:
>>> 
>>> Hi,
>>> 
>>> Please review fix in Jlink test. The fix is to close the Stream which works 
>>> with a file system.
>>> 
>>> Review: http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.01 
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8178867
>>> 
> If you want, you can get rid of temporary javaFiles list and use 
> .forEach(args::add) instead.
> 

Yes. Not suggesting you do this, just for educational purposes you can also do 
this (not tested) e.g.:

var argStream = Stream.of("-d", destination.toString(), "--module-source-path", 
srcpath);
try (var pathStream = Files.walk(source)) {
argStream = Stream.concat(argStream,
pathStream.map(Path::toString).filter(s -> s.endsWith(".java")));

int rc = JAVAC_TOOL.run(System.out, System.err, 
argStream.toArray(String[]::new));
Assert.assertEquals(rc, 0);
}

Paul.

Re: RFR: 8178867: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java failed to clean up files

[Andrey Nazarov]

> On 10 Apr 2018, at 11:47, Alan Bateman  wrote:
> 
> On 10/04/2018 19:44, Andrey Nazarov wrote:
>> Anyone?
>> 
>>> On 6 Apr 2018, at 17:10, Andrey Nazarov  wrote:
>>> 
>>> Hi,
>>> 
>>> Please review fix in Jlink test. The fix is to close the Stream which works 
>>> with a file system.
>>> 
>>> Review: http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.01 
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8178867
>>> 
> If you want, you can get rid of temporary javaFiles list and use 
> .forEach(args::add) instead.
Yes. It looks cleaner. 
http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.03
> 
> -Alan

Re: RFR: 8178867: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java failed to clean up files

[Jonathan Gibbons]

+1


On 4/10/18 11:44 AM, Andrey Nazarov wrote:

Anyone?


On 6 Apr 2018, at 17:10, Andrey Nazarov  wrote:

Hi,

Please review fix in Jlink test. The fix is to close the Stream which works 
with a file system.

Review: http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.01 

JBS: https://bugs.openjdk.java.net/browse/JDK-8178867

—Thanks,
Andrei

Re: RFR: 8178867: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java failed to clean up files

[Alan Bateman]

On 10/04/2018 19:44, Andrey Nazarov wrote:

Anyone?


On 6 Apr 2018, at 17:10, Andrey Nazarov  wrote:

Hi,

Please review fix in Jlink test. The fix is to close the Stream which works 
with a file system.

Review: http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.01 

JBS: https://bugs.openjdk.java.net/browse/JDK-8178867

If you want, you can get rid of temporary javaFiles list and use 
.forEach(args::add) instead.


-Alan

Re: RFR: 8178867: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java failed to clean up files

[Andrey Nazarov]

Anyone?

> On 6 Apr 2018, at 17:10, Andrey Nazarov  wrote:
> 
> Hi,
> 
> Please review fix in Jlink test. The fix is to close the Stream which works 
> with a file system.
> 
> Review: http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.01 
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8178867
> 
> —Thanks,
> Andrei

RFR: 8178867: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java failed to clean up files

[Andrey Nazarov]

Hi,

Please review fix in Jlink test. The fix is to close the Stream which works 
with a file system.

Review: http://cr.openjdk.java.net/~anazarov/JDK-8178867/webrev.01 

JBS: https://bugs.openjdk.java.net/browse/JDK-8178867

—Thanks,
Andrei