Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix
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
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
> 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
> 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
Hi, Still no tests when directory where files are extracted is not empty. It very common case. —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
> 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
> 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
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 wrote: > > [ forwarding to a more appropriate list to review this change ] > >> Begin forwarded message: >> >> From: Chris Hegarty >> Subject: RFR [9] 8166568 & 8169492 jmod extract and bug fix >> Date: 6 December 2016 at 10:46:08 GMT >> To: core-libs-dev >> >> 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 >