Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v3]

2024-01-18 Thread Sam James
> The LFS64 symbols provided by glibc are not part of any standard and were 
> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). 
> This commit replaces the usage of LFS64 symbols with their regular 
> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will 
> always act as their -64 variants on glibc.

Sam James has updated the pull request incrementally with one additional commit 
since the last revision:

  Add message for assert
  
  Not all C++ stds implement it w/o.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16329/files
  - new: https://git.openjdk.org/jdk/pull/16329/files/4df24d81..e02b0715

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=16329=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=16329=01-02

  Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/16329.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16329/head:pull/16329

PR: https://git.openjdk.org/jdk/pull/16329


Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v2]

2024-01-18 Thread David Holmes
On Fri, 19 Jan 2024 06:08:21 GMT, Sam James  wrote:

>> The LFS64 symbols provided by glibc are not part of any standard and were 
>> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). 
>> This commit replaces the usage of LFS64 symbols with their regular 
>> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions 
>> will always act as their -64 variants on glibc.
>
> Sam James has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add off_t static_asserts
>   
>   Signed-off-by: Sam James 

Thanks, update seems fine.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16329#pullrequestreview-1831637139


Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v2]

2024-01-18 Thread Sam James
> The LFS64 symbols provided by glibc are not part of any standard and were 
> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). 
> This commit replaces the usage of LFS64 symbols with their regular 
> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will 
> always act as their -64 variants on glibc.

Sam James has updated the pull request incrementally with one additional commit 
since the last revision:

  Add off_t static_asserts
  
  Signed-off-by: Sam James 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16329/files
  - new: https://git.openjdk.org/jdk/pull/16329/files/4432b9d8..4df24d81

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=16329=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=16329=00-01

  Stats: 5 lines in 3 files changed: 5 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/16329.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16329/head:pull/16329

PR: https://git.openjdk.org/jdk/pull/16329


Re: RFR: 8318696: Do not use LFS64 symbols on Linux

2024-01-18 Thread David Holmes
On Sat, 16 Dec 2023 06:38:38 GMT, Sam James  wrote:

> My preference for moving forward is:
>
> 1.  I add some static_assert for off_t to the modified JVM bits to be safe 
> (that's what I tested with);
> 2.  We keep this PR for the build-only fixes which are semantically identical 
> to the previous code - this PR currently "preserves" bitness, it doesn't fix 
> anything other than avoiding use of foo64() where it's unnecessary by 
> converting to foo, but the foo64() use was right - just a glibcism - until 
> now. From what I can tell, the JVM code touched here was correct, just 
> relying on glibcisms;
> 3. In another PR, we look at the general 32-bit LFS situation which may 
> involve runtime fixes if - as it appears - off_t is actually 32-bit on x86 
> systems right now and non foo64() functions are being used. Then we either 
> port more stuff to use foo64() (not ideal), or consistently use foo() in more 
> places with FILE_OFFSET_BITS.

I'm unclear how the current PR is expected to proceed under this plan. Does # 2 
also keep the source code changes in this PR? Should the asserts from # 1 also 
be added to this PR?

This issue is now hitting hard on Alpine Linux 3.19 - see 
[JDK-8324153](https://bugs.openjdk.org/browse/JDK-8324153)

-

PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1899810531


Re: RFR: 8318696: Do not use LFS64 symbols on Linux

2024-01-18 Thread Sam James
On Tue, 24 Oct 2023 01:36:32 GMT, Sam James  wrote:

> The LFS64 symbols provided by glibc are not part of any standard and were 
> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). 
> This commit replaces the usage of LFS64 symbols with their regular 
> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will 
> always act as their -64 variants on glibc.

Ah, sorry, I thought I'd pushed those asserts ;)

Yes, this PR would be identical under the plan other than adding the asserts I 
promised and forgot about.

Let me look now.

-

PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1899812128


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v7]

2024-01-18 Thread Julian Waters
On Thu, 18 Jan 2024 16:21:46 GMT, Julian Waters  wrote:

>> [JDK-8323008](https://bugs.openjdk.org/browse/JDK-8323008) reports unwanted 
>> autoconf flags being added to the command line, and solves the issue by 
>> filtering out the added flags by force. This is not optimal however, as 
>> doing so leaves the autoconf message that the flags were added still 
>> displaying during the configure process, which is confusing. Instead, 
>> setting a couple of fields to disable the autoconf internals responsible for 
>> the unwanted flag is a cleaner solution
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Implement custom AC_PROG_CC and AC_PROG_CXX in util.m4

I found out that the issue of having AC_PROG_CC and AC_PROG_CXX being called by 
AC_REQUIRE can be solved by directly overwriting them in util.m4, rather than 
providing our own UTIL_PROG_CC and so on. Unsure how satisfactory this is, but 
I have here a stripped down version of both that omits:

For AC_PROG_CC
- The default check from a list of compilers if the compilers to search for 
list is empty, this now means AC_PROG_CC has to always be called with the 
argument at all times. We always do this, so shouldn't be a problem
- The check whether the compiler supports GNU C. This shouldn't be relevant
- The ill fated check that adds -std=gnu11. This was the one that the other 
issue wanted to get rid of

For AC_PROG_CXX
- Similarly, the default check from a list of compilers if the compilers to 
search for list is empty, and the check for CCC if it is set (We don't use CCC 
as fair as I can tell), this now means AC_PROG_CXX has to always be called with 
the argument at all times. We again always do this, so shouldn't be a problem
- The check whether the compiler supports GNU C++. This shouldn't be relevant
- The ill fated check that adds -std=gnu++11. This is also the one that the 
other issue wanted to get rid of

Notably, I left the check that checks if the compiler accepts -g in there, as I 
was unsure if we need it or not. I also tried removing the check for the object 
file extension, but apparently autoconf needs that to function properly. The 
exe file extension check is misleadingly named, because in actuality all of 
AC_PROG_CC and AC_PROG_CXX's functionality is in it,and it's also likely 
required for autoconf to work properly

-

PR Comment: https://git.openjdk.org/jdk/pull/17401#issuecomment-1899483894


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v4]

