[8u] RFR Backport JDK-8154313, Generated javadoc scattered all over the place

[Jiri Vanek]

On 9/2/19 6:16 PM, Severin Gehwolf wrote:
> Hi,
> 
> On Thu, 2019-08-29 at 12:31 +0200, Jiri Vanek wrote:
>> Updated:
>>>> http://cr.openjdk.java.net/~jvanek/8154313/ (
>>>> https://bugs.openjdk.java.net/browse/JDK-8154313)
>> http://cr.openjdk.java.net/~jvanek/8154313/webrev/
> 
> Thanks for the update. I should note that this backport introduces the
> new target as 'docs-zip' right away instead of the original name of
> 'zip-docs' in JDK 9 which then got renamed to 'docs-zip' via JDK-
> 8168772. Not sure if that was already the case for v1 of the webrev.

Yes. I Intentionally introduced the jdk11 name, rather then jdk9 one.  I strongly suggest to keep
the target aligned with jdk11. I would say, that the target was not even solidly noticed in jdk9/10
> 
>>>
>>> Does "make clean-docs" work as expected? It appears it'll leave
>>> bundles/jdk-*-docs.zip behind.
>>
>> No. And never was. I had added
>> clean-docs-zip target now.
> 
> What do you mean by "And never was."? Are you suggesting it doesn't
> work on JDK 9+? If so JDK-8168772 seems a likely culprit. Anyway...

sorry for not being clear. The clean was not working in original my original backport. It is of
course working now, and in jdk11.
> 
>>> Perhaps add a condition for "docs" component in CleanComponent here?
>>> http://hg.openjdk.java.net/jdk8u/jdk8u-dev/file/cd2e5f820a0d/make/MakeHelpers.gmk#l299
>>>
>>> Then it would work similar to what the JDK 9 patch did with:
>>> http://hg.openjdk.java.net/jdk9/jdk9/rev/ec69c5bf68a6#l3.7
>>>
>>
>> Thank you verey  much for guiding review!
>>
> 
> make/Main.gmk:
> 
>  202 # Remove everything, except the output from configure.
>  203 clean: clean-langtools clean-corba clean-jaxp clean-jaxws clean-hotspot clean-jdk clean-nashorn clean-images clean-overlay-images clean-bootcycle-build clean-docs clean-docs-zip clean-test
> 
> You are adding clean-docs-zip to the default "clean" target. Yet, "make
> docs" doesn't seem to build docs-zip. That seems inconsistent. I'd
> require an explicit "make clean-docs-zip" call. I.e. don't add it to
> the list of dep-targets for "make clean".

Isn't maybe better to add docs-zip to default docs target?
> 
>  257 .PHONY: langtools corba jaxp jaxws hotspot jdk nashorn images overlay-images install test docs docs-zip docs-zip-only
>  258 .PHONY: langtools-only corba-only jaxp-only jaxws-only hotspot-only jdk-only nashorn-only images-only overlay-images-only install-only test-only docs-only
> 
> Style: It seems all 'foo-only' targets are on one line, and 'foo' on
> another. Please move the docs-zip-only addition to line 258.

I see. will be fixed.
> 
> Looks OK otherwise.
> 
> Thanks,
> Severin
> 


-- 
Jiri Vanek
Senior QE engineer, OpenJDK QE lead, Mgr.
Red Hat Czech
jvanek at redhat.com    M: +420775390109