RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Magnus Ihse Bursie
When the build system tries to figure out which modules that should be
included by the javadoc build, it locates the set of modules "required"
by already included modules, starting from an initial set and repeating
recursively (a method which we unfortunately called "transitive").

However, for javadoc, we should only look at those re-exported modules
that are classified by a "requires transitive" relationship (which I
therefore have called "re-exported" to not clash with the established
term "transitive").

The effect of getting this wrong is e.g. that we copy module graph png
files that we should not, and that we look at too broad a set of files
for looking at changes that should trigger a re-build of the javadoc.

Bug: https://bugs.openjdk.java.net/browse/JDK-8180480
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.01

/Magnus
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Erik Joelsson
Looks good.

/Erik


On 2017-05-18 09:54, Magnus Ihse Bursie wrote:

> When the build system tries to figure out which modules that should be
> included by the javadoc build, it locates the set of modules
> "required" by already included modules, starting from an initial set
> and repeating recursively (a method which we unfortunately called
> "transitive").
>
> However, for javadoc, we should only look at those re-exported modules
> that are classified by a "requires transitive" relationship (which I
> therefore have called "re-exported" to not clash with the established
> term "transitive").
>
> The effect of getting this wrong is e.g. that we copy module graph png
> files that we should not, and that we look at too broad a set of files
> for looking at changes that should trigger a re-build of the javadoc.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8180480
> WebRev:
> http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.01
>
> /Magnus

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Mandy Chung
In reply to this post by Magnus Ihse Bursie