2024-01-18 Thread Julian Waters
Well, there's a certain individual that can help out if need be ;)

best regards,
Julian

On Fri, Jan 19, 2024 at 6:46 AM Magnus Ihse Bursie
 wrote:
>
> On 2024-01-18 10:44, Julian Waters wrote:
>
> If I understand you correctly, this new hypothetical system would replace 
> autoconf as such?
>
> configure -> autoconf -> compiled configure -> make
> to
> configure -> compile Java -> Java configure program -> make
>
> Yep, something like that. With a thin shell wrapper (think gradlew) to launch 
> `java Configure` or something like that. If the progress with multi-file java 
> launching continues properly, we might even be able to skip the compile step.
>
> I have a prototype sketch I've been working on from time to time. This 
> discussion actually inspired me to go back to it and see if I can convert 
> into a more proper proof-of-concept.
>
> /Magnus


Re: CFV: New Build Group Member: Christoph Langer

2024-01-18 Thread Julian Waters
Vote: yes

best regards,
Julian


Re: RFR: 8323675: Race in jdk.javadoc-gendata [v2]

2024-01-18 Thread Erik Joelsson
On Thu, 18 Jan 2024 13:54:23 GMT, Erik Joelsson  wrote:

>> @lahodaj Just to be absolutely clear: In `jdk.javadoc-gendata`, we're 
>> calling two tools: Not only `JavadocElementList` (which only requires 
>> source, not class files), but also `CreateSymbols build-javadoc-data`. Can 
>> you confirm that this too only requires sources?
>> 
>> (I take it that this usage is different from `CreateSymbols build-ctsym` as 
>> called in `jdk.compiler-gendata`.)
>
> Looking at `make/modules/jdk.javadoc/Gendata.gmk` the recipe never references 
> `$(JDK_OUTPUTDIR)/modules/`, the output dir where the compiled classes are 
> located. It only references the output of `$(call GetModuleSrcPath)` (and 
> some static src dirs), so I'm pretty confident that this is correct.

Somewhat related to this, while investigating another bug and with this PR 
fresh in memory, I think we are missing another dependency. 

In `make/modules/jdk.javadoc/Gendata.gmk`, we are copying the generated files 
into `jdk.javadoc.interim` (part of interim langtools). This makes me believe 
that the generated files are needed when we use `NEW_JAVADOC` from interim 
langtools to build the API docs for the JDK. However, there is no dependency 
declared from any of the `docs-*-api-javadoc` targets to `jdk.javadoc-gendata`, 
so if this works today, it's just by luck.

I see two possible outcomes:

1. If the generated files from `jdk.javadoc-gendata` are expected to be present 
and used when generating the main API docs for the JDK, then we need to add 
dependencies for that.
2. If the generated files aren't actually needed, we should stop copying them 
to avoid unnecessary work and non deterministic behavior.

Not sure if we should hijack this PR for this problem, probably better to file 
a separate issue. @lahodaj what do you think about this?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17402#discussion_r1458067165


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v4]

2024-01-18 Thread Magnus Ihse Bursie

On 2024-01-18 10:44, Julian Waters wrote:

If I understand you correctly, this new hypothetical system would 
replace autoconf as such?

configure -> autoconf -> compiled configure -> make
to
configure -> compile Java -> Java configure program -> make


Yep, something like that. With a thin shell wrapper (think gradlew) to 
launch `java Configure` or something like that. If the progress with 
multi-file java launching continues properly, we might even be able to 
skip the compile step.


I have a prototype sketch I've been working on from time to time. This 
discussion actually inspired me to go back to it and see if I can 
convert into a more proper proof-of-concept.


/Magnus


Re: CFV: New Build Group Member: Christoph Langer

2024-01-18 Thread Magnus Ihse Bursie

Vote: yes

/Magnus

On 2024-01-18 17:46, Baesken, Matthias wrote:


I hereby nominate Christoph Langer (clanger) to Membership in the 
Build Group.


Christoph is a long standing member of the JVM team at SAP and a long 
time OpenJDK contributor [3].


He is a member of the OpenJDK Members Group and the Vulnerability 
Group, and Reviewer in a number of projects.


Christoph has contributed over 100 openjdk/jdk changes [4], among 
those also a number of build related changes.


