RFR: jmod remove

Chris Hegarty chris.hegarty at oracle.com
Wed May 9 07:31:32 PDT 2012


Alan,

I finally got back to this. Comments inline...

Updated webrev:
    http://cr.openjdk.java.net/~chegar/jigsaw/jmodrm.05/webrev/

> 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.

Right, there was an error in my initial change, we only want to generate 
a unique name here not actually create the directory. If the directory 
is created there could be a problem when doing the move.  I encountered 
this on some Windows systems.

I retained the logic ( same as createTempDirectory ) to generate the 
name, but it could use UUID or similar.

> A few minor comments:
>
> Library.java defines the mids parameter, not not the dry parameter.

Got it, thanks.

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

Done.

> 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?

I went with ensureNotInConfiguration.

> 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.

I renamed this to deleteTreeUnchecked. I kept deleteTreeUnchecked as it 
makes more of an effort to actually delete all the files. Eventually, I 
think we should remove deleteTree and replace it with 
deleteTreeUnchecked, but this could be done separately as there may be 
more discussion around it.

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

Reverted to Files.delete and catch the exception.

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

Thanks, done.

-Chris.

> 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