> On May 18, 2017, at 12:54 AM, Magnus Ihse Bursie <[hidden email]> wrote:
>
> When the build system tries to figure out which modules that should be included by the javadoc build, it locates the set of modules "required" by already included modules, starting from an initial set and repeating recursively (a method which we unfortunately called "transitive”).

Which module has this name clash?

> However, for javadoc, we should only look at those re-exported modules that are classified by a "requires transitive" relationship (which I therefore have called "re-exported" to not clash with the established term "transitive”).

Javadoc uses the “Indirect Exports” term for the exported API packages are from the “requires transitive” modules.

I suggest to rename “ReExported” to “IndirectRequires” or “Indirect”.

Mandy

> The effect of getting this wrong is e.g. that we copy module graph png files that we should not, and that we look at too broad a set of files for looking at changes that should trigger a re-build of the javadoc.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8180480
> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.01
>
> /Magnus

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Jonathan Gibbons


On 5/18/17 8:30 AM, Mandy Chung wrote:

>> On May 18, 2017, at 12:54 AM, Magnus Ihse Bursie <[hidden email]> wrote:
>>
>> When the build system tries to figure out which modules that should be included by the javadoc build, it locates the set of modules "required" by already included modules, starting from an initial set and repeating recursively (a method which we unfortunately called "transitive”).
> Which module has this name clash?
>
>> However, for javadoc, we should only look at those re-exported modules that are classified by a "requires transitive" relationship (which I therefore have called "re-exported" to not clash with the established term "transitive”).
> Javadoc uses the “Indirect Exports” term for the exported API packages are from the “requires transitive” modules.
>
> I suggest to rename “ReExported” to “IndirectRequires” or “Indirect”.
>
> Mandy
Agreed, re-exports is not the correct terminology here.
>
>> The effect of getting this wrong is e.g. that we copy module graph png files that we should not, and that we look at too broad a set of files for looking at changes that should trigger a re-build of the javadoc.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180480
>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.01
>>
>> /Magnus

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Magnus Ihse Bursie
In reply to this post by Mandy Chung


On 2017-05-18 17:30, Mandy Chung wrote:
>> On May 18, 2017, at 12:54 AM, Magnus Ihse Bursie <[hidden email]> wrote:
>>
>> When the build system tries to figure out which modules that should be included by the javadoc build, it locates the set of modules "required" by already included modules, starting from an initial set and repeating recursively (a method which we unfortunately called "transitive”).
> Which module has this name clash?
>
>> However, for javadoc, we should only look at those re-exported modules that are classified by a "requires transitive" relationship (which I therefore have called "re-exported" to not clash with the established term "transitive”).
> Javadoc uses the “Indirect Exports” term for the exported API packages are from the “requires transitive” modules.

"Indirect exports" was a good term. I dropped the "Deps" part of the
name and it all became much clearer.

Here's an updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.02

/Magnus

>
> I suggest to rename “ReExported” to “IndirectRequires” or “Indirect”.
>
> Mandy
>
>> The effect of getting this wrong is e.g. that we copy module graph png files that we should not, and that we look at too broad a set of files for looking at changes that should trigger a re-build of the javadoc.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180480
>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.01
>>
>> /Magnus

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Erik Joelsson
Looks good.

/Erik


On 2017-05-19 10:08, Magnus Ihse Bursie wrote:

>
>
> On 2017-05-18 17:30, Mandy Chung wrote:
>>> On May 18, 2017, at 12:54 AM, Magnus Ihse Bursie
>>> <[hidden email]> wrote:
>>>
>>> When the build system tries to figure out which modules that should
>>> be included by the javadoc build, it locates the set of modules
>>> "required" by already included modules, starting from an initial set
>>> and repeating recursively (a method which we unfortunately called
>>> "transitive”).
>> Which module has this name clash?
>>
>>> However, for javadoc, we should only look at those re-exported
>>> modules that are classified by a "requires transitive" relationship
>>> (which I therefore have called "re-exported" to not clash with the
>>> established term "transitive”).
>> Javadoc uses the “Indirect Exports” term for the exported API
>> packages are from the “requires transitive” modules.
>
> "Indirect exports" was a good term. I dropped the "Deps" part of the
> name and it all became much clearer.
>
> Here's an updated webrev:
> http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.02 
>
>
> /Magnus
>
>>
>> I suggest to rename “ReExported” to “IndirectRequires” or “Indirect”.
>>
>> Mandy
>>
>>> The effect of getting this wrong is e.g. that we copy module graph
>>> png files that we should not, and that we look at too broad a set of
>>> files for looking at changes that should trigger a re-build of the
>>> javadoc.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180480
>>> WebRev:
>>> http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.01
>>>
>>> /Magnus
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Mandy Chung
In reply to this post by Magnus Ihse Bursie

> On May 19, 2017, at 1:08 AM, Magnus Ihse Bursie <[hidden email]> wrote:
>
>
> "Indirect exports" was a good term. I dropped the "Deps" part of the name and it all became much clearer.
>
> Here's an updated webrev:
> http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.02
>

+  $1_INDIRECT_EXPORTS := $$(call FindTransitiveIndirectExportsForModules, $$($1_MODULES))

Nit: “exported” term is for API but not for dependency.  

s/$1_INDIRECT_EXPORTS/$1_TRANSITIVE_MODULES
s/FindTransitiveIndirectExportsForModules/FindTransitiveIndirectDepsForModules

Otherwise, looks good.  No need for a new webrev:

Mandy
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180480 Use "requires transitive" relationship when determining modules for javadoc

Magnus Ihse Bursie
On 2017-05-19 16:47, Mandy Chung wrote:

>> On May 19, 2017, at 1:08 AM, Magnus Ihse Bursie <[hidden email]> wrote:
>>
>>
>> "Indirect exports" was a good term. I dropped the "Deps" part of the name and it all became much clearer.
>>
>> Here's an updated webrev:
>> http://cr.openjdk.java.net/~ihse/JDK-8180480-docs-should-use-requires-transitive/webrev.02
>>
> +  $1_INDIRECT_EXPORTS := $$(call FindTransitiveIndirectExportsForModules, $$($1_MODULES))
>
> Nit: “exported” term is for API but not for dependency.
>
> s/$1_INDIRECT_EXPORTS/$1_TRANSITIVE_MODULES
> s/FindTransitiveIndirectExportsForModules/FindTransitiveIndirectDepsForModules
>
> Otherwise, looks good.  No need for a new webrev:

Well, but that's not really just a "nit", is it? The problem here was
that "transitive deps" had been used in the names of our macros in a
different way than "requires transitive" from module-info.java implies,
and now we're just back in that confusion anyway.

Oh well. I'll settle for those names (but under some protest!) for jdk
9, and then we'll have to make sure we get a better naming scheme that
aligns all the way for jdk 10.

/Magnus

>
> Mandy