What is especially important is the fact that Christoph has 
build/production related expertise on a lot of different OS platforms


(macOS, Windows, Linux, AIX).

Votes are due by 1st February 2024, 13:00 CET.

Only current Members of the Build Group [1] are eligible

to vote on this nomination.  Votes must be cast in the open by

replying to this mailing list

For Lazy Consensus voting instructions, see [2].

Matthias Baesken

[1] https://openjdk.org/census#build

[2] https://openjdk.org/groups/#member-vote

[3] https://openjdk.org/census#clanger

[4] 
https://github.com/search?q=repo%3Aopenjdk%2Fjdk+author-name%3A%22Christoph+Langer%22=commits 



Integrated: JDK-8320458: Improve structural navigation in API documentation

2024-01-18 Thread Hannes Wallnöfer
On Mon, 11 Dec 2023 16:36:09 GMT, Hannes Wallnöfer  wrote:

> This is a rather big change to update the structural navigation in API 
> documentation generated by JavaDoc. It adds a table of contents for the 
> current page to module, package, and class documentation, and replaces the 
> old sub-navigation bar with a breadcrumb-style links in those pages. The 
> table of contents is displayed as sidebar on wide/desktop displays and 
> integrated into the collapsible menu for narrow/mobile displays. The 
> generated docs can be browsed here:
> 
> https://cr.openjdk.org/~hannesw/8320458/api.00/
> 
> This change includes improvements in `stylesheet.css` and `script.js` that 
> are not strictly related to the navigation changes, but happened as a 
> consequence of the necessary restructuring. All these are described together 
> with the more on-topic changes in the list below.
> 
>  - The table of contents is composed as the respective writers generate the 
> pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of 
> new type `ListBuilder`. If the field is not `null` it is used to build the 
> table of contents as the page is built using either one of the  
> new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` 
> directly.
>  - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate 
> the markup for the sidebar and it is added to the page via the 
> `BodyContents.setSideContent` method.
>  - Both existing navigation bars (top and sub-navigation) get an additional 
> `` container with CSS class `nav-content` that uses a flex layout for 
> its content. This also handles vertical positioning, so the old workaround 
> for vertical of the language version label in `Docs.gmk` is not necessary 
> anymore.
>  - Apart from modules, packages, and classes, other pages that were converted 
> to obtain a table of contents are the "Constant Field Values" page and the 
> Help page. 
>  - Originally, I used the `` element for the sidebar, but I learned 
> that this was the wrong element as it is meant for content that is not 
> strictly related to the main content of the page. The prevailing notion seems 
> to be that a table of contents is a navigation element and therefore should 
> use the `` element, so I used that for the TOC sidebar. The same applies 
> for the breadcrumbs sub-navigation, so I left the header `` element 
> wrapped around both top and sub-navigation.
>  - For the new lists in TOC and breadcrumbs I used ordered list elements 
> `` instead of unordered `` we use everywhere else, as that is what 
> should be used when list order is important...

This pull request has now been integrated.

Changeset: 81df265e
Author:Hannes Wallnöfer 
URL:   
https://git.openjdk.org/jdk/commit/81df265e41d393cdde87729e091dd465934071fd
Stats: 3104 lines in 71 files changed: 1670 ins; 873 del; 561 mod

8320458: Improve structural navigation in API documentation

Reviewed-by: erikj, jjg

-

PR: https://git.openjdk.org/jdk/pull/17062


Re: CFV: New Build Group Member: Christoph Langer

2024-01-18 Thread erik . joelsson

Vote: yes

/Erik

On 1/18/24 08:46, Baesken, Matthias wrote:


I hereby nominate Christoph Langer (clanger) to Membership in the 
Build Group.


Christoph is a long standing member of the JVM team at SAP and a long 
time OpenJDK contributor [3].


He is a member of the OpenJDK Members Group and the Vulnerability 
Group, and Reviewer in a number of projects.


Christoph has contributed over 100 openjdk/jdk changes [4], among 
those also a number of build related changes.


What is especially important is the fact that Christoph has 
build/production related expertise on a lot of different OS platforms


(macOS, Windows, Linux, AIX).

Votes are due by 1st February 2024, 13:00 CET.

Only current Members of the Build Group [1] are eligible

to vote on this nomination.  Votes must be cast in the open by

replying to this mailing list

For Lazy Consensus voting instructions, see [2].

Matthias Baesken

[1] https://openjdk.org/census#build

[2] https://openjdk.org/groups/#member-vote

[3] https://openjdk.org/census#clanger

[4] 
https://github.com/search?q=repo%3Aopenjdk%2Fjdk+author-name%3A%22Christoph+Langer%22=commits 



CFV: New Build Group Member: Christoph Langer

2024-01-18 Thread Baesken, Matthias

I hereby nominate Christoph Langer (clanger) to Membership in the Build Group.

Christoph is a long standing member of the JVM team at SAP and a long time 
OpenJDK contributor [3].
He is a member of the OpenJDK Members Group and the Vulnerability Group, and 
Reviewer in a number of projects.
Christoph has contributed over 100 openjdk/jdk changes [4], among those also a 
number of build related changes.
What is especially important is the fact that Christoph has build/production 
related expertise on a lot of different OS platforms
(macOS, Windows, Linux, AIX).

Votes are due by 1st February 2024, 13:00 CET.

Only current Members of the Build Group [1] are eligible
to vote on this nomination.  Votes must be cast in the open by
replying to this mailing list

For Lazy Consensus voting instructions, see [2].

Matthias Baesken

[1] https://openjdk.org/census#build
[2] https://openjdk.org/groups/#member-vote
[3] https://openjdk.org/census#clanger
[4] 
https://github.com/search?q=repo%3Aopenjdk%2Fjdk+author-name%3A%22Christoph+Langer%22=commits


Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v6]

2024-01-18 Thread Jonathan Gibbons
On Thu, 18 Jan 2024 16:25:49 GMT, Hannes Wallnöfer  wrote:

>> This is a rather big change to update the structural navigation in API 
>> documentation generated by JavaDoc. It adds a table of contents for the 
>> current page to module, package, and class documentation, and replaces the 
>> old sub-navigation bar with a breadcrumb-style links in those pages. The 
>> table of contents is displayed as sidebar on wide/desktop displays and 
>> integrated into the collapsible menu for narrow/mobile displays. The 
>> generated docs can be browsed here:
>> 
>> https://cr.openjdk.org/~hannesw/8320458/api.00/
>> 
>> This change includes improvements in `stylesheet.css` and `script.js` that 
>> are not strictly related to the navigation changes, but happened as a 
>> consequence of the necessary restructuring. All these are described together 
>> with the more on-topic changes in the list below.
>> 
>>  - The table of contents is composed as the respective writers generate the 
>> pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of 
>> new type `ListBuilder`. If the field is not `null` it is used to build the 
>> table of contents as the page is built using either one of the  
>> new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` 
>> directly.
>>  - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate 
>> the markup for the sidebar and it is added to the page via the 
>> `BodyContents.setSideContent` method.
>>  - Both existing navigation bars (top and sub-navigation) get an additional 
>> `` container with CSS class `nav-content` that uses a flex layout for 
>> its content. This also handles vertical positioning, so the old workaround 
>> for vertical of the language version label in `Docs.gmk` is not necessary 
>> anymore.
>>  - Apart from modules, packages, and classes, other pages that were 
>> converted to obtain a table of contents are the "Constant Field Values" page 
>> and the Help page. 
>>  - Originally, I used the `` element for the sidebar, but I learned 
>> that this was the wrong element as it is meant for content that is not 
>> strictly related to the main content of the page. The prevailing notion 
>> seems to be that a table of contents is a navigation element and therefore 
>> should use the `` element, so I used that for the TOC sidebar. The same 
>> applies for the breadcrumbs sub-navigation, so I left the header `` 
>> element wrapped around both top and sub-navigation.
>>  - For the new lists in TOC and breadcrumbs I used ordered list elements 
>> `` instead of unordered `` we use everywhere else, as that is what 
>> should be used when...
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Initialize tableOfContents field in HtmlDoclet constructor

