RFR: JDK-8223355: Redundant output by javadoc

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

RFR: JDK-8223355: Redundant output by javadoc

Jonathan Gibbons-2
This change addresses a bug that was observed while playing with small demo API: some files may be generated twice.

The fix is to build a single set of the files to be generated.

Note: this bug also affected the generation of the main JDK API docs.  According to the log file, before the fix we were generating 14812 files; now, we are generating only 10146 files! That's a drop of 446 files or over 30%!

-------------

Commit messages:
 - JDK-8223355: Redundant output by javadoc

Changes: https://git.openjdk.java.net/jdk/pull/2652/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2652&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8223355
  Stats: 76 lines in 2 files changed: 73 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2652.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2652/head:pull/2652

PR: https://git.openjdk.java.net/jdk/pull/2652
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8223355: Redundant output by javadoc [v2]

Jonathan Gibbons-2
> This change addresses a bug that was observed while playing with small demo API: some files may be generated twice.
>
> The fix is to build a single set of the files to be generated.
>
> Note: this bug also affected the generation of the main JDK API docs.  According to the log file, before the fix we were generating 14812 files; now, we are generating only 10146 files! That's a drop of 4466 files or over 30%!

Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision:

  Add a default-on check that "chatty" lines in the output are unique
  (primarily about generating files)
 
  Remove a redundant misleading "Generating" message: it only showed the base filename and if it had shown the full name, it would have been a duplicate

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2652/files
  - new: https://git.openjdk.java.net/jdk/pull/2652/files/5a20e8e7..186993e7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2652&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2652&range=00-01

  Stats: 68 lines in 2 files changed: 55 ins; 9 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2652.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2652/head:pull/2652

PR: https://git.openjdk.java.net/jdk/pull/2652
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8223355: Redundant output by javadoc

liach
In reply to this post by Jonathan Gibbons-2
On Fri, 19 Feb 2021 18:51:44 GMT, Jonathan Gibbons <[hidden email]> wrote:

> This change addresses a bug that was observed while playing with small demo API: some files may be generated twice.
>
> The fix is to build a single set of the files to be generated.
>
> Note: this bug also affected the generation of the main JDK API docs.  According to the log file, before the fix we were generating 14812 files; now, we are generating only 10146 files! That's a drop of 4466 files or over 30%!

Just curious, does this bug affect users who specify all files individually or users who just specify a directory without detailing selection rules for classes or packages?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2652
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8223355: Redundant output by javadoc

Jonathan Gibbons-2
On Sat, 20 Feb 2021 01:44:45 GMT, liach <[hidden email]> wrote:

> Just curious, does this bug affect users who specify all files individually or users who just specify a directory without detailing selection rules for classes or packages?

Somewhat surprisingly, this happens in all the following cases:

* specify a module on the module source path (`--module-source-path` _path_ `--module` _m_)
* specify a module on the source path (`--source-path` _path_ `--module` _m_)
* specify files on the command line (`$(find` _path_ `-name *.java)`)

It does not occur when specifying a package on the source path:

* specify a module on the source path (`--source-path` _path_  _p_)

-------------

PR: https://git.openjdk.java.net/jdk/pull/2652
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8223355: Redundant output by javadoc [v2]

Hannes Wallnöfer
In reply to this post by Jonathan Gibbons-2
On Fri, 19 Feb 2021 22:11:04 GMT, Jonathan Gibbons <[hidden email]> wrote:

>> This change addresses a bug that was observed while playing with small demo API: some files may be generated twice.
>>
>> The fix is to build a single set of the files to be generated.
>>
>> Note: this bug also affected the generation of the main JDK API docs.  According to the log file, before the fix we were generating 14812 files; now, we are generating only 10146 files! That's a drop of 4466 files or over 30%!
>
> Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision:
>
>   Add a default-on check that "chatty" lines in the output are unique
>   (primarily about generating files)
>  
>   Remove a redundant misleading "Generating" message: it only showed the base filename and if it had shown the full name, it would have been a duplicate

That was quite the oversight. Kudos for spotting this!

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DocFilesHandlerImpl.java line 201:

> 199:
> 200:         HtmlDocletWriter docletWriter = new DocFileWriter(configuration, dfilePath, element, pkg);
> 201:         configuration.messages.notice("doclet.Generating_0", docletWriter.filename.getPath());

I guess this is to prevent duplicate output. It could probably have been solved by printing the HTML file's whole path (`docletWriter.filename` just contains the base name), or including the destination path. But I'm not convinced we need to generate a message for each and every file we copy, so it's probably fine as it is.

-------------

Marked as reviewed by hannesw (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2652
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8223355: Redundant output by javadoc [v2]

Hannes Wallnöfer
On Mon, 22 Feb 2021 09:12:17 GMT, Hannes Wallnöfer <[hidden email]> wrote:

>> Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Add a default-on check that "chatty" lines in the output are unique
>>   (primarily about generating files)
>>  
>>   Remove a redundant misleading "Generating" message: it only showed the base filename and if it had shown the full name, it would have been a duplicate
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DocFilesHandlerImpl.java line 201:
>
>> 199:
>> 200:         HtmlDocletWriter docletWriter = new DocFileWriter(configuration, dfilePath, element, pkg);
>> 201:         configuration.messages.notice("doclet.Generating_0", docletWriter.filename.getPath());
>
> I guess this is to prevent duplicate output. It could probably have been solved by printing the HTML file's whole path (`docletWriter.filename` just contains the base name), or including the destination path. But I'm not convinced we need to generate a message for each and every file we copy, so it's probably fine as it is.

Please ignore that comment, I just saw there's already a message with the full path.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2652
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8223355: Redundant output by javadoc [v2]

Jonathan Gibbons-2
On Mon, 22 Feb 2021 09:14:48 GMT, Hannes Wallnöfer <[hidden email]> wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DocFilesHandlerImpl.java line 201:
>>
>>> 199:
>>> 200:         HtmlDocletWriter docletWriter = new DocFileWriter(configuration, dfilePath, element, pkg);
>>> 201:         configuration.messages.notice("doclet.Generating_0", docletWriter.filename.getPath());
>>
>> I guess this is to prevent duplicate output. It could probably have been solved by printing the HTML file's whole path (`docletWriter.filename` just contains the base name), or including the destination path. But I'm not convinced we need to generate a message for each and every file we copy, so it's probably fine as it is.
>
> Please ignore that comment, I just saw there's already a message with the full path.

FWIW, based on other recent experiences, I think there is the potential to have more versatile control of which messages to suppress. `-quiet` just does everything. But this is another discussion.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2652
Reply | Threaded
Open this post in threaded view
|

Integrated: JDK-8223355: Redundant output by javadoc

Jonathan Gibbons-2
In reply to this post by Jonathan Gibbons-2
On Fri, 19 Feb 2021 18:51:44 GMT, Jonathan Gibbons <[hidden email]> wrote:

> This change addresses a bug that was observed while playing with small demo API: some files may be generated twice.
>
> The fix is to build a single set of the files to be generated.
>
> Note: this bug also affected the generation of the main JDK API docs.  According to the log file, before the fix we were generating 14812 files; now, we are generating only 10146 files! That's a drop of 4466 files or over 30%!

This pull request has now been integrated.

Changeset: 53b15453
Author:    Jonathan Gibbons <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/53b15453
Stats:     144 lines in 4 files changed: 128 ins; 9 del; 7 mod

8223355: Redundant output by javadoc

Reviewed-by: hannesw

-------------

PR: https://git.openjdk.java.net/jdk/pull/2652