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

Severin Gehwolf sgehwolf at redhat.com
Thu Aug 22 08:47:34 UTC 2019


Hi Jiri,

On Wed, 2019-08-21 at 19:32 +0200, Jiri Vanek wrote:
> Hello!
> 
> http://cr.openjdk.java.net/~jvanek/8154313/ (https://bugs.openjdk.java.net/browse/JDK-8154313)
> 
> This is patch which was pushed (in slightly different form) to jdk9, and then was changing through
> every jdk until its form in jdk11 ad up.

Next time, please mention the modifications you had to make and why.
Makes a reviewer's life easier :)

make/Javadoc.gmk
----------------

FULL_VERSION changed to VERSION_STRING in JDK 9+, hence the change in
JAVADOC_ARCHIVE_NAME. Ok.

 222 PLATFORM_DOCSDIR = $(DOCSDIR)/platform
 223 
 224 
 225 JAVADOC_ARCHIVE_NAME := jdk-$(FULL_VERSION)-docs.zip
 226 JAVADOC_ARCHIVE_ASSEMBLY_DIR :=  $(DOCSTMPDIR)/zip-docs
 227 JAVADOC_ARCHIVE_DIR := $(OUTPUT_ROOT)/bundles
 228 JAVADOC_ARCHIVE := $(JAVADOC_ARCHIVE_DIR)/$(JAVADOC_ARCHIVE_NAME)

Please remove the extra empty line on line 224.

 352 #
 353 
 354 $(JAVADOC_ARCHIVE): $(COREAPI_INDEX_FILE)
 355         @$(ECHO) "Compressing javadoc to single $(JAVADOC_ARCHIVE_NAME)" ;

Shouldn't this be something like this? The JDK 9 patch logs this to log info:
http://hg.openjdk.java.net/jdk9/jdk9/rev/ec69c5bf68a6#l1.41

@$(ECHO) $(LOG_INFO) "Compressing javadoc to single $(JAVADOC_ARCHIVE_NAME)"

 356         $(MKDIR) -p $(JAVADOC_ARCHIVE_DIR) ;
 357         $(RM) -r $(JAVADOC_ARCHIVE_ASSEMBLY_DIR) ;
 358         $(MKDIR) -p $(JAVADOC_ARCHIVE_ASSEMBLY_DIR);
 359         all_roots=`$(FIND) $(DOCSDIR) | $(GREP) index.html `; \

Why is "grep -v old/doclet" missing from the JDK 8 patch?
http://hg.openjdk.java.net/jdk9/jdk9/rev/ec69c5bf68a6#l1.45

Are extra semicolons on lines 355-358 needed? It makes the patch
diverge from the JDK 9 version for no good reason...

 360         pushd $(JAVADOC_ARCHIVE_ASSEMBLY_DIR); \
 361         for index_file in $${all_roots} ; do \
 362           target_dir=`dirname $${index_file}`; \
 363           name=`$(ECHO) $${target_dir} | $(SED) "s;/spec;;" | $(SED) "s;.*/;;"`; \
 364           $(LN) -s $${target_dir}  $${name}; \
 365         done; \
 366         $(ZIP) -q -r $(JAVADOC_ARCHIVE) * ; \
 367         popd ;
 368

Adding new "zip-docs" target is missing from .PHONY in this patch. Please add it.
http://hg.openjdk.java.net/jdk9/jdk9/rev/ec69c5bf68a6#l1.58

make/Main.gmk
-------------

No comments.

Does "make clean-docs" work as expected? It appears it'll leave
bundles/jdk-*-docs.zip behind.

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

Thanks,
Severin



More information about the jdk8u-dev mailing list