Nice.

-

Marked as reviewed by jjg (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17062#pullrequestreview-1830111954


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v7]

2024-01-18 Thread Julian Waters
> [JDK-8323008](https://bugs.openjdk.org/browse/JDK-8323008) reports unwanted 
> autoconf flags being added to the command line, and solves the issue by 
> filtering out the added flags by force. This is not optimal however, as doing 
> so leaves the autoconf message that the flags were added still displaying 
> during the configure process, which is confusing. Instead, setting a couple 
> of fields to disable the autoconf internals responsible for the unwanted flag 
> is a cleaner solution

Julian Waters has updated the pull request incrementally with one additional 
commit since the last revision:

  Implement custom AC_PROG_CC and AC_PROG_CXX in util.m4

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17401/files
  - new: https://git.openjdk.org/jdk/pull/17401/files/1e37103f..8d8d3749

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17401=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=17401=05-06

  Stats: 64 lines in 1 file changed: 64 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/17401.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17401/head:pull/17401

PR: https://git.openjdk.org/jdk/pull/17401


Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v6]

2024-01-18 Thread Hannes Wallnöfer
> This is a rather big change to update the structural navigation in API 
> documentation generated by JavaDoc. It adds a table of contents for the 
> current page to module, package, and class documentation, and replaces the 
> old sub-navigation bar with a breadcrumb-style links in those pages. The 
> table of contents is displayed as sidebar on wide/desktop displays and 
> integrated into the collapsible menu for narrow/mobile displays. The 
> generated docs can be browsed here:
> 
> https://cr.openjdk.org/~hannesw/8320458/api.00/
> 
> This change includes improvements in `stylesheet.css` and `script.js` that 
> are not strictly related to the navigation changes, but happened as a 
> consequence of the necessary restructuring. All these are described together 
> with the more on-topic changes in the list below.
> 
>  - The table of contents is composed as the respective writers generate the 
> pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of 
> new type `ListBuilder`. If the field is not `null` it is used to build the 
> table of contents as the page is built using either one of the  
> new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` 
> directly.
>  - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate 
> the markup for the sidebar and it is added to the page via the 
> `BodyContents.setSideContent` method.
>  - Both existing navigation bars (top and sub-navigation) get an additional 
> `` container with CSS class `nav-content` that uses a flex layout for 
> its content. This also handles vertical positioning, so the old workaround 
> for vertical of the language version label in `Docs.gmk` is not necessary 
> anymore.
>  - Apart from modules, packages, and classes, other pages that were converted 
> to obtain a table of contents are the "Constant Field Values" page and the 
> Help page. 
>  - Originally, I used the `` element for the sidebar, but I learned 
> that this was the wrong element as it is meant for content that is not 
> strictly related to the main content of the page. The prevailing notion seems 
> to be that a table of contents is a navigation element and therefore should 
> use the `` element, so I used that for the TOC sidebar. The same applies 
> for the breadcrumbs sub-navigation, so I left the header `` element 
> wrapped around both top and sub-navigation.
>  - For the new lists in TOC and breadcrumbs I used ordered list elements 
> `` instead of unordered `` we use everywhere else, as that is what 
> should be used when list order is important...

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Initialize tableOfContents field in HtmlDoclet constructor

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17062/files
  - new: https://git.openjdk.org/jdk/pull/17062/files/bf8cff91..e777ca9d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17062=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=17062=04-05

  Stats: 21 lines in 7 files changed: 7 ins; 5 del; 9 mod
  Patch: https://git.openjdk.org/jdk/pull/17062.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17062/head:pull/17062

PR: https://git.openjdk.org/jdk/pull/17062


Integrated: 8323995: Suppress notes generated on incremental microbenchmark builds

2024-01-18 Thread Claes Redestad
On Wed, 17 Jan 2024 13:35:23 GMT, Claes Redestad  wrote:

> Incrementally building the OpenJDK microbenchmarks emit a "Note: Benchmark 
> entries for already exists, overwriting" for each microbenchmark 
> class. This is generated by the JMH BenchmarkProcessor annotation processor 
> via the javac messager framework.
> 
> To carefully disable this informative message without disabling all linters 
> and warnings we can supply the undocumented `-XDsuppressNotes` flag.

This pull request has now been integrated.

Changeset: 4c1a0fc5
Author:Claes Redestad 
URL:   
https://git.openjdk.org/jdk/commit/4c1a0fc58fc3da5d3fd0205ffd1660331be485f0
Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod

8323995: Suppress notes generated on incremental microbenchmark builds

Reviewed-by: erikj, ihse

-

PR: https://git.openjdk.org/jdk/pull/17464


Re: RFR: 8323995: Suppress notes generated on incremental microbenchmark builds

2024-01-18 Thread Claes Redestad
On Wed, 17 Jan 2024 13:35:23 GMT, Claes Redestad  wrote:

> Incrementally building the OpenJDK microbenchmarks emit a "Note: Benchmark 
> entries for already exists, overwriting" for each microbenchmark 
> class. This is generated by the JMH BenchmarkProcessor annotation processor 
> via the javac messager framework.
> 
> To carefully disable this informative message without disabling all linters 
> and warnings we can supply the undocumented `-XDsuppressNotes` flag.

Thanks for reviewing!

-

PR Comment: https://git.openjdk.org/jdk/pull/17464#issuecomment-1898715717


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v6]

2024-01-18 Thread Julian Waters
> [JDK-8323008](https://bugs.openjdk.org/browse/JDK-8323008) reports unwanted 
> autoconf flags being added to the command line, and solves the issue by 
> filtering out the added flags by force. This is not optimal however, as doing 
> so leaves the autoconf message that the flags were added still displaying 
> during the configure process, which is confusing. Instead, setting a couple 
> of fields to disable the autoconf internals responsible for the unwanted flag 
> is a cleaner solution

Julian Waters has updated the pull request incrementally with one additional 
commit since the last revision:

  Partially revert toolchain.m4

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17401/files
  - new: https://git.openjdk.org/jdk/pull/17401/files/71a93ea0..1e37103f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17401=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=17401=04-05

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/17401.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17401/head:pull/17401

PR: https://git.openjdk.org/jdk/pull/17401


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v5]

2024-01-18 Thread Julian Waters
> [JDK-8323008](https://bugs.openjdk.org/browse/JDK-8323008) reports unwanted 
> autoconf flags being added to the command line, and solves the issue by 
> filtering out the added flags by force. This is not optimal however, as doing 
> so leaves the autoconf message that the flags were added still displaying 
> during the configure process, which is confusing. Instead, setting a couple 
> of fields to disable the autoconf internals responsible for the unwanted flag 
> is a cleaner solution

Julian Waters has updated the pull request incrementally with one additional 
commit since the last revision:

  Revert util.m4

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17401/files
  - new: https://git.openjdk.org/jdk/pull/17401/files/776f8aa4..71a93ea0

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17401=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=17401=03-04

  Stats: 50 lines in 1 file changed: 0 ins; 50 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/17401.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17401/head:pull/17401

PR: https://git.openjdk.org/jdk/pull/17401


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v4]

2024-01-18 Thread Julian Waters
On Thu, 18 Jan 2024 13:59:30 GMT, Erik Joelsson  wrote:

> > What fun, I just found out that omitting the call to AC_PROG_CC and 
> > AC_PROG_CXX has no effect, and they're _still_ called magically from 
> > somewhere by autoconf even if you don't call them. I'm genuinely at a loss 
> > for words :)
> 
> If I'm to guess, I think this is caused by AC_PROG_* being defined as 
> AC_DEFUN_ONCE and every other autoconf macro that uses compilers for checks 
> will implicitly call AC_PROG_* first if it hasn't been called yet. My 
> understanding of these implicit calls is to make a more declarative 
> programming style possible.

It's not so much the AC_DEFUN_ONCE, but actually due to other autoconf macros 
calling AC_REQUIRE([AC_PROG_CC]) and AC_REQUIRE([AC_PROG_CXX]), for instance 
our calls to AC_PROG_CPP and AC_PROG_CXXCPP will cause AC_PROG_CC and 
AC_PROG_CXX respectively to be implicitly called

-

PR Comment: https://git.openjdk.org/jdk/pull/17401#issuecomment-1898554535


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v4]

2024-01-18 Thread Erik Joelsson
On Wed, 17 Jan 2024 15:30:36 GMT, Julian Waters  wrote:

> What fun, I just found out that omitting the call to AC_PROG_CC and 
> AC_PROG_CXX has no effect, and they're _still_ called magically from 
> somewhere by autoconf even if you don't call them. I'm genuinely at a loss 
> for words :)

If I'm to guess, I think this is caused by AC_PROG_* being defined as 
AC_DEFUN_ONCE and every other autoconf macro that uses compilers for checks 
will implicitly call AC_PROG_* first if it hasn't been called yet. My 
understanding of these implicit calls is to make a more declarative programming 
style possible.

-

PR Comment: https://git.openjdk.org/jdk/pull/17401#issuecomment-1898532921


Re: RFR: 8323675: Race in jdk.javadoc-gendata [v2]

2024-01-18 Thread Erik Joelsson
On Thu, 18 Jan 2024 07:25:48 GMT, Magnus Ihse Bursie  wrote:

>> Uh, sorry for not being precise. I was talking of the 
>> `JavadocElementList`/`jdk.javadoc-gendata` - that does not use classfiles, 
>> only sources. `jdk.compiler-gendata`/`ct.sym` generation surely does use 
>> (all) the new JDK classfiles. Sorry for the confusion.
>
> @lahodaj Just to be absolutely clear: In `jdk.javadoc-gendata`, we're calling 
> two tools: Not only `JavadocElementList` (which only requires source, not 
> class files), but also `CreateSymbols build-javadoc-data`. Can you confirm 
> that this too only requires sources?
> 
> (I take it that this usage is different from `CreateSymbols build-ctsym` as 
> called in `jdk.compiler-gendata`.)

Looking at `make/modules/jdk.javadoc/Gendata.gmk` the recipe never references 
`$(JDK_OUTPUTDIR)/modules/`, the output dir where the compiled classes are 
located. It only references the output of `$(call GetModuleSrcPath)` (and some 
static src dirs), so I'm pretty confident that this is correct.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17402#discussion_r1457474979


Re: RFR: 8323675: Race in jdk.javadoc-gendata [v2]

2024-01-18 Thread Erik Joelsson
On Thu, 18 Jan 2024 07:34:31 GMT, Magnus Ihse Bursie  wrote:

>> In [JDK-8318913](https://bugs.openjdk.org/browse/JDK-8318913), the 
>> symbolgenerator started to look at current sources as well. This means that 
>> the gensrc stage needs to be completed before this is run. A dependency was 
>> added for jdk.compiler-gendata, but unfortunately the same tool is run also 
>> in jdk.javadoc-gendata, where no such safeguard was created.
>> 
>> The result is that the build can fail intermittently with:
>> 
>> .../module-info.java:77: error: module not found on module source path
>> module java.base {
>> ^
>> error: cannot access module-info
>>   cannot resolve modules
>> Exception in thread "main" java.lang.AssertionError
>> at 
>> jdk.compiler.interim/com.sun.tools.javac.util.Assert.error(Assert.java:155)
>> at 
>> jdk.compiler.interim/com.sun.tools.javac.util.Assert.checkNonNull(Assert.java:62)
>> at 
>> jdk.compiler.interim/com.sun.tools.javac.comp.Modules.allModules(Modules.java:1225)
>> at 
>> jdk.compiler.interim/com.sun.tools.javac.comp.Modules.getObservableModule(Modules.java:1450)
>> at 
>> jdk.compiler.interim/com.sun.tools.javac.model.JavacElements.getModuleElement(JavacElements.java:144)
>> at 
>> jdk.compiler.interim/com.sun.tools.javac.model.JavacElements.getModuleElement(JavacElements.java:89)
>> at 
>> build.tools.symbolgenerator.JavadocElementList.main(JavadocElementList.java:98)
>> Compiling up to 2 files for BUILD_BREAKITERATOR_BASE
>> Compiling up to 2 files for BUILD_BREAKITERATOR_LD
>> make[3]: *** [.../_element_lists.marker] Error 1
>> Gendata.gmk:74: recipe for target '.../_element_lists.marker' failed
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Let jdk.javadoc-gendata only depend on GENSRC_TARGETS

Marked as reviewed by erikj (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/17402#pullrequestreview-1829723247


Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v5]

2024-01-18 Thread Hannes Wallnöfer
> This is a rather big change to update the structural navigation in API 
> documentation generated by JavaDoc. It adds a table of contents for the 
> current page to module, package, and class documentation, and replaces the 
> old sub-navigation bar with a breadcrumb-style links in those pages. The 
> table of contents is displayed as sidebar on wide/desktop displays and 
> integrated into the collapsible menu for narrow/mobile displays. The 
> generated docs can be browsed here:
> 
> https://cr.openjdk.org/~hannesw/8320458/api.00/
> 
> This change includes improvements in `stylesheet.css` and `script.js` that 
> are not strictly related to the navigation changes, but happened as a 
> consequence of the necessary restructuring. All these are described together 
> with the more on-topic changes in the list below.
> 
>  - The table of contents is composed as the respective writers generate the 
> pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of 
> new type `ListBuilder`. If the field is not `null` it is used to build the 
> table of contents as the page is built using either one of the  
> new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` 
> directly.
>  - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate 
> the markup for the sidebar and it is added to the page via the 
> `BodyContents.setSideContent` method.
>  - Both existing navigation bars (top and sub-navigation) get an additional 
> `` container with CSS class `nav-content` that uses a flex layout for 
> its content. This also handles vertical positioning, so the old workaround 
> for vertical of the language version label in `Docs.gmk` is not necessary 
> anymore.
>  - Apart from modules, packages, and classes, other pages that were converted 
> to obtain a table of contents are the "Constant Field Values" page and the 
> Help page. 
>  - Originally, I used the `` element for the sidebar, but I learned 
> that this was the wrong element as it is meant for content that is not 
> strictly related to the main content of the page. The prevailing notion seems 
> to be that a table of contents is a navigation element and therefore should 
> use the `` element, so I used that for the TOC sidebar. The same applies 
> for the breadcrumbs sub-navigation, so I left the header `` element 
> wrapped around both top and sub-navigation.
>  - For the new lists in TOC and breadcrumbs I used ordered list elements 
> `` instead of unordered `` we use everywhere else, as that is what 
> should be used when list order is important...

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Address review feedback
   - Use record instead of array in ListBuilder
   - Rename TableOfContents.getSideBar to toContent

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17062/files
  - new: https://git.openjdk.org/jdk/pull/17062/files/70d06e59..bf8cff91

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17062=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=17062=03-04

  Stats: 14 lines in 7 files changed: 1 ins; 1 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/17062.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17062/head:pull/17062

