Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

[Alan Bateman]

On 09/12/2016 14:09, Remi Forax wrote:


Thanks Alan,
so I withdraw my snarky comment.
Not a problem. There is cleanup to be done in several areas, this is 
one. For the short term then we need to get the implementation aligned 
with the proposals in the JSR and then get the cleanup done before ZBB.


-Alan

Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

[Chris Hegarty]

I'll work with Alan to get these changes re-based on top of
the refactoring in jake. Consider this review request withdrawn.

-Chris.

On 09/12/16 14:09, Remi Forax wrote:

Thanks Alan,
so I withdraw my snarky comment.

Rémi


On December 9, 2016 12:25:19 PM GMT+01:00, Alan Bateman 
 wrote:

On 08/12/2016 14:41, Remi Forax wrote:


Chris,
this patch is rather ugly.
Patching the ModuleDescriptor is not a good idea, maybe it's means

that we need a system of metadata associated with a ModuleDescriptor,

the hash of a module (which is not part of the module spec but the

OpenJDK implementation) can be also considered as metadata.



There is refactoring in flight that drops the JDK-specific meta data
from ModuleDescriptor. Once all the pieces are merged together then it
should look a lot better.

-Alan.

Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

[Remi Forax]

Thanks Alan,
so I withdraw my snarky comment.

Rémi


On December 9, 2016 12:25:19 PM GMT+01:00, Alan Bateman 
 wrote:
>On 08/12/2016 14:41, Remi Forax wrote:
>
>> Chris,
>> this patch is rather ugly.
>> Patching the ModuleDescriptor is not a good idea, maybe it's means
>that we need a system of metadata associated with a ModuleDescriptor,
>> the hash of a module (which is not part of the module spec but the
>OpenJDK implementation) can be also considered as metadata.
>>
>There is refactoring in flight that drops the JDK-specific meta data 
>from ModuleDescriptor. Once all the pieces are merged together then it 
>should look a lot better.
>
>-Alan.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

[Alan Bateman]

On 08/12/2016 14:41, Remi Forax wrote:


Chris,
this patch is rather ugly.
Patching the ModuleDescriptor is not a good idea, maybe it's means that we need 
a system of metadata associated with a ModuleDescriptor,
the hash of a module (which is not part of the module spec but the OpenJDK 
implementation) can be also considered as metadata.

There is refactoring in flight that drops the JDK-specific meta data 
from ModuleDescriptor. Once all the pieces are merged together then it 
should look a lot better.


-Alan.

Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

[Chris Hegarty]

Remi,

> On 8 Dec 2016, at 14:41, Remi Forax <fo...@univ-mlv.fr> wrote:
> 
> Chris,
> this patch is rather ugly.

I agree that it is not pretty, but the changes are mostly mechanical.

> Patching the ModuleDescriptor is not a good idea, maybe it's means that we 
> need a system of metadata associated with a ModuleDescriptor,
> the hash of a module (which is not part of the module spec but the OpenJDK 
> implementation) can be also considered as metadata.

As you have pointed out, this is no different from how hashes are
already handled today.

-Chris.

> regards,
> Rémi
> 
> - Mail original -
>> De: "Chris Hegarty" <chris.hega...@oracle.com>
>> À: "jigsaw-dev" <jigsaw-dev@openjdk.java.net>
>> Envoyé: Jeudi 8 Décembre 2016 14:49:38
>> Objet: RFR [9] 8170859 : Run time and tool support for ModuleResolution
> 
>> This is a review request for the link time and run time support for
>> the mechanism to support incubator modules [1]. More specifically,
>> the addition of options to the jmod and jar tools to pass through the
>> relevant values, as per JEP 11, and support for these in the form
>> of a new ModuleResolution attribute ( that will be documented in
>> JEP 261 ).
>> 
>> http://cr.openjdk.java.net/~chegar/8170859/00/
>> 
>> -Chris.
>> 
>> [1] http://openjdk.java.net/jeps/11