RFR [9] Warning notice for types in Incubator Modules

Mon Jan 30 16:20:34 UTC 2017

Thanks Erik and Magnus,

Since this is already pushed, I propose to file a new issue to
push these cleanups.

diff --git a/make/Javadoc.gmk b/make/Javadoc.gmk
--- a/make/Javadoc.gmk
+++ b/make/Javadoc.gmk
@@ -314,7 +314,7 @@
    $1_INDEX_FILE := $$(JAVADOC_OUTPUTDIR)/$$($1_OUTPUT_DIRNAME)/index.html

    # Rule for actually running javadoc
-  $$($1_INDEX_FILE): $(BUILD_TOOLS_JDK) $$($1_VARDEPS_FILE) 
$$($1_PACKAGE_DEPS) $$($1_DEPS)
+  $$($1_INDEX_FILE): $$(BUILD_TOOLS_JDK) $$($1_VARDEPS_FILE) 
$$($1_PACKAGE_DEPS) $$($1_DEPS)
  	$$(call LogWarn, Generating Javadoc from $$(words $$($1_PACKAGES)) 
package(s) for $$($1_OUTPUT_DIRNAME))
  	$$(call MakeDir, $$(@D))
          ifneq ($$($1_PACKAGES_FILE), )
@@ -743,7 +743,7 @@

 
################################################################################

-docs-javadoc: $(BUILD_TOOLS_JDK) $(TARGETS)
+docs-javadoc: $(TARGETS)

  docs-copy: $(COPY_TARGETS)


-Chris.

On 30/01/17 11:29, Erik Joelsson wrote:
> Hello,
>
> On 2017-01-30 11:58, Chris Hegarty wrote:
>> Magnus,
>>
>> On 26/01/17 11:45, Magnus Ihse Bursie wrote:
>>> On 2017-01-25 13:51, Chris Hegarty wrote:
>>>>> On 24 Jan 2017, at 16:44, Erik Joelsson <erik.joelsson at oracle.com>
>>>>> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Build changes look good except for one thing. In Javadoc.gmk, the
>>>>> dependency on $(BUILD_TOOLS_JDK) needs to be set for
>>>>> $$($1_INDEX_FILE) (on line 317), where the actual recipes are
>>>>> created. Setting it on the phony docs-javadoc target will not help
>>>>> incremental builds.
>>>> Updated in place
>>>>   http://cr.openjdk.java.net/~chegar/incubator_taglet/
>>>
>>> It's not ideal that a top-level target has dependencies from the JDK
>>> repo, but since we have no or few pre-existing top-level tools, I
>>> understand if you hesitate to add a new framework for such tools. If we
>>> ever do introduce more such tools, I think we should move this along.
>>
>> Ok.
>>
>>> I think the new -taglet options to the DEFAULT_JAVADOC_OPTIONS variable.
>>> As  the documentation says, the DEFAULT_JAVADOC_TAGS is there to specify
>>> the ordering of tags in the output. Unless the -taglet options
>>> implicitly generates one or more -tag options in it's place,
>>
>> It's an inline tag, so by definition has no order.
>>
>>> that's the
>>> wrong place to put it. If it does, perhaps a comment indicating this
>>> hidden behavior would be in place.
>>
>> It seemed best to keep all tag related options together,
>> rather then spreading them across multiple variables.
>>
>>> In the line: "$$($1_INDEX_FILE): $(BUILD_TOOLS_JDK) $$($1_VARDEPS_FILE)
>>> $$($1_PACKAGE_DEPS) $$($1_DEPS)", please use a double $ ($$) for all
>>> variables. While technically not necessary in this particular case, it's
>>> misleading
>>
>> How so?
>>
>>> and we've had our share of bugs due to single $ in eval'd macros.
>>
>> Will this specific variable cause a problem, if so how?
>>
> No, this variable will not cause a problem, but we have sort of agreed
> (though I have been a bit slack about it) to be consistent with all
> variable references inside macro bodies that will be eval'd. The trouble
> starts if the value of such a variable contains something that make has
> a special interpretation for, like a ':' or a ',' or a '$'. Then the
> eval will re-interpret those and weird, very hard to debug errors, start
> appearing.
>>> In the line: "docs-javadoc: $(BUILD_TOOLS_JDK) $(TARGETS)", please
>>> remove the reference to BUILD_TOOLS_JDK. It is not needed.
>>
>> Why is this not needed.
>>
> So we have these targets:
>
> * docs-javadoc, which is a named phony target which we use as part of
> the external API for the Javadoc.gmk makefile.
> * For each call to the SetupJavadoc macro, we have $$($1_INDEX_FILE)
> (which evaluates to something like
> build/linux-x64/images/docs/api/index.html). This is a specific file
> being generated by a recipe. All of these are added to the TARGETS
> variable through which all of them are set as prerequisites to
> docs-javadoc.
> * The targets in $(BUILD_TOOLS_JDK) represents the files generated by
> building the tools. We add these files as prerequisites to each of the
> index files so that if the build tool has changed, the javadoc will get
> regenerated. This is what we want to achieve for proper incremental
> behavior.
>
> Building the tools has no inherent value in itself, as they are not part
> of the product, they are only needed to generate proper javadocs. As
> such, it is conceptually wrong (as well as redundant) to also add the
> tools as prerequisites to the docs-javadoc target, because this target
> symbolizes building all the javadoc. Each of those javadoc runs do
> require the tools however, so each of those javadoc runs need to have
> the tools as prerequisites.
>
> I had missed that you didn't remove that change in your updated review btw.
>
> /Erik
>
>> -Chris.
>>
>>> /Magnus
>>>
>>>
>>>>
>>>> -Chris.
>>>>
>>>>> /Erik
>>>>>
>>>>>
>>>>> On 2017-01-24 15:08, Chris Hegarty wrote:
>>>>>> As per the guidance in JEP 11 [1], the javadoc for types in
>>>>>> incubator modules should have a clear and explicit warning
>>>>>> notice. To that end, I’ve added a simple inline taglet that can
>>>>>> be used to effectively inject a standard notice, and applied it
>>>>>> to all public types in the HTTP module ( I’ll cleanup the line
>>>>>> width formatting before pushing ).
>>>>>>
>>>>>> http://cr.openjdk.java.net/~chegar/incubator_taglet/
>>>>>>
>>>>>> -Chris.
>>>>>>
>>>>>> P.S. this work is, somewhat, in preparation for when we move
>>>>>> to a single documentation bundle [2].
>>>>>>
>>>>>> [1] http://openjdk.java.net/jeps/11
>>>>>> [2] http://openjdk.java.net/jeps/299
>>>
>