PR: https://git.openjdk.org/jdk/pull/17062


Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]

2024-01-18 Thread Hannes Wallnöfer
On Tue, 16 Jan 2024 19:47:47 GMT, Jonathan Gibbons  wrote:

> * Medium-size suggestion/question:
>   consider init-ing `HtmlDocletWriter.tableOfContents` directly in the 
> constructor for `HtmlDocletWriter` and not in the constructors for the 
> subtypes.

I agree it would be nice, but not sure how to do it without proliferation of 
`HtmlDocletWriter` constructors and/or constructor arguments (we have 15 direct 
`HtmlDocletWriter` subclasses of which 5 have a table of contents, and they are 
using both available super-constructors).

-

PR Comment: https://git.openjdk.org/jdk/pull/17062#issuecomment-1898279784


Integrated: 8323667: Library debug files contain non-reproducible full gcc include paths

2024-01-18 Thread Andrew Leonard
On Fri, 12 Jan 2024 14:33:01 GMT, Andrew Leonard  wrote:

> For gcc toolchains in ALLOW_ABSOLUTE_PATHS_IN_OUTPUT=False builds, this PR 
> finds the location of the gcc system include paths, and adds 
> -fdebug-prefix-map flags to map them to a standard location. Thus making the 
> debuginfo and resulting libraries reproducible when using DevKits in 
> different path locations.

