RFR: jmod remove

Alan Bateman Alan.Bateman at oracle.com
Wed May 2 07:23:56 PDT 2012


On 30/04/2012 22:22, Chris Hegarty wrote:
> Thanks Jon and Paul, both very good suggestions.
>
> Updated:
>   http://cr.openjdk.java.net/~chegar/jigsaw/jmodrm.04/webrev/
Thanks for adding "rm" as a command and fixing the other issues that 
came up in the first round.

I've looked at the latest webrev and it's looks much better.

Paul's suggestion to atomically move the module directory to a trash 
location is good suggestion and I see you've named it ".trash". One 
comment on moduleTrashDir is that it duplicates the logic for temporary 
directories and maybe it would be simper to use use 
Files.createTempDirectory to create a directory in the trash directory.

A few minor comments:

Library.java defines the mids parameter, not not the dry parameter.

SimpleLibrary.remove, you can use try-with-resources at L1501.

checkRootsRequire - I wonder if we can find a better name for this 
method, the logic seems right and should work for service usage too but 
the name of the method seems a bit odd. Maybe 
ensureNotPresentInConfiguration or something short?

Just on consistency of naming, then with the change then Files defines 
both deleteTree and deleteAllUnchecked to do a recursive delete which 
seems a bit inconsistent.

Shouldn't Files. deleteUnchecked just return any exception thrown by 
delete rather than dropping the useful exception that delete throws?

Nothing really to do with this patch but ModuleFile.remove(File) could 
use Files.readAllLines which would remove much of the code.

Aside from a bit of clean-up then I think this will be great to have 
(and urgently needed now because %mids means that we can't rm modules 
anymore).

-Alan




More information about the jigsaw-dev mailing list