Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

[Magnus Ihse Bursie]

On 2017-03-29 13:45, Magnus Ihse Bursie wrote:
> On 2017-03-25 22:33, Mandy Chung wrote:
>> I edited the module descriptions per your feedback.  I also revised
>> GenGraphs tool to take a properties file to customize the dot graphs
>> for javadoc use.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/
> I just discovered a problem with this patch when I tried to apply it 
> locally.
>
> With this patch, it is not possible to run "make docs-javadoc" in a 
> clean build. The problem is that ModuleTools.gmk is broken, and 
> overrides the value of BUILD_TOOLS_JDK from Tools.gmk.

Suggested patch, which solves the problem:

diff --git a/make/ModuleTools.gmk b/make/ModuleTools.gmk
--- a/make/ModuleTools.gmk
+++ b/make/ModuleTools.gmk
@@ -1,5 +1,5 @@
  #
-# Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights 
reserved.
+# Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights 
reserved.
  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  #
  # This code is free software; you can redistribute it and/or modify it
@@ -23,8 +23,9 @@
  # questions.
  #

-include $(SPEC)
-include MakeBase.gmk
+ifndef _MODULE_TOOLS_GMK
+_MODULE_TOOLS_GMK := 1
+
  include JavaCompilation.gmk

  TOOLS_CLASSES_DIR := $(BUILDTOOLS_OUTPUTDIR)/tools_jigsaw_classes
@@ -32,7 +33,7 @@
  # To avoid reevaluating the compilation setup for the tools each time 
this file
  # is included, the actual compilation is handled by 
CompileModuleTools.gmk. The
  # following trick is used to be able to declare a dependency on the 
built tools.
-BUILD_TOOLS_JDK := $(call SetupJavaCompilationCompileTarget, \
+BUILD_JIGSAW_TOOLS := $(call SetupJavaCompilationCompileTarget, \
      BUILD_JIGSAW_TOOLS, $(TOOLS_CLASSES_DIR))

  TOOL_GENGRAPHS := $(BUILD_JAVA) -esa -ea -cp $(TOOLS_CLASSES_DIR) \
@@ -47,3 +48,5 @@
      -cp $(TOOLS_CLASSES_DIR) \
      --add-exports java.base/jdk.internal.module=ALL-UNNAMED \
      build.tools.jigsaw.AddPackagesAttribute
+
+endif # _MODULE_TOOLS_GMK

/Magnus
>
> /Magnus
>
>> Magnus, Erik,
>>
>> I modified Javadoc.gmk and Main.gmk to add a new target to generate
>> .dot files for javadoc use.  GenGraphs tool depends on the exploded
>> image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
>> variable for us to enable @moduleGraph taglet until JDK-8176785 is
>> resolved.
>>
>> thanks
>> Mandy
>>
>>> On Mar 25, 2017, at 2:36 AM, Alan Bateman <Alan.Bateman at oracle.com> 
>>> wrote:
>>>
>>> On 24/03/2017 21:50, Mandy Chung wrote:
>>>
>>>> Alan,
>>>>
>>>> I took another round of edits on the module descriptions:
>>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/
>>>>
>>>> Once we have the unified docs, it will make it easier to review
>>>> the module summary page for all modules where we will revise
>>>> these module descriptions again.
>>>>
>>> I went through the updated module descriptions.
>>>
>>> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I 
>>> think they will read okay once that is added.
>>>
>> Fixed. I missed that.
>>
>>> jdk.httpserver currently has "Defines the JDK-specific API for HTTP 
>>> server", it might be better to re-shuffle this to "Defines the API 
>>> for the JDK-specific HTTP server”.
>>>
>> This reads better.
>>
>>> I think the only one that needs re-examination is jdk.charsets. The 
>>> java.base module contains the standard charsets and all other 
>>> charsets needed to start the runtime on any of the supported 
>>> configurations. It thus varies by platform with jdk.charsets 
>>> providing the charsets that aren't in java.base. Finding the right 
>>> description is difficult, maybe we should start with "Charset 
>>> provider for the charsets that are not in java.base (mostly double 
>>> byte and IBM charsets". I could imagine linking this to the 
>>> "Supported encodings" docs page in time.
>>>
>> Let’s start with this version.  I expect we will refine the module
>> description further next couple weeks.
>>
>>> Everything else looks good.
>>
>> Thanks
>> Mandy
>