This pull request has now been integrated.

Changeset: 57fad677
Author:Andrew Leonard 
URL:   
https://git.openjdk.org/jdk/commit/57fad677819ae3142782f811a8fba94b38f5a74c
Stats: 55 lines in 1 file changed: 54 ins; 0 del; 1 mod

8323667: Library debug files contain non-reproducible full gcc include paths

Reviewed-by: erikj, ihse

-

PR: https://git.openjdk.org/jdk/pull/17399


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v4]

2024-01-18 Thread Julian Waters
On Thu, 18 Jan 2024 09:23:24 GMT, Magnus Ihse Bursie  wrote:

> > (Sometimes I wonder whether Java could've gone the way V8 went and used 
> > Python or Java itself as the build system instead, oh well)
> 
> While I think we're stuck with make for the long run, a long-term goal for me 
> has been (and still is) to replace autoconf with a system written in Java. 
> Fortunately, we have a well defined interface: The user runs `configure` with 
> a set of well-know arguments, we check for a bunch of requirements and the 
> validity of the requested build configuration, and generates a well-defined 
> spec.gmk file as a result. This tight encapsulation means it is possible to 
> make a drop-in replacement in Java. The challenge is "just" to make sure we 
> don't loose any functionality, nor skip over any of the odd fixes and 
> adaptation to real-world build systems that the current configure script has 
> collected over the years.

