RFR: jmod remove

[Alan Bateman]

On 19/04/2012 09:47, Chris Hegarty wrote:
> I heard the following expression a few times, 'rm -rf has reached the 
> limit of its usefulness'! So it's about time we had a 'jmod remove'.
>
> The webrev is based upon Mandy's alias support patch [1]. I figured it 
> would most likely be going in after the alias support.
>
> http://cr.openjdk.java.net/~chegar/jigsaw/jmodrm.02/webrev/
>
> Essentially, you can remove any module not required by a 
> configuration. Fails if you try to remove a module which will break a 
> configuration, unless you specify --force, or remove both the root 
> module along with the dependent module.
>
> I added a comment about the parent/child relationship. That is, a 
> module library has no notion of its children so we could remove a 
> module in a parent library that is not part of any configuration in 
> that library, but may be part of a configuration in a child library. I 
> think this is fine for now, but we may have to revisit the 
> parent/child relationship in the future ( beyond the scope of jmod 
> remove ).
Thanks for taking this one, I agree it's very useful to have remove 
implemented even with the issue of child repositories.

I've looked through the changes and overall the approach seems okay. A 
couple of comments on the webrev:

- I think we should consider having Library define the remove method (as 
an abstract method of course).

- On the javadoc then adding @throws ConfigurationException and 
IOException would be useful (it wasn't obvious to me initially that a 
ConfigurationException is thrown when module can't be removed because it 
exists in some configuration). Also it javadoc should make it clear that 
IOException may be thrown after some, but not all, modules have been 
removed.

- In Librarian's static initializer then it would be great to have "rm" 
map to Remove.class too, my preference is "jmod rm" over "jmod remove".

- Suppose that there is an error after one or more modules have been 
removed, shouldn't we still refresh and update the module directory? 
We'll need to make the refresh very robust as bad things will happen in 
the wild.

- removeWhenLocked doesn't check the return from delete so it may 
silently fail (Files.delete is probably called for here as it will give 
the right exception).

- Should the dry run print a message so that it's clear what will be 
removed? Also having the dry run recover from bad input would be useful too.

- SimpleLibrary L1487, typo "forceably" -> "forcibly".

- SimpleLibrary L1507, "FileChannel lc" -> "FileChannel fc".

- It might be better to rename the test to jmod-rm.sh to be consistent 
with the other tests. Also would be possible to include a test that uses 
views so that it works as expected.

Otherwise thanks again for taking this one.

-Alan.