[modules-dev] Review request for 6608500

[Dave Bristor]

Stanley M. Ho wrote:
> 
> Dave Bristor wrote:
>> This is the prior in behavior, unchanged.  I changed code so it's now 
>> in temp. But then I observed that, since we don't set the property in 
>> all our tests, an expansion directory is created for each test in 
>> temp!  Kelly / JPRT won't be pleased, and that's contrary to the point 
>> of this bugfix.  I propose to revert the code for now, and cause the 
>> expansion directory to be created on an as-needed basis, during 
>> install(), as we discussed yesterday w.r.t. the other bug on my plate, 
>> 6605100.  An alternative would be to modify all of our tests to set 
>> the extension directory's path as part of this fix.  The webrev has 
>> the code reverted to using source directory.
> 
> I'm okay if we revert the code for now, but I think we should definitely 
> address it in a bug fix soon. Regardless, expanding into the source 
> directory won't be the correct behavior; the expanded directory should 
> be either somewhere in temp, or in the repository's cache defined in the 
> implementation.
> 
> I think the real problem in the tests is that we don't remove the 
> expansion directory after each test is executed. Modifying all of our 
> tests to set the expansion directory would be one way. An alternative is 
> to call Repository.shutdown() before each test exits (or implement the 
> shutdownOnExit() method), and the repository's shutdown could clean up 
> the expansion directory automatically.

I think we have to wait for releaseModule / disableModuleDefinition before 
this will work: tests on Windows fail because some code has a file descriptor 
open.  URLRepository.shutdown() does try to remove them but does not ascertain 
that they have been removed.  LocalRepository.shutdown() doesn't have file 
removal yet implemented, but we have 6559067 which I can do Real Soon Now.

Thanks,
	Dave

> 
> - Stanley