That sounds like a huge potential improvement, at least to me. I've also just 
come across JEP-138 (Fascinating cache of JDK history within that JEP! Never 
knew the JDK used to only work with make), so it's interesting to see if 
autoconf can be phased out after its introduction 9 years ago. If I understand 
you correctly, this new hypothetical system would replace autoconf as such?

configure -> autoconf -> compiled configure -> make
to
configure -> compile Java -> Java configure program -> make

-

PR Comment: https://git.openjdk.org/jdk/pull/17401#issuecomment-1898128782


Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]

2024-01-18 Thread Hannes Wallnöfer
On Tue, 16 Jan 2024 19:46:30 GMT, Jonathan Gibbons  wrote:

>> Hannes Wallnöfer has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Consolidate TOC functionality into new TableOfContents class
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TableOfContents.java
>  line 83:
> 
>> 81:  * @return a content object
>> 82:  */
>> 83: protected Content getSideBar(boolean hasFilterInput) {
> 
> (Just asking)
> 
> In other builders for "complex" content, like `Table` and `Navigation` , we 
> have methods like `toContent` or `getContent`.  Should we follow that naming 
> convention?  While the name `getSidebar` is certainly more descriptive, is 
> the returned object inherently a sidebar -- or is the fact that it is a 
> sidebar just an artifact of how we currently choose to present the content?

It is actually up to the CSS how the table of contents is rendered. I'm 
renaming the method to `toContent()`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1457174804


Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]

