Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Andrey Nazarov]

Looks good, thanks

—Andrey
> On 8 Dec 2016, at 19:16, Alan Bateman  wrote:
> 
> 
> 
> On 08/12/2016 13:09, Chris Hegarty wrote:
>> :
>>> 
>>> Looks good.  I agree with Alan and it’d be good to take the destination 
>>> directory to which the contents are written.  FYI.  jimage extract command 
>>> takes —-dir option.
>> Updated webrev contain a target destination dir:
>>   http://cr.openjdk.java.net/~chegar/8166568_8169492.01/
>> 
> --dir looks good, thanks for adding that.
> 
> -Alan

Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Alan Bateman]

On 08/12/2016 13:09, Chris Hegarty wrote:

:


Looks good.  I agree with Alan and it’d be good to take the destination 
directory to which the contents are written.  FYI.  jimage extract command 
takes —-dir option.

Updated webrev contain a target destination dir:
   http://cr.openjdk.java.net/~chegar/8166568_8169492.01/


--dir looks good, thanks for adding that.

-Alan

Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Mandy Chung]

> On Dec 8, 2016, at 5:09 AM, Chris Hegarty  wrote:
> 
> 
> Updated webrev contain a target destination dir:
>  http://cr.openjdk.java.net/~chegar/8166568_8169492.01/
> 

Looks fine (including the separate patch to JmodTest.java).

Mandy

Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Chris Hegarty]

> On 8 Dec 2016, at 14:45, Andrey Nazarov  wrote:
> 
> Hi,
> 
> Still no tests when directory where files are extracted is not empty. It very 
> common case.

Consider this patch added: 

  diff --git a/test/tools/jmod/JmodTest.java b/test/tools/jmod/JmodTest.java
--- a/test/tools/jmod/JmodTest.java
+++ b/test/tools/jmod/JmodTest.java
@@ -177,6 +177,11 @@
 jmod("extract",
  "--dir", "extractTestDir",
  MODS_DIR.resolve("fooExtractDir.jmod").toString())
+.assertSuccess();
+
+jmod("extract",
+ "--dir", "extractTestDir",
+ MODS_DIR.resolve("fooExtractDir.jmod").toString())
 .assertSuccess()
 .resultChecker(r -> {
 // check a sample of the extracted files


When the dir is not empty, it just overwrites / adds.

-Chris.

> —Andrey 
>> On 8 Dec 2016, at 16:09, Chris Hegarty  wrote:
>> 
>> 
>>> On 8 Dec 2016, at 00:44, Mandy Chung  wrote:
>>> 
>>> 
 On Dec 6, 2016, at 2:46 AM, Chris Hegarty  wrote:
 
 This change adds a basic option to the jmod tool to extract all its 
 contents to
 the current working directory, 8166568 [1]. Additionally, there is a bug 
 fix for
 a public mutable static, 8169492 [2].
 
 http://cr.openjdk.java.net/~chegar/8166568_8169492.00/
>>> 
>>> 
>>> Looks good.  I agree with Alan and it’d be good to take the destination 
>>> directory to which the contents are written.  FYI.  jimage extract command 
>>> takes —-dir option.
>> 
>> Updated webrev contain a target destination dir:
>> http://cr.openjdk.java.net/~chegar/8166568_8169492.01/
>> 
>> -Chris
>> 
> 

Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Mandy Chung]

> On Dec 6, 2016, at 2:46 AM, Chris Hegarty  wrote:
> 
> This change adds a basic option to the jmod tool to extract all its contents 
> to
> the current working directory, 8166568 [1]. Additionally, there is a bug fix 
> for
> a public mutable static, 8169492 [2].
> 
> http://cr.openjdk.java.net/~chegar/8166568_8169492.00/


Looks good.  I agree with Alan and it’d be good to take the destination 
directory to which the contents are written.  FYI.  jimage extract command 
takes —-dir option.

Mandy

Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Andrey Nazarov]

Hi Chris,

Changes looks good. I think tests should be added for case when there are 
extracted files in directory.


—Andrey
> On 6 Dec 2016, at 14:44, Chris Hegarty <chris.hega...@oracle.com> wrote:
> 
> [ forwarding to a more appropriate list to review this change ]
> 
>> Begin forwarded message:
>> 
>> From: Chris Hegarty <chris.hega...@oracle.com>
>> Subject: RFR [9] 8166568 & 8169492 jmod extract and bug fix
>> Date: 6 December 2016 at 10:46:08 GMT
>> To: core-libs-dev <core-libs-...@openjdk.java.net>
>> 
>> This change adds a basic option to the jmod tool to extract all its contents 
>> to
>> the current working directory, 8166568 [1]. Additionally, there is a bug fix 
>> for
>> a public mutable static, 8169492 [2].
>> 
>> http://cr.openjdk.java.net/~chegar/8166568_8169492.00/
>> 
>> -Chris.
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8166568
>> [2] https://bugs.openjdk.java.net/browse/JDK-8169492
> 

Re: Fwd: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Alan Bateman]

On 06/12/2016 11:44, Chris Hegarty wrote:


[ forwarding to a more appropriate list to review this change ]


Begin forwarded message:

From: Chris Hegarty <chris.hega...@oracle.com>
Subject: RFR [9] 8166568 & 8169492 jmod extract and bug fix
Date: 6 December 2016 at 10:46:08 GMT
To: core-libs-dev <core-libs-...@openjdk.java.net>

This change adds a basic option to the jmod tool to extract all its contents to
the current working directory, 8166568 [1]. Additionally, there is a bug fix for
a public mutable static, 8169492 [2].

http://cr.openjdk.java.net/~chegar/8166568_8169492.00/

This looks okay to me although I would expect it would be useful to 
extract to a specific location too.


-Alan

Fwd: RFR [9] 8166568 & 8169492 jmod extract and bug fix

[Chris Hegarty]

[ forwarding to a more appropriate list to review this change ]

> Begin forwarded message:
> 
> From: Chris Hegarty <chris.hega...@oracle.com>
> Subject: RFR [9] 8166568 & 8169492 jmod extract and bug fix
> Date: 6 December 2016 at 10:46:08 GMT
> To: core-libs-dev <core-libs-...@openjdk.java.net>
> 
> This change adds a basic option to the jmod tool to extract all its contents 
> to
> the current working directory, 8166568 [1]. Additionally, there is a bug fix 
> for
> a public mutable static, 8169492 [2].
> 
> http://cr.openjdk.java.net/~chegar/8166568_8169492.00/
> 
> -Chris.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8166568
> [2] https://bugs.openjdk.java.net/browse/JDK-8169492