RFR 8192986: Inconsistent handling of exploded modules in jlink

[Sundararajan Athijegannathan]

Hi,

* On stricter check for missing module-info.class in a directory: There 
is a similar check for non-modular jars.

jlink --add-modules java.base --module-path t.jar  --output foo
Error: Unable to derive module descriptor for t.jar

t.jar in the above command is a non-modular jar. I think it is better to 
detect and report non-modular exploded dirs as well.

* Fixed to avoid second resolve for "module-info.class" in JlinkTask.java

Please review updated webrev: 
http://cr.openjdk.java.net/~sundar/8192986/webrev.02/

Thanks
-Sundar

On 07/12/17, 9:15 PM, Sundararajan Athijegannathan wrote:
> Hi,
>
> Comments below...
>
> On 07/12/17, 8:54 PM, Claes Redestad wrote:
>> Hi Sundar,
>>
>> thanks for picking this up so quick!
>>
>> On 2017-12-07 16:21, Sundararajan Athijegannathan wrote:
>>> Updated: http://cr.openjdk.java.net/~sundar/8192986/webrev.01/
>>
>> Looks ok, butunless my understanding is flawed it seems the logic is 
>> now getting more strict about a directory on the module path 
>> containing a well-formed module. Should this be made more graceful, 
>> say ignore empty directories? Maybe just warn about malformed and/or 
>> missing modules?
> I'd prefer stricter checks. But I'd like to hear from others as well...
>> Nits:
>>
>> JlinkTask: resolves module-info.class twice (resolve once and pass as 
>> parameter?)
>
> Yes, I'll fix that.
>
>>
>> ExplodedModuleNameTest:
>>
>>   58         if (helper == null) {
>>   59             System.err.println("Test not run");
>>   60             return;
>>   61         }
>>
>> Should this fail the test (by throwing an exception)?
>
> This is similar to other tests. For eg. ModuleNamesOrderTest
>
>>   66         // rename the module containing directory
>>   67         Path renamedModDir = 
>> modDir.resolveSibling("modified_mod8192986");
>>   68         // copy the content from original directory to modified 
>> name directory
>>   69         copyDir(modDir, renamedModDir);
>>
>> Any reason not to use Files.move(modDir, 
>> renamedModDir|,StandardCopyOption.REPLACE_EXISTING|) instead of 
>> copying here?
>>
>
> https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#move(java.nio.file.Path,%20java.nio.file.Path,%20java.nio.file.CopyOption...) 
>
>
> "To move a file tree may involve copying rather than moving 
> directories and this can be done using the copy method in conjunction 
> with the Files.walkFileTree utility method."
>
> Thanks,
> -Sundar
>
>> Thanks!
>>
>> /Claes
>>