2024-01-18 Thread Hannes Wallnöfer
On Tue, 16 Jan 2024 19:26:21 GMT, Jonathan Gibbons  wrote:

>> Hannes Wallnöfer has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Consolidate TOC functionality into new TableOfContents class
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriter.java
>  line 99:
> 
>> 97: 
>> 98: pHelper = new PropertyUtils.PropertyHelper(configuration, 
>> typeElement);
>> 99: tableOfContents = new TableOfContents(this);
> 
> Is there a reason to do this here (and also in `ModuleWriter` and 
> `PackageWriter` etc) rather than in the (shared) super-constructor for 
> `HtmlDocletWriter` ?

The reason for doing it in the concrete writer classes is that only some 
`HtmlDocletWriter` subclasses have a table of contents, and I don't think 
there's an easy way to figure out in the super-constructor which instances need 
a table of contents and which don't. We could have the subclasses pass it as an 
argument to the super-constructor I guess.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1457162662


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v4]

2024-01-18 Thread Magnus Ihse Bursie
On Thu, 18 Jan 2024 09:12:46 GMT, Julian Waters  wrote:

> (Sometimes I wonder whether Java could've gone the way V8 went and used 
> Python or Java itself as the build system instead, oh well)

While I think we're stuck with make for the long run, a long-term goal for me 
has been (and still is) to replace autoconf with a system written in Java. 
Fortunately, we have a well defined interface: The user runs `configure` with a 
set of well-know arguments, we check for a bunch of requirements and the 
validity of the requested build configuration, and generates a well-defined 
spec.gmk file as a result. This tight encapsulation means it is possible to 
make a drop-in replacement in Java. The challenge is "just" to make sure we 
don't loose any functionality, nor skip over any of the odd fixes and 
adaptation to real-world build systems that the current configure script has 
collected over the years.

-

PR Comment: https://git.openjdk.org/jdk/pull/17401#issuecomment-1898097805


Re: RFR: 8323672: Suppress unwanted autoconf added flags in CC and CXX [v4]

2024-01-18 Thread Julian Waters
On Thu, 18 Jan 2024 07:37:11 GMT, Magnus Ihse Bursie  wrote:

> Then it sounds like this is not a viable approach.

There's definitely gotta be a way to make autoconf do what we want that's out 
there, I'll work on it and post any findings back here. I've just found out 
that the implicit calls to AC_PROG_CC are due to AC_PROG_CPP calling AC_REQUIRE 
on AC_PROG_CC for instance



> Also, check your inbox for a personal invitation to The "wtf-autoconf-rly?" 
> Club. ;-)

I think I'm already a long time member of that club by this point...
(Sometimes I wonder whether Java could've gone the way V8 went and used Python 
or Java itself as the build system instead, oh well)

-

PR Comment: https://git.openjdk.org/jdk/pull/17401#issuecomment-1898080485


Re: RFR: 8323995: Suppress notes generated on incremental microbenchmark builds

2024-01-18 Thread Magnus Ihse Bursie
On Wed, 17 Jan 2024 13:35:23 GMT, Claes Redestad  wrote:

> Incrementally building the OpenJDK microbenchmarks emit a "Note: Benchmark 
> entries for already exists, overwriting" for each microbenchmark 
> class. This is generated by the JMH BenchmarkProcessor annotation processor 
> via the javac messager framework.
> 
> To carefully disable this informative message without disabling all linters 
> and warnings we can supply the undocumented `-XDsuppressNotes` flag.

LGTM

-

Marked as reviewed by ihse (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17464#pullrequestreview-1829157912