[jdk19] RFR: 8278274: Update nroff pages in JDK 19 before RC

2022-07-17 Thread David Holmes
Please review these changes to the nroff manpage files so that they match their 
markdown sources that Oracle maintains.

All pages at a minimum have 19-ea replaced with 19, and copyright set to 2022 
if needed.  Additionally:

The Java manpage was missing updates from:
- [JDK-8282018](https://bugs.openjdk.org/browse/JDK-8282018): Add captions to 
tables on java man page.

The Java manpage has slight formatting differences from:
- [JDK-8262004](https://bugs.openjdk.org/browse/JDK-8262004): Classpath 
separator: Man page says semicolon; should be colon on Linux
- [JDK-8236569](https://bugs.openjdk.org/browse/JDK-8236569): -Xss not multiple 
of 4K does not work for the main thread on macOS

The Java manpage has a typo fixed in mainline by 
[JDK-8279047](https://bugs.openjdk.org/browse/JDK-8279047) (for JDK 20)


The keytool manpage was missing updates from:
- [JDK-8282014](https://bugs.openjdk.org/browse/JDK-8282014): Add captions to 
tables on keytool man page.
- [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
default key sizes and algorithms based on CNSA

The jar manpage was missing updates from:
- [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod man 
pages need the new --date documenting from CSR 
[JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)

The jarsigner manpage was missing updates from:
- [JDK-8282015](https://bugs.openjdk.org/browse/JDK-8282015): Add captions to 
tables on jarsigner man page.
- [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
default key sizes and algorithms based on CNSA

The javadoc manpage was missing updates from:
- [JDK-8279034](https://bugs.openjdk.org/browse/JDK-8279034): Update man page 
for javadoc `--date` option

The jmod manpage was missing updates from:
- [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod man 
pages need the new --date documenting from CSR 
[JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)

The jpackage manpage was missing updates from:
- [JDK-8285146](https://bugs.openjdk.org/browse/JDK-8285146): Document jpackage 
resource dir feature
- [JDK-8284695](https://bugs.openjdk.org/browse/JDK-8284695): Update jpackage 
man pages for JDK 19
- [JDK-8284209](https://bugs.openjdk.org/browse/JDK-8284209): Replace remaining 
usages of 'a the' in source code

The jshell manpage was missing updates from:
- [JDK-8282016](https://bugs.openjdk.org/browse/JDK-8282016): Add captions to 
tables on jshell man page.

-

Commit messages:
 - 8278274: Update nroff pages in JDK 19 before RC

Changes: https://git.openjdk.org/jdk19/pull/145/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19&pr=145&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8278274
  Stats: 515 lines in 28 files changed: 431 ins; 16 del; 68 mod
  Patch: https://git.openjdk.org/jdk19/pull/145.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/145/head:pull/145

PR: https://git.openjdk.org/jdk19/pull/145


Re: [jdk19] RFR: 8278274: Update nroff pages in JDK 19 before RC

2022-07-18 Thread David Holmes
On Mon, 18 Jul 2022 15:26:23 GMT, Jonathan Gibbons  wrote:

>> Please review these changes to the nroff manpage files so that they match 
>> their markdown sources that Oracle maintains.
>> 
>> All pages at a minimum have 19-ea replaced with 19, and copyright set to 
>> 2022 if needed.  Additionally:
>> 
>> The Java manpage was missing updates from:
>> - [JDK-8282018](https://bugs.openjdk.org/browse/JDK-8282018): Add captions 
>> to tables on java man page.
>> 
>> The Java manpage has slight formatting differences from:
>> - [JDK-8262004](https://bugs.openjdk.org/browse/JDK-8262004): Classpath 
>> separator: Man page says semicolon; should be colon on Linux
>> - [JDK-8236569](https://bugs.openjdk.org/browse/JDK-8236569): -Xss not 
>> multiple of 4K does not work for the main thread on macOS
>> 
>> The Java manpage has a typo fixed in mainline by 
>> [JDK-8279047](https://bugs.openjdk.org/browse/JDK-8279047) (for JDK 20)
>> 
>> 
>> The keytool manpage was missing updates from:
>> - [JDK-8282014](https://bugs.openjdk.org/browse/JDK-8282014): Add captions 
>> to tables on keytool man page.
>> - [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
>> default key sizes and algorithms based on CNSA
>> 
>> The jar manpage was missing updates from:
>> - [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod 
>> man pages need the new --date documenting from CSR 
>> [JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)
>> 
>> The jarsigner manpage was missing updates from:
>> - [JDK-8282015](https://bugs.openjdk.org/browse/JDK-8282015): Add captions 
>> to tables on jarsigner man page.
>> - [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
>> default key sizes and algorithms based on CNSA
>> 
>> The javadoc manpage was missing updates from:
>> - [JDK-8279034](https://bugs.openjdk.org/browse/JDK-8279034): Update man 
>> page for javadoc `--date` option
>> 
>> The jmod manpage was missing updates from:
>> - [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod 
>> man pages need the new --date documenting from CSR 
>> [JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)
>> 
>> The jpackage manpage was missing updates from:
>> - [JDK-8285146](https://bugs.openjdk.org/browse/JDK-8285146): Document 
>> jpackage resource dir feature
>> - [JDK-8284695](https://bugs.openjdk.org/browse/JDK-8284695): Update 
>> jpackage man pages for JDK 19
>> - [JDK-8284209](https://bugs.openjdk.org/browse/JDK-8284209): Replace 
>> remaining usages of 'a the' in source code
>> 
>> The jshell manpage was missing updates from:
>> - [JDK-8282016](https://bugs.openjdk.org/browse/JDK-8282016): Add captions 
>> to tables on jshell man page.
>
> The version changes in each file look good (`19-ea` to `19`).
> The changes for javadoc look good.
> 
> I looked over the other changes for other files, and while they look good, I 
> cannot speak for their technical accuracy. That being said, this is an 
> automated process deriving info from upstream, so is likely OK.

Thanks for the review @jonathan-gibbons ! I'll wait a day in case there are any 
further comments.

> src/java.base/share/man/keytool.1 line 456:
> 
>> 454: \f[CB]PrivateKeyEntry\f[R] for the signer that already exists in the
>> 455: keystore.
>> 456: This option is used to sign the certificate with the signer?s private
> 
> Not a problem with this PR as such, but we still have a `?` character in the 
> output.

Yeah I spotted that too, but it would need to be fixed in source and nroff.  
Must be some kind of "smart quote" from an editor. Do you think this needs to 
be fixed or just handle it in mainline?

-

PR: https://git.openjdk.org/jdk19/pull/145


[jdk19] Integrated: 8278274: Update nroff pages in JDK 19 before RC

2022-07-19 Thread David Holmes
On Sun, 17 Jul 2022 22:44:02 GMT, David Holmes  wrote:

> Please review these changes to the nroff manpage files so that they match 
> their markdown sources that Oracle maintains.
> 
> All pages at a minimum have 19-ea replaced with 19, and copyright set to 2022 
> if needed.  Additionally:
> 
> The Java manpage was missing updates from:
> - [JDK-8282018](https://bugs.openjdk.org/browse/JDK-8282018): Add captions to 
> tables on java man page.
> 
> The Java manpage has slight formatting differences from:
> - [JDK-8262004](https://bugs.openjdk.org/browse/JDK-8262004): Classpath 
> separator: Man page says semicolon; should be colon on Linux
> - [JDK-8236569](https://bugs.openjdk.org/browse/JDK-8236569): -Xss not 
> multiple of 4K does not work for the main thread on macOS
> 
> The Java manpage has a typo fixed in mainline by 
> [JDK-8279047](https://bugs.openjdk.org/browse/JDK-8279047) (for JDK 20)
> 
> 
> The keytool manpage was missing updates from:
> - [JDK-8282014](https://bugs.openjdk.org/browse/JDK-8282014): Add captions to 
> tables on keytool man page.
> - [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
> default key sizes and algorithms based on CNSA
> 
> The jar manpage was missing updates from:
> - [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod 
> man pages need the new --date documenting from CSR 
> [JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)
> 
> The jarsigner manpage was missing updates from:
> - [JDK-8282015](https://bugs.openjdk.org/browse/JDK-8282015): Add captions to 
> tables on jarsigner man page.
> - [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
> default key sizes and algorithms based on CNSA
> 
> The javadoc manpage was missing updates from:
> - [JDK-8279034](https://bugs.openjdk.org/browse/JDK-8279034): Update man page 
> for javadoc `--date` option
> 
> The jmod manpage was missing updates from:
> - [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod 
> man pages need the new --date documenting from CSR 
> [JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)
> 
> The jpackage manpage was missing updates from:
> - [JDK-8285146](https://bugs.openjdk.org/browse/JDK-8285146): Document 
> jpackage resource dir feature
> - [JDK-8284695](https://bugs.openjdk.org/browse/JDK-8284695): Update jpackage 
> man pages for JDK 19
> - [JDK-8284209](https://bugs.openjdk.org/browse/JDK-8284209): Replace 
> remaining usages of 'a the' in source code
> 
> The jshell manpage was missing updates from:
> - [JDK-8282016](https://bugs.openjdk.org/browse/JDK-8282016): Add captions to 
> tables on jshell man page.

This pull request has now been integrated.

Changeset: 618f3a82
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk19/commit/618f3a82a4d45cdb66b86259ae60dd1c322b987b
Stats: 515 lines in 28 files changed: 431 ins; 16 del; 68 mod

8278274: Update nroff pages in JDK 19 before RC

Reviewed-by: jjg

-

PR: https://git.openjdk.org/jdk19/pull/145


Re: [jdk19] RFR: 8278274: Update nroff pages in JDK 19 before RC

2022-07-19 Thread David Holmes
On Mon, 18 Jul 2022 23:27:58 GMT, David Holmes  wrote:

>> src/java.base/share/man/keytool.1 line 456:
>> 
>>> 454: \f[CB]PrivateKeyEntry\f[R] for the signer that already exists in the
>>> 455: keystore.
>>> 456: This option is used to sign the certificate with the signer?s private
>> 
>> Not a problem with this PR as such, but we still have a `?` character in the 
>> output.
>
> Yeah I spotted that too, but it would need to be fixed in source and nroff.  
> Must be some kind of "smart quote" from an editor. Do you think this needs to 
> be fixed or just handle it in mainline?

Filed [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626). It can easily 
be fixed before RDP2.

-

PR: https://git.openjdk.org/jdk19/pull/145


RFR: 8290489: Initial nroff manpage generation for JDK 20

2022-07-20 Thread David Holmes
The version will be 20-ea and the copyright year 2023 (for March 2023 release 
date).

Thanks.

-

Commit messages:
 - 8290489: Initial nroff manpage generation for JDK 20

Changes: https://git.openjdk.org/jdk/pull/9581/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9581&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290489
  Stats: 28 lines in 28 files changed: 0 ins; 0 del; 28 mod
  Patch: https://git.openjdk.org/jdk/pull/9581.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9581/head:pull/9581

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


Re: RFR: 8290489: Initial nroff manpage generation for JDK 20

2022-07-20 Thread David Holmes
On Thu, 21 Jul 2022 01:03:46 GMT, Daniel D. Daugherty  
wrote:

>> The version will be 20-ea and the copyright year 2023 (for March 2023 
>> release date).
>> 
>> Thanks.
>
> Thumbs up. This is a trivial change.

Thanks @dcubed-ojdk !

-

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


Integrated: 8290489: Initial nroff manpage generation for JDK 20

2022-07-21 Thread David Holmes
On Thu, 21 Jul 2022 00:34:53 GMT, David Holmes  wrote:

> The version will be 20-ea and the copyright year 2023 (for March 2023 release 
> date).
> 
> Thanks.

This pull request has now been integrated.

Changeset: e9f97b2e
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk/commit/e9f97b2e8cf301ba6b69101e5efc5c71d26bc87b
Stats: 28 lines in 28 files changed: 0 ins; 0 del; 28 mod

8290489: Initial nroff manpage generation for JDK 20

Reviewed-by: dcubed

-

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


Re: suggest improvement for java doc for getting address of local host

2022-08-04 Thread David Holmes

On 5/08/2022 8:24 am, mark.yagnatin...@barclays.com wrote:

I suspect that this is the wrong list; please redirect me if so.


Redirected to net-dev

Cheers,
David


The docs for this method:

https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/net/InetAddress.html#getLocalHost() 



say that it “Returns the address of the local host. This is achieved by 
retrieving the name of the host from the system, then resolving that 
name into an InetAddress.”


This is true enough in simple cases, but horribly misleading in 
non-trivial cases.


As far as I can tell it can’t return “the address” because the local 
host may have multiple addresses.


Instead it returns “an address” of the local host.  My laptop right now 
has 3 addresses: IPv6 loopback, IPv4 from home WiFi, and IPv4 from 
company network via VPN.


As far as I can tell (please correct me if I’m wrong!!) if there are 
multiple addresses, the current implementation makes no effort to pick a 
more “useful” one.


Instead, it takes whatever addresses it gets from the OS and returns the 
first one.


Thus, in theory, and even in practice, it’s perfectly possible to get 
into a situation where all of the following hold:


“It worked fine yesterday!”

“I didn’t change anything, honest!”

And yet, “nothing works today” (because, unbeknownst to mere mortals 
like me, the OS happened to return things in a different order today.)


Again, please correct me if I’m wrong and there actually are some 
guarantees, either by virtue of the spec, or by virtue of “it just so 
happens that in practice you will tend to get XYZ”.


But if I’m right, then I suggest the following tweak to the documentation:

Returns the address of the local host.  If the local host has multiple 
addresses, an arbitrary one will be chosen and returned.


This is achieved by retrieving the name of the host from the system, 
then resolving that name into a list of one or more `InetAddress`es and 
then choosing an arbitrary address from that list.


(irrelevant but: really wish Javadoc had a lightweight notation for code 
like markdown’s `code(blocks)`)


Just my two cents.  Again, please redirect me if this is the wrong list, 
and correct me if I’m wrong re: no guarantees about which address is 
returned.


This message is for information purposes only. It is not a 
recommendation, advice, offer or solicitation to buy or sell a product 
or service, nor an official confirmation of any transaction. It is 
directed at persons who are professionals and is intended for the 
recipient(s) only. It is not directed at retail customers. This message 
is subject to the terms at: 
https://www.cib.barclays/disclosures/web-and-email-disclaimer.html 
.


For important disclosures, please see: 
https://www.cib.barclays/disclosures/sales-and-trading-disclaimer.html 
 
regarding marketing commentary from Barclays Sales and/or Trading desks, 
who are active market participants; 
https://www.cib.barclays/disclosures/barclays-global-markets-disclosures.html 
 
regarding our standard terms for Barclays Corporate and Investment Bank 
where we trade with you in principal-to-principal wholesale markets 
transactions; and in respect to Barclays Research, including disclosures 
relating to specific issuers, see: http://publicresearch.barclays.com 
.
__ 

If you are incorporated or operating in Australia, read these important 
disclosures: 
https://www.cib.barclays/disclosures/important-disclosures-asia-pacific.html 
.

__
For more details about how we use personal information, see our privacy 
notice: 
https://www.cib.barclays/disclosures/personal-information-use.html 
.

__



Re: [jdk19] RFR: 8278274: Update nroff pages in JDK 19 before RC

2022-08-07 Thread David Holmes

Hi Patrick,

On 6/08/2022 7:05 pm, Patrick Pfeifer wrote:

On Mon, 18 Jul 2022 15:22:01 GMT, Jonathan Gibbons  wrote:


Please review these changes to the nroff manpage files so that they match their 
markdown sources that Oracle maintains.


Not a problem with this PR as such, but we still have a `?` character in the 
output.


Hello @jonathan-gibbons

Excuse my hijacking / piggy-backing on this conversation!

When you say


Not a problem with this PR as such, but we still have a `?` character in the 
output.


This strongly suggests that there must be an input to those man pages 
somewhere. :-) Would you mind to point me to it? Are they even included in the 
git repo?


The sources to the manpages are not currently open sourced and so are 
held in Oracle's internal repository. Making them open-sourced has been 
a goal for some time now but has to overcome legal obstacles, but we are 
hopeful it will be "soon".



Also, I was wondering why these man pages (i.e. the outputs) are obviously not 
included any more in the OpenJDK builds, e.g. https://jdk.java.net/19/ ? I am 
not sure when they were removed or if they were ever included in _open_jdk 
builds at all, but I see them in the Oracle JDK 1.8 Build.

I find them really nice to read! Are there other output formats, e.g. HTML, 
that might be are deployed somewhere on the web?


As you found, Oracle does provide these in html format.

Cheers,
David



-

PR: https://git.openjdk.org/jdk19/pull/145


Re: RFR: JDK-8297164: Update troff man pages and CheckManPageOptions.java

2022-11-17 Thread David Holmes
On Thu, 17 Nov 2022 22:23:53 GMT, Jonathan Gibbons  wrote:

> Please review an update for the troff man pages, following the recent update 
> to upgrade to use pandoc 2.19.2
> (See https://bugs.openjdk.org/browse/JDK-8297165)
> 
> In conjunction with this, one javadoc test also needs to be updated, to work 
> with the new form of output generated by the new version of pandoc.

Hi @jonathan-gibbons ,

I notice that in the new version dash characters are no longer escaped as `-`, 
do these still display correctly?

-

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


Re: RFR: JDK-8297164: Update troff man pages and CheckManPageOptions.java

2022-11-20 Thread David Holmes
On Thu, 17 Nov 2022 22:23:53 GMT, Jonathan Gibbons  wrote:

> Please review an update for the troff man pages, following the recent update 
> to upgrade to use pandoc 2.19.2
> (See https://bugs.openjdk.org/browse/JDK-8297165)
> 
> In conjunction with this, one javadoc test also needs to be updated, to work 
> with the new form of output generated by the new version of pandoc.

Looks okay.

Thanks for doing this before the end-of-release updates!

-

Marked as reviewed by dholmes (Reviewer).

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


Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor

2023-01-05 Thread David Holmes
On Fri, 6 Jan 2023 02:20:53 GMT, Archie L. Cobbs  wrote:

> This PR adds a new lint warning category `this-escape`.
> 
> It also adds `@SuppressWarnings` annotations as needed to the JDK itself to 
> allow the JDK to continue to compile with `-Xlint:all`.
> 
> A 'this' escape warning is generated for a constructor `A()` in a class `A` 
> when the compiler detects that the following situation is _in theory 
> possible:_
> * Some subclass `B extends A` exists, and `B` is defined in a separate source 
> file (i.e., compilation unit)
> * Some constructor `B()` of `B` invokes `A()` as its superclass constructor
> * During the execution of `A()`, some non-static method of `B.foo()` could 
> get invoked, perhaps indirectly
> 
> In the above scenario, `B.foo()` would execute before `A()` has returned and 
> before `B()` has performed any initialization. To the extent `B.foo()` 
> accesses any fields of `B` - all of which are still uninitialized - it is 
> likely to function incorrectly.
> 
> Note, when determining if a 'this' escape is possible, the compiler makes no 
> assumptions about code outside of the current compilation unit. It doesn't 
> look outside of the current source file to see what might actually happen 
> when a method is invoked. It does follow method and constructors within the 
> current compilation unit, and applies a simplified 
> union-of-all-possible-branches data flow analysis to see where 'this' could 
> end up.
> 
> From my review, virtually all of the warnings generated in the various JDK 
> modules are valid warnings in the sense that a 'this' escape, as defined 
> above, is really and truly possible. However, that doesn't imply that any 
> bugs were found within the JDK - only that the possibility of a certain type 
> of bug exists if certain superclass constructors are used by someone, 
> somewhere, someday.
> 
> For several "popular" classes, this PR also adds `@implNote`'s to the 
> offending constructors so that subclass implementors are made aware of the 
> threat. For one example, `TreeMap(Map)` invokes `putAll()` and `put()`.
> 
> More details and a couple of motivating examples are given in an included 
> [doc 
> file](https://github.com/archiecobbs/jdk/blob/ThisEscape/src/java.base/share/classes/java/lang/doc-files/ThisEscape.html)
>  that these `@implNote`'s link to. See also the recent thread on `amber-dev` 
> for some background.
> 
> Ideally, over time the owners of the various modules would review their 
> `@SuppressWarnings("this-escape")` annotations and determine which other 
> constructors also warranted such an `@implNote`.
> 
> Because of all the`@SuppressWarnings` annotations, this PR touches a bunch of 
> different JDK modules. My apologies for that. Adding these annotations was 
> determined to be the more conservative approach, as compared to just 
> excepting `this-escape` from various module builds globally.
> 
> **Patch Navigation Guide**
> 
> * Non-trivial compiler changes:
> * `src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java`
> * `src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java`
> * `src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java`
> * `src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java`
> * 
> `src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java`
> * 
> `src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties`
> * 
> `src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties`
> 
> * Javadoc additions of `@implNote`:
> 
> * `src/java.base/share/classes/java/io/PipedReader.java`
> * `src/java.base/share/classes/java/io/PipedWriter.java`
> * `src/java.base/share/classes/java/lang/Throwable.java`
> * `src/java.base/share/classes/java/util/ArrayDeque.java`
> * `src/java.base/share/classes/java/util/EnumMap.java`
> * `src/java.base/share/classes/java/util/HashSet.java`
> * `src/java.base/share/classes/java/util/Hashtable.java`
> * `src/java.base/share/classes/java/util/LinkedList.java`
> * `src/java.base/share/classes/java/util/TreeMap.java`
> * `src/java.base/share/classes/java/util/TreeSet.java`
> 
> * New unit tests
> * `test/langtools/tools/javac/warnings/ThisEscape/*.java`
> 
> * **Everything else** is just adding `@SuppressWarnings("this-escape")`

Hi Archie,

The associated JBS issue has been dormant for 6+ years and this is a very 
intrusive change affecting many, many files. Has the resurrection of this 
project previously been discussed somewhere?

-

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


Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

2023-01-08 Thread David Holmes
On Sat, 7 Jan 2023 21:08:07 GMT, Archie L. Cobbs  wrote:

>> This PR adds a new lint warning category `this-escape`.
>> 
>> It also adds `@SuppressWarnings` annotations as needed to the JDK itself to 
>> allow the JDK to continue to compile with `-Xlint:all`.
>> 
>> A 'this' escape warning is generated for a constructor `A()` in a class `A` 
>> when the compiler detects that the following situation is _in theory 
>> possible:_
>> * Some subclass `B extends A` exists, and `B` is defined in a separate 
>> source file (i.e., compilation unit)
>> * Some constructor `B()` of `B` invokes `A()` as its superclass constructor
>> * During the execution of `A()`, some non-static method of `B.foo()` could 
>> get invoked, perhaps indirectly
>> 
>> In the above scenario, `B.foo()` would execute before `A()` has returned and 
>> before `B()` has performed any initialization. To the extent `B.foo()` 
>> accesses any fields of `B` - all of which are still uninitialized - it is 
>> likely to function incorrectly.
>> 
>> Note, when determining if a 'this' escape is possible, the compiler makes no 
>> assumptions about code outside of the current compilation unit. It doesn't 
>> look outside of the current source file to see what might actually happen 
>> when a method is invoked. It does follow method and constructors within the 
>> current compilation unit, and applies a simplified 
>> union-of-all-possible-branches data flow analysis to see where 'this' could 
>> end up.
>> 
>> From my review, virtually all of the warnings generated in the various JDK 
>> modules are valid warnings in the sense that a 'this' escape, as defined 
>> above, is really and truly possible. However, that doesn't imply that any 
>> bugs were found within the JDK - only that the possibility of a certain type 
>> of bug exists if certain superclass constructors are used by someone, 
>> somewhere, someday.
>> 
>> For several "popular" classes, this PR also adds `@implNote`'s to the 
>> offending constructors so that subclass implementors are made aware of the 
>> threat. For one example, `TreeMap(Map)` invokes `putAll()` and `put()`.
>> 
>> More details and a couple of motivating examples are given in an included 
>> [doc 
>> file](https://github.com/archiecobbs/jdk/blob/ThisEscape/src/java.base/share/classes/java/lang/doc-files/ThisEscape.html)
>>  that these `@implNote`'s link to. See also the recent thread on `amber-dev` 
>> for some background.
>> 
>> Ideally, over time the owners of the various modules would review their 
>> `@SuppressWarnings("this-escape")` annotations and determine which other 
>> constructors also warranted such an `@implNote`.
>> 
>> Because of all the`@SuppressWarnings` annotations, this PR touches a bunch 
>> of different JDK modules. My apologies for that. Adding these annotations 
>> was determined to be the more conservative approach, as compared to just 
>> excepting `this-escape` from various module builds globally.
>> 
>> **Patch Navigation Guide**
>> 
>> * Non-trivial compiler changes:
>> * `src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java`
>> * `src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java`
>> * `src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java`
>> * `src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java`
>> * 
>> `src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java`
>> * 
>> `src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties`
>> * 
>> `src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties`
>> 
>> * Javadoc additions of `@implNote`:
>> 
>> * `src/java.base/share/classes/java/io/PipedReader.java`
>> * `src/java.base/share/classes/java/io/PipedWriter.java`
>> * `src/java.base/share/classes/java/lang/Throwable.java`
>> * `src/java.base/share/classes/java/util/ArrayDeque.java`
>> * `src/java.base/share/classes/java/util/EnumMap.java`
>> * `src/java.base/share/classes/java/util/HashSet.java`
>> * `src/java.base/share/classes/java/util/Hashtable.java`
>> * `src/java.base/share/classes/java/util/LinkedList.java`
>> * `src/java.base/share/classes/java/util/TreeMap.java`
>> * `src/java.base/share/classes/java/util/TreeSet.java`
>> 
>> * New unit tests
>> * `test/langtools/tools/javac/warnings/ThisEscape/*.java`
>> 
>> * **Everything else** is just adding `@SuppressWarnings("this-escape")`
>
> Archie L. Cobbs has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix incorrect @bug numbers in unit tests.

All your new files need a copyright and GPL header.

-

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


[jdk20] RFR: 8290919: Update nroff pages in JDK 20 before RC

2023-01-22 Thread David Holmes
There was one missing update in javac.1 from:

[JDK-8245246](https://bugs.openjdk.org/browse/JDK-8245246): Deprecate -profile 
option in javac

otherwise the change is limited to dropping the "-ea".

Thanks.

-

Commit messages:
 - Merge branch 'master' into 8290919-manpages
 - 8290919: Update nroff pages in JDK 20 before RC

Changes: https://git.openjdk.org/jdk20/pull/112/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk20&pr=112&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290919
  Stats: 29 lines in 28 files changed: 1 ins; 0 del; 28 mod
  Patch: https://git.openjdk.org/jdk20/pull/112.diff
  Fetch: git fetch https://git.openjdk.org/jdk20 pull/112/head:pull/112

PR: https://git.openjdk.org/jdk20/pull/112


Re: [jdk20] RFR: 8290919: Update nroff pages in JDK 20 before RC

2023-01-22 Thread David Holmes
On Mon, 23 Jan 2023 07:15:08 GMT, Iris Clark  wrote:

>> There was one missing update in javac.1 from:
>> 
>> [JDK-8245246](https://bugs.openjdk.org/browse/JDK-8245246): Deprecate 
>> -profile option in javac
>> 
>> otherwise the change is limited to dropping the "-ea".
>> 
>> Thanks.
>
> Marked as reviewed by iris (Reviewer).

Thanks for the reviews @irisclark  and @AlanBateman !

-

PR: https://git.openjdk.org/jdk20/pull/112


[jdk20] Integrated: 8290919: Update nroff pages in JDK 20 before RC

2023-01-23 Thread David Holmes
On Mon, 23 Jan 2023 02:59:42 GMT, David Holmes  wrote:

> There was one missing update in javac.1 from:
> 
> [JDK-8245246](https://bugs.openjdk.org/browse/JDK-8245246): Deprecate 
> -profile option in javac
> 
> otherwise the change is limited to dropping the "-ea".
> 
> Thanks.

This pull request has now been integrated.

Changeset: fd752178
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk20/commit/fd752178e364fb5deeec062bef3dde1fea1dcbe3
Stats: 29 lines in 28 files changed: 1 ins; 0 del; 28 mod

8290919: Update nroff pages in JDK 20 before RC

Reviewed-by: iris, alanb

-

PR: https://git.openjdk.org/jdk20/pull/112


RFR: 8290918: Initial nroff manpage generation for JDK 21

2023-01-23 Thread David Holmes
Please review this simple update to the manpage to set the version to 21-ea.

This change also corrects a typo in javac.1 that was manually introduced by 
JDK-8300591

Thanks.

-

Commit messages:
 - 8290918: Initial nroff manpage generation for JDK 21

Changes: https://git.openjdk.org/jdk/pull/12154/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12154&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290918
  Stats: 28 lines in 27 files changed: 0 ins; 0 del; 28 mod
  Patch: https://git.openjdk.org/jdk/pull/12154.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12154/head:pull/12154

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


Re: RFR: 8290918: Initial nroff manpage generation for JDK 21

2023-01-23 Thread David Holmes
On Mon, 23 Jan 2023 23:24:06 GMT, Lance Andersen  wrote:

>> Please review this simple update to the manpage to set the version to 21-ea.
>> 
>> This change also corrects a typo in javac.1 that was manually introduced by 
>> JDK-8300591
>> 
>> Thanks.
>
> Marked as reviewed by lancea (Reviewer).

Thanks for the review @LanceAndersen

-

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


Re: RFR: 8290918: Initial nroff manpage generation for JDK 21

2023-01-23 Thread David Holmes
On Tue, 24 Jan 2023 01:28:53 GMT, Iris Clark  wrote:

>> Please review this simple update to the manpage to set the version to 21-ea.
>> 
>> This change also corrects a typo in javac.1 that was manually introduced by 
>> JDK-8300591
>> 
>> Thanks.
>
> Marked as reviewed by iris (Reviewer).

Thanks for the review @irisclark !

-

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


Re: RFR: 8290918: Initial nroff manpage generation for JDK 21

2023-01-23 Thread David Holmes
On Tue, 24 Jan 2023 05:54:44 GMT, Joe Darcy  wrote:

>> Please review this simple update to the manpage to set the version to 21-ea.
>> 
>> This change also corrects a typo in javac.1 that was manually introduced by 
>> JDK-8300591
>> 
>> Thanks.
>
> Marked as reviewed by darcy (Reviewer).

Thanks for the review @jddarcy !

I think that suffices for this trivial change.

-

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


Integrated: 8290918: Initial nroff manpage generation for JDK 21

2023-01-23 Thread David Holmes
On Mon, 23 Jan 2023 22:59:22 GMT, David Holmes  wrote:

> Please review this simple update to the manpage to set the version to 21-ea.
> 
> This change also corrects a typo in javac.1 that was manually introduced by 
> JDK-8300591
> 
> Thanks.

This pull request has now been integrated.

Changeset: 6dd8723f
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk/commit/6dd8723f66a22e626d98c74cff0b0b344a62626d
Stats: 28 lines in 27 files changed: 0 ins; 0 del; 28 mod

8290918: Initial nroff manpage generation for JDK 21

Reviewed-by: lancea, iris, darcy

-

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


Re: RFR: 8303485: Replacing os.name for operating system customization

2023-03-08 Thread David Holmes
On Wed, 8 Mar 2023 19:15:16 GMT, Roger Riggs  wrote:

> Improvements to support OS specific customization for JDK internal use:
>  - To select values and code; allowing elimination of unused code and values
>  - Optionally evaluated by build processes, compilation, or archiving (i.e. 
> CDS)
>  - Simple API to replace adhoc comparisons with `os.name`
>  - Clear and consistent use across build, runtime, and JDK modules
>  
> The PR includes updates within java.base to use the new API.

I guess I'm surprised this hasn't been done long before now. :)

Just a couple of drive by comments (I agree with comments made by others).

Has this totally killed of BSD support on the JDK side? I thought building 
non-macOS BSD was still viable, but perhaps not - certainly not after this 
change.

Thanks

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 48:

> 46:  * For example,
> 47:  * {@snippet lang = "java":
> 48:  * if (OperatingSystem.current() == Windows) {

Doesn't `Windows` need to be prefixed with `OperatingSystem` here? Ditto for 
dispatch example following.

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 105:

> 103:  */
> 104: @ForceInline
> 105: public static boolean isMac() {

suggestion: isMacOS

-

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


Re: RFR: 8303485: Replacing os.name for operating system customization

2023-03-09 Thread David Holmes
On Thu, 9 Mar 2023 16:01:21 GMT, Roger Riggs  wrote:

>> src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 74:
>> 
>>> 72:  * The Mac OS X Operating system.
>>> 73:  */
>>> 74: Mac("Mac OS X"),
>> 
>> The current spelling used by Apple is "macOS", probably to align to other 
>> Apple systems (iOS, iPadOS, etc.).
>
> Names and branding have changed over the years. 
> It may be prudent to remove the name entirely and stick to a 'generic' 
> identification of the OS as the Enum name.

The current branding is "macOS" and we made a lot of changes in various places 
(but not all of course) to ensure that we use "macOS". Even if this can change 
in the future I think starting with the current terminology rather than 
defining something new, would be much better.

-

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


Re: RFR: 8303485: Replacing os.name for operating system customization

2023-03-09 Thread David Holmes
The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Thu, 9 Mar 2023 16:12:26 GMT, Roger Riggs  wrote:

>> src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 105:
>> 
>>> 103:  */
>>> 104: @ForceInline
>>> 105: public static boolean isMac() {
>> 
>> suggestion: isMacOS
>
> As above, sticking to a generic term can make this a bit less sensitive to 
> branding changes.

Lets at least be consistent and not introduce a rage of terms. "mac" is a piece 
of hardware.

-

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


Re: RFR: 8303485: Replacing os.name for operating system customization

2023-03-09 Thread David Holmes
On Thu, 9 Mar 2023 17:12:53 GMT, Roger Riggs  wrote:

>>> I would argue that we should keep the replacement string matching the make 
>>> variable its getting the value from. It makes it easier to trace the origin 
>>> of the value.
>> 
>> Then we can define a new make variable that will also allow it be a 
>> different value than `OPENJDK_TARGET_OS` if desirable.
>
> There is no need to define a new variable, there are already too many in the 
> build and the build and the compiled files should be using the same symbols.

IMO the naming should make sense from the perspective of reading the 
post-processed source file - so the variable should refer to "current" not 
"target".

-

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


Re: RFR: 8303485: Replacing os.name for operating system customization

2023-03-09 Thread David Holmes
On Thu, 9 Mar 2023 17:18:35 GMT, Roger Riggs  wrote:

>> That would not yield a compile time constant.
>> The TARGET_IS_XXX values must evaluate to compile time constants as 
>> evaluated by javac.
>
> Using the naming from the build makes it clearer that there is a dependency 
> between the build names and those in the template.

Don't we have a conditional compilation ability with these template files such 
that we can just generate true or false depending on the OS?

-

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


Re: RFR: 8303485: Replacing os.name for operating system customization

2023-03-09 Thread David Holmes
On Fri, 10 Mar 2023 05:09:49 GMT, Mandy Chung  wrote:

>> Don't we have a conditional compilation ability with these template files 
>> such that we can just generate true or false depending on the OS?
>
> Good point.  `OperatingSystemProps.java` can be a OS-specific class like: 
> 
> src/java.base/macosx/classes/jdk/internal/misc/OperatingSystemProps.java
>   linux/classes/jdk/internal/misc/OperatingSystemProps.java
>   windows/classes/jdk/internal/misc/OperatingSystemProps.java
> 
> 
> OTOH, if we include `os.arch`, that would need to be a template.   A single 
> template file to include both OS and ARCH would do it.

I was literally thinking of the `#if[xx]` conditional compilation facility 
rather than an OS specific file, but okay ...

-

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


Re: RFR: 8303485: Replacing os.name for operating system customization [v6]

2023-03-26 Thread David Holmes

On 25/03/2023 3:06 am, Roger Riggs wrote:

On Fri, 24 Mar 2023 16:53:05 GMT, Michael Osipov  wrote:


If you are referring to "Red Hat Enterprise Linux", it'd be correct (AFAIK) to say that Red Hat identifies 
"Red Hat Enterprise Linux" as an operating system, but I wouldn't go so far as to say that Red Hat calls 
"Linux" an operating system (well... subject to the reality of human inaccuracy). And the validity and 
correctness of the term "GNU/Linux" is *definitely* disputed (and likely always will be).

Maybe "Operating systems based on the Linux kernel" would be satisfactory?


That is acceptable, totally.


Not the rabbit-hole I expected to go down today or for an informal comment on 
an internal API.
I have no skin in that game.
https://www.redhat.com/en/topics/linux/what-is-linux


Also:

https://www.oracle.com/au/linux/

David


RFR: 8304478: Initial nroff manpage generation for JDK 22

2023-06-13 Thread David Holmes
Updated the version to 22-ea and year to 2024.

The following unpublished changes will also be included in this update:
- [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
contains a special character
- [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
- [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) Deprecate 
jdeps -profile option

The following changes were never applied to the closed sources and are "lost" 
by this update. These changes will need to be re-applied directly in JDK 21 and 
JDK 22:
- [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
java.lang.NoClassDefFoundError exception on running fully legitimate code
- [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
for calling overridable methods from a constructor

Thanks.

-

Commit messages:
 - 8304478: Initial nroff manpage generation for JDK 22

Changes: https://git.openjdk.org/jdk/pull/14462/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14462&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8304478
  Stats: 140 lines in 28 files changed: 1 ins; 105 del; 34 mod
  Patch: https://git.openjdk.org/jdk/pull/14462.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14462/head:pull/14462

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


Re: RFR: 8304478: Initial nroff manpage generation for JDK 22

2023-06-13 Thread David Holmes
On Wed, 14 Jun 2023 06:08:59 GMT, Alan Bateman  wrote:

>> Updated the version to 22-ea and year to 2024.
>> 
>> The following unpublished changes will also be included in this update:
>> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool 
>> manpage contains a special character
>> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
>> jarsigner man page after 
>> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
>> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
>> Deprecate jdeps -profile option
>> 
>> The following changes, to `javac.1`, were never applied to the closed 
>> sources and are "lost" by this update. These changes will need to be 
>> re-applied directly in JDK 21 and JDK 22:
>> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
>> java.lang.NoClassDefFoundError exception on running fully legitimate code
>> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
>> for calling overridable methods from a constructor
>> 
>> Thanks.
>
> Looks fine.

Thanks @AlanBateman !

-

PR Comment: https://git.openjdk.org/jdk/pull/14462#issuecomment-1590531861


Re: RFR: 8304478: Initial nroff manpage generation for JDK 22

2023-06-14 Thread David Holmes
On Wed, 14 Jun 2023 19:21:01 GMT, Archie Cobbs  wrote:

> Just curious, since you have access to the secret closed sources, can you not 
> backport these changes yourself? Instead of just deleting them and expecting 
> someone else to rescue them from oblivion?

@archiecobbs  we (Oracle) will take care of restoring this text so please don't 
be concerned about that. It will just be a temporary glitch. It should have 
been handled when the original PRs were done. It needs to be handled as a 
separate issue and PR though - whether that were to happen before or after this 
PR is somewhat immaterial.

-

PR Comment: https://git.openjdk.org/jdk/pull/14462#issuecomment-1592014026


Re: RFR: 8304478: Initial nroff manpage generation for JDK 22

2023-06-14 Thread David Holmes
On Wed, 14 Jun 2023 09:25:14 GMT, Serguei Spitsyn  wrote:

>> Updated the version to 22-ea and year to 2024.
>> 
>> The following unpublished changes will also be included in this update:
>> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool 
>> manpage contains a special character
>> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
>> jarsigner man page after 
>> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
>> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
>> Deprecate jdeps -profile option
>> 
>> The following changes, to `javac.1`, were never applied to the closed 
>> sources and are "lost" by this update. These changes will need to be 
>> re-applied directly in JDK 21 and JDK 22:
>> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
>> java.lang.NoClassDefFoundError exception on running fully legitimate code
>> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
>> for calling overridable methods from a constructor
>> 
>> Thanks.
>
> The Serviceability changes look good.
> Thanks,
> Serguei

Thanks for the additional reviews @sspitsyn , @LanceAndersen  and @mlchung !

-

PR Comment: https://git.openjdk.org/jdk/pull/14462#issuecomment-1592014943


Re: RFR: 8304478: Initial nroff manpage generation for JDK 22

2023-06-14 Thread David Holmes
On Wed, 14 Jun 2023 04:53:58 GMT, David Holmes  wrote:

> Updated the version to 22-ea and year to 2024.
> 
> The following unpublished changes will also be included in this update:
> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
> jarsigner man page after 
> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> The following changes, to `javac.1`, were never applied to the closed sources 
> and are "lost" by this update. These changes will need to be re-applied 
> directly in JDK 21 and JDK 22:
> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
> java.lang.NoClassDefFoundError exception on running fully legitimate code
> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
> for calling overridable methods from a constructor
> 
> Thanks.

I've filed https://bugs.openjdk.org/browse/JDK-8310067 for the javac manpage 
fixup.

-

PR Comment: https://git.openjdk.org/jdk/pull/14462#issuecomment-1592056009


Re: RFR: 8304478: Initial nroff manpage generation for JDK 22 [v2]

2023-06-19 Thread David Holmes
> Updated the version to 22-ea and year to 2024.
> 
> The following unpublished changes will also be included in this update:
> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
> jarsigner man page after 
> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> The following changes, to `javac.1`, were never applied to the closed sources 
> and are "lost" by this update. These changes will need to be re-applied 
> directly in JDK 21 and JDK 22:
> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
> java.lang.NoClassDefFoundError exception on running fully legitimate code
> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
> for calling overridable methods from a constructor
> 
> Thanks.

David Holmes has updated the pull request incrementally with one additional 
commit since the last revision:

  Pick up javac and jshell changes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14462/files
  - new: https://git.openjdk.org/jdk/pull/14462/files/3efc518f..e8ff96d1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14462&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14462&range=00-01

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

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


Re: RFR: 8304478: Initial nroff manpage generation for JDK 22

2023-06-19 Thread David Holmes
On Wed, 14 Jun 2023 04:53:58 GMT, David Holmes  wrote:

> Updated the version to 22-ea and year to 2024.
> 
> The following unpublished changes will also be included in this update:
> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
> jarsigner man page after 
> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> The following changes, to `javac.1`, were never applied to the closed sources 
> and are "lost" by this update. These changes will need to be re-applied 
> directly in JDK 21 and JDK 22:
> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
> java.lang.NoClassDefFoundError exception on running fully legitimate code
> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
> for calling overridable methods from a constructor
> 
> Thanks.

The javac changes have been restored so I am merging them into this PR - the 
only difference should be minor formatting changes.

This has also now picked up the jshell changes from JDK-8308988.

-

PR Comment: https://git.openjdk.org/jdk/pull/14462#issuecomment-1597836257


Integrated: 8304478: Initial nroff manpage generation for JDK 22

2023-06-19 Thread David Holmes
On Wed, 14 Jun 2023 04:53:58 GMT, David Holmes  wrote:

> Updated the version to 22-ea and year to 2024.
> 
> The following unpublished changes will also be included in this update:
> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
> jarsigner man page after 
> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> The following changes, to `javac.1`, were never applied to the closed sources 
> and are "lost" by this update. These changes will need to be re-applied 
> directly in JDK 21 and JDK 22:
> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
> java.lang.NoClassDefFoundError exception on running fully legitimate code
> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
> for calling overridable methods from a constructor
> 
> Thanks.

This pull request has now been integrated.

Changeset: b2e86aef
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk/commit/b2e86aef65f4d579896b6db83aaad408b6c580d4
Stats: 129 lines in 28 files changed: 24 ins; 57 del; 48 mod

8304478: Initial nroff manpage generation for JDK 22

Reviewed-by: alanb, sspitsyn, mchung, lancea

-

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


Re: RFR: 8304478: Initial nroff manpage generation for JDK 22

2023-06-19 Thread David Holmes
On Wed, 14 Jun 2023 04:53:58 GMT, David Holmes  wrote:

> Updated the version to 22-ea and year to 2024.
> 
> The following unpublished changes will also be included in this update:
> - [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> - [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update 
> jarsigner man page after 
> [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> - [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> The following changes, to `javac.1`, were never applied to the closed sources 
> and are "lost" by this update. These changes will need to be re-applied 
> directly in JDK 21 and JDK 22:
> - [JDK-8296656](https://bugs.openjdk.org/browse/JDK-8296656): 
> java.lang.NoClassDefFoundError exception on running fully legitimate code
> - [JDK-8015831](https://bugs.openjdk.org/browse/JDK-8015831): Add lint check 
> for calling overridable methods from a constructor
> 
> Thanks.

The javac update has also now pulled in JDK-8308456

-

PR Comment: https://git.openjdk.org/jdk/pull/14462#issuecomment-1597839767


Re: RFR: 8311043: Remove trailing blank lines in source files

2023-06-29 Thread David Holmes
On Wed, 28 Jun 2023 16:54:51 GMT, Leo Korinth  wrote:

> Remove trailing "blank" lines in source files.
> 
> I like to use global-whitespace-cleanup-mode, but I can not use it if the 
> files are "dirty" to begin with. This fix will make more files "clean". I 
> also considered adding a check for this in jcheck for Skara, however it seems 
> jcheck code handling hunks does not track end-of-files. Thus I will only 
> clean the files.
> 
> The fix removes trailing lines matching ^[[:space:]]*$ in
> 
> - *.java
> - *.cpp
> - *.hpp
> - *.c
> - *.h 
> 
> I have applied the following bash script to each file:
> 
> file="$1"
> 
> while [[ $(tail -n 1 "$file") =~ ^[[:space:]]*$ ]]; do
> truncate -s -1 "$file"
> done
> 
> `git diff --ignore-space-change --ignore-blank-lines  master` displays no 
> changes
> `git diff --ignore-blank-lines  master` displays one change

This seems to run contrary to the requirement that files end in a newline, 
which git will complain about if the newline is missing.

It also seems far too large and disruptive.

-

PR Comment: https://git.openjdk.org/jdk/pull/14698#issuecomment-1612567676


Re: RFR: 8311043: Remove trailing blank lines in source files

2023-06-29 Thread David Holmes
On Wed, 28 Jun 2023 16:54:51 GMT, Leo Korinth  wrote:

> Remove trailing "blank" lines in source files.
> 
> I like to use global-whitespace-cleanup-mode, but I can not use it if the 
> files are "dirty" to begin with. This fix will make more files "clean". I 
> also considered adding a check for this in jcheck for Skara, however it seems 
> jcheck code handling hunks does not track end-of-files. Thus I will only 
> clean the files.
> 
> The fix removes trailing lines matching ^[[:space:]]*$ in
> 
> - *.java
> - *.cpp
> - *.hpp
> - *.c
> - *.h 
> 
> I have applied the following bash script to each file:
> 
> file="$1"
> 
> while [[ $(tail -n 1 "$file") =~ ^[[:space:]]*$ ]]; do
> truncate -s -1 "$file"
> done
> 
> `git diff --ignore-space-change --ignore-blank-lines  master` displays no 
> changes
> `git diff --ignore-blank-lines  master` displays one change

Neither the PR diffs nor the webrev make it easy to see exactly what is being 
changed here. It appeared to me that the last empty line of each file was being 
deleted, leaving no newline at the end.

But to me this is too disruptive with no tangible benefit. And you need buy-in 
from all the different areas affected by this.

-

PR Comment: https://git.openjdk.org/jdk/pull/14698#issuecomment-1613043398


Re: RFR: 8311043: Remove trailing blank lines in source files

2023-06-29 Thread David Holmes
On Thu, 29 Jun 2023 13:05:58 GMT, Leo Korinth  wrote:

> My changes look like this in the diff output
> ```
>  }
> -
> ```

Thanks for showing  this and other output. To me this looked like the final 
newline had been removed. I would have expected to see something that more 
obviously showed more than one blank line before hand and exactly one after.

-

PR Comment: https://git.openjdk.org/jdk/pull/14698#issuecomment-1613985636


[jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread David Holmes
Main changes are to use 21 instead of 21-ea. In addition the following files 
contain additional updates from the closed sources:

- src/java.base/share/man/java.1

 [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
manpage markdown source for JFR filename expansion
 [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
StartFlightRecording: consider moving mention of multiple comma-separated 
parameters to the front

There are also some formatting differences related to:

 [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help output 
for --module-path / -p is incomplete

Likely a different version of pandoc was used.

- src/java.base/share/man/keytool.1

 [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
contains a special character

- src/jdk.compiler/share/man/javac.1

 [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac tool 
page for -proc:full

also some formatting differences.

- src/jdk.jartool/share/man/jarsigner.1

 [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)

- src/jdk.jcmd/share/man/jcmd.1

 [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
manpage markdown source for JFR filename expansion

- src/jdk.jdeps/share/man/jdeps.1

 [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) Deprecate 
jdeps -profile option

- src/jdk.jfr/share/man/jfr.1

Formatting changes.

- src/jdk.jshell/share/man/jshell.1

 [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
manpage to include TOOLING description

-

Commit messages:
 - 8300937: Update nroff pages in JDK 21 before RC

Changes: https://git.openjdk.org/jdk21/pull/151/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=151&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8300937
  Stats: 163 lines in 28 files changed: 37 ins; 60 del; 66 mod
  Patch: https://git.openjdk.org/jdk21/pull/151.diff
  Fetch: git fetch https://git.openjdk.org/jdk21.git pull/151/head:pull/151

PR: https://git.openjdk.org/jdk21/pull/151


Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread David Holmes
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

Thanks for the reviews everyone.

-

PR Comment: https://git.openjdk.org/jdk21/pull/151#issuecomment-1659164663


[jdk21] Integrated: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread David Holmes
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

This pull request has now been integrated.

Changeset: 0439d584
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk21/commit/0439d584221f4d92fd1f4097a331f83dcdb2c12c
Stats: 163 lines in 28 files changed: 37 ins; 60 del; 66 mod

8300937: Update nroff pages in JDK 21 before RC

Reviewed-by: mchung, iris, egahlin

-

PR: https://git.openjdk.org/jdk21/pull/151


Re: RFR: JDK-8313764: Offer JVM HS functionality to shared lib load operations done by the JDK codebase [v2]

2023-08-27 Thread David Holmes
On Wed, 23 Aug 2023 15:18:03 GMT, Matthias Baesken  wrote:

>> Currently there is a number of functionality that would be interesting to 
>> have for shared lib load operations in the JDK C code.
>> Some examples :
>> Events::log_dll_message for hs-err files reporting
>> JFR event NativeLibraryLoad
>> There is the need to update the shared lib Cache on AIX ( see 
>> LoadedLibraries::reload() , see also 
>> https://bugs.openjdk.org/browse/JDK-8314152 ),
>> this is currently not fully in sync with libs loaded form jdk c-libs and 
>> sometimes reports outdated information
>> 
>> Offer an interface (e.g. jvm.cpp) to support this.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   windows aarch64 build issues

Sorry but looking at the hotspot part of this I do not like the code in jvm.cpp 
at all - it is  far too messy. I expected to see a simple interface to 
os::dlopen which then handles all the platform specific issues.

I'm also somewhat dubious about having all the JDK code use a VM interface for 
this, The usual coupling to the VM is to just provide native implementations of 
core library methods, not to have the VM provide a general purpose utility 
interface like dynamic library loading.

-

PR Comment: https://git.openjdk.org/jdk/pull/15264#issuecomment-1694907802


Re: RFR: 8267174: Many test files have the wrong Copyright header

2023-09-11 Thread David Holmes
On Tue, 5 Sep 2023 22:49:41 GMT, Erik Joelsson  wrote:

> There are a number of files in the `test` directory that have an incorrect 
> copyright header, which includes the "classpath" exception text. This patch 
> removes that text from all test files that I could find it in. I did this 
> using a combination of `sed` and `grep`. Reviewing this patch is probably 
> easier using the raw patch file or a suitable webrev format.
> 
> It's my assumption that these headers were introduced by mistake as it's 
> quite easy to copy the wrong template when creating new files.

Looks good. I have often pointed out that the CPE was not relevant for test 
files but ...

Thanks

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15573#pullrequestreview-1619274864


Re: RFR: JDK-8313764: Offer JVM HS functionality to shared lib load operations done by the JDK codebase [v2]

2023-10-17 Thread David Holmes
On Mon, 16 Oct 2023 15:04:51 GMT, Matthias Baesken  wrote:

>> Matthias Baesken has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   windows aarch64 build issues
>
> Hello, any comments about the idea of calling into 'os::dll_load' instead ? 
> That would indeed make the coding smaller and less 'messy' .

@MBaesken I'm not at all sure what it would look like - sorry. But there 
doesn't seem to be any general support from the library folk for this.

-

PR Comment: https://git.openjdk.org/jdk/pull/15264#issuecomment-1767673904


RFR: 8322065: Initial nroff manpage generation for JDK 23

2023-12-13 Thread David Holmes
Updated the version to 23-ea and year to 2024.

This initial generation also picks up the unpublished changes from:

- [JDK-8302233](https://bugs.openjdk.org/browse/JDK-8302233) (keytool & 
jarsigner)
- [JDK-8290702](https://bugs.openjdk.org/browse/JDK-8290702) (javadoc) (JDK 23 
backport)
- [JDK-8321384](https://bugs.openjdk.org/browse/JDK-8321384) (javadoc)


In addition this includes the updates for

- [JDK-8309981](https://bugs.openjdk.org/browse/8309981) Remove expired flags 
in JDK 23

Thanks

-

Commit messages:
 - 8322065: Initial nroff manpage generation for JDK 23
 - 8309981: Remove expired flags in JDK 23

Changes: https://git.openjdk.org/jdk/pull/17101/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17101&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8322065
  Stats: 216 lines in 29 files changed: 47 ins; 61 del; 108 mod
  Patch: https://git.openjdk.org/jdk/pull/17101.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17101/head:pull/17101

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


Re: RFR: 8322065: Initial nroff manpage generation for JDK 23

2023-12-14 Thread David Holmes
On Thu, 14 Dec 2023 09:17:05 GMT, Pavel Rappo  wrote:

> Thanks for doing this, David. I only note that the changes for JDK-8321384 
> were published in [JDK-8308715](https://bugs.openjdk.org/browse/JDK-8308715), 
> which was integrated in the mainline before JDK 22 RDP 1. So they are already 
> present in the mainline.

Ah I see. Thanks for correcting that, I will update the PR and JBS issue.

And thanks for looking at this @pavelrappo .

-

PR Comment: https://git.openjdk.org/jdk/pull/17101#issuecomment-1855755042


Re: RFR: 8322065: Initial nroff manpage generation for JDK 23

2023-12-14 Thread David Holmes
On Thu, 14 Dec 2023 09:01:17 GMT, Alan Bateman  wrote:

> Initially I wondered if JDK-8309981 should be separated but include keeps 
> things in sync so I think okay.

Thanks for the review @AlanBateman .

Yeah I was in two minds there myself. I started fixing 
[JDK-8309981](https://bugs.openjdk.org/browse/JDK-8309981) only to discover 
that the start of release updates had not been done as part of the start of 
release, so I figured I may as well fix it all together given I'd generated all 
the updated files anyway. But I'm still a little unsure ... in fact I think I 
will remove it in the morning.

-

PR Comment: https://git.openjdk.org/jdk/pull/17101#issuecomment-1855761906


Re: RFR: 8322065: Initial nroff manpage generation for JDK 23 [v2]

2023-12-14 Thread David Holmes
> Updated the version to 23-ea and year to 2024.
> 
> This initial generation also picks up the unpublished changes from:
> 
> - [JDK-8302233](https://bugs.openjdk.org/browse/JDK-8302233) (keytool & 
> jarsigner)
> - [JDK-8290702](https://bugs.openjdk.org/browse/JDK-8290702) (javadoc) (JDK 
> 23 backport)
> 
> Thanks

David Holmes has updated the pull request incrementally with one additional 
commit since the last revision:

  Revert "8309981: Remove expired flags in JDK 23"
  
  This reverts commit 0324a90e936ae01e42ae099e7235156326cc318a.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17101/files
  - new: https://git.openjdk.org/jdk/pull/17101/files/65a8c9ed..8b052141

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17101&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17101&range=00-01

  Stats: 23 lines in 2 files changed: 10 ins; 11 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/17101.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17101/head:pull/17101

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


Integrated: 8322065: Initial nroff manpage generation for JDK 23

2023-12-14 Thread David Holmes
On Thu, 14 Dec 2023 05:46:01 GMT, David Holmes  wrote:

> Updated the version to 23-ea and year to 2024.
> 
> This initial generation also picks up the unpublished changes from:
> 
> - [JDK-8302233](https://bugs.openjdk.org/browse/JDK-8302233) (keytool & 
> jarsigner)
> - [JDK-8290702](https://bugs.openjdk.org/browse/JDK-8290702) (javadoc) (JDK 
> 23 backport)
> 
> Thanks

This pull request has now been integrated.

Changeset: 692be577
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk/commit/692be577385844bf00a01ff10e390e014191569f
Stats: 193 lines in 27 files changed: 36 ins; 51 del; 106 mod

8322065: Initial nroff manpage generation for JDK 23

Reviewed-by: alanb

-

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


[jdk22] RFR: 8322066: Update troff manpages in JDK 22 before RC

2024-02-04 Thread David Holmes
This update drops the "ea" from the version string.

We also propagate the following fixes from the markdown sources to the troff 
manpage file:

JDK-8322478: Update java manpage for multi-file source-code launcher
JDK-8302233: HSS/LMS: keytool and jarsigner changes
JDK-8318971: Better Error Handling for Jar Tool When Processing Non-existent 
Files


Thanks.

-

Commit messages:
 - 8322066: Update troff pages in JDK 22 before RC

Changes: https://git.openjdk.org/jdk22/pull/104/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk22&pr=104&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8322066
  Stats: 160 lines in 28 files changed: 63 ins; 15 del; 82 mod
  Patch: https://git.openjdk.org/jdk22/pull/104.diff
  Fetch: git fetch https://git.openjdk.org/jdk22.git pull/104/head:pull/104

PR: https://git.openjdk.org/jdk22/pull/104


Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-04 Thread David Holmes
On Mon, 5 Feb 2024 03:07:43 GMT, Kim Barrett  wrote:

> I consider the "format '%p' expects argument of type 'void*" warnings to be 
> not at all helpful. Fortunately we don't use '%p' in HotSpot,

We do use it in hotspot. Not a huge amount as we have the legacy format 
specifiers for PTR_FORMAT etc.

-

PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926298655


Re: [jdk22] RFR: 8322066: Update troff manpages in JDK 22 before RC

2024-02-05 Thread David Holmes
On Mon, 5 Feb 2024 21:58:23 GMT, Joe Darcy  wrote:

>> This update drops the "ea" from the version string.
>> 
>> We also propagate the following fixes from the markdown sources to the troff 
>> manpage file:
>> 
>> JDK-8322478: Update java manpage for multi-file source-code launcher
>> JDK-8302233: HSS/LMS: keytool and jarsigner changes
>> JDK-8318971: Better Error Handling for Jar Tool When Processing Non-existent 
>> Files
>> 
>> 
>> Thanks.
>
> Marked as reviewed by darcy (Reviewer).

Thanks for the reviews @jddarcy  and @irisclark !

-

PR Comment: https://git.openjdk.org/jdk22/pull/104#issuecomment-1928783920


[jdk22] Integrated: 8322066: Update troff manpages in JDK 22 before RC

2024-02-05 Thread David Holmes
On Sun, 4 Feb 2024 22:43:28 GMT, David Holmes  wrote:

> This update drops the "ea" from the version string.
> 
> We also propagate the following fixes from the markdown sources to the troff 
> manpage file:
> 
> JDK-8322478: Update java manpage for multi-file source-code launcher
> JDK-8302233: HSS/LMS: keytool and jarsigner changes
> JDK-8318971: Better Error Handling for Jar Tool When Processing Non-existent 
> Files
> 
> 
> Thanks.

This pull request has now been integrated.

Changeset: ac7a3c00
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk22/commit/ac7a3c00bbfde173ced08d8745b308bc0ac9649b
Stats: 160 lines in 28 files changed: 63 ins; 15 del; 82 mod

8322066: Update troff manpages in JDK 22 before RC

Reviewed-by: darcy, iris

-

PR: https://git.openjdk.org/jdk22/pull/104


Re: RFR: 8325163: Enable -Wpedantic on clang

2024-02-05 Thread David Holmes
On Mon, 5 Feb 2024 15:42:40 GMT, Kim Barrett  wrote:

> I thought we didn't, because we were instead supposed to use INTPTR_FORMAT and
the (currently?) equivalent PTR_FORMAT. Those macros aren't legacy, they are
to provide consistent output across platforms. "%p" provides implementation
defined output. 

My understanding/recollection was that those issues with `%p` had gone away and 
so we could start using it.

-

PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1928814503


Re: RFR: 8325163: Enable -Wpedantic on clang [v2]

2024-02-05 Thread David Holmes
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie  wrote:

>> Inspired by (the later backed-out) 
>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to 
>> enable `-Wpedantic` for clang. This has already found some irregularities in 
>> the code, like mistakenly using `#import` instead of `#include`. In this 
>> patch, I disable warnings for these individual buggy or badly written files, 
>> but I intend to post follow-up issues on the respective teams to have them 
>> properly fixed.
>> 
>> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since 
>> individual warnings in `-Wpedantic` cannot be disabled. This means that code 
>> like this:
>> 
>> 
>> #define DEBUG_ONLY(code) code;
>> 
>> DEBUG_ONLY(foo());
>> 
>> 
>> will result in a `; ;`. This breaks the C standard, but is benign, and we 
>> use it all over the place. On clang, we can ignore this by 
>> `-Wno-extra-semi`, but this is not available on gcc.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   FIx dtrace build

I think given the warnings have been enabled globally, but disabled locally 
where they cause build failures, and JBS issues will be filed for each of those 
special cases, then this is a reasonable change to make. It is then up to 
component teams to fix code where applicable, and enable disabled warnings in 
the future.

-

PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1928824757


Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v3]

2024-03-06 Thread David Holmes
On Thu, 7 Mar 2024 03:00:11 GMT, Guoxiong Li  wrote:

>> Matthias Baesken has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Introduce windows jni_util_md.h
>
> src/java.base/aix/native/libnio/ch/Pollset.c line 3:
> 
>> 1: /*
>> 2:  * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights 
>> reserved.
>> 3:  * Copyright (c) 2012, 2024 SAP SE. All rights reserved.
> 
> It seems you only need to update the copyright of the company you work for.

Oracle requests/requires that the Oracle copyright always be updated when a 
file is modified.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18132#discussion_r1515660944


Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v4]

2024-03-07 Thread David Holmes
On Thu, 7 Mar 2024 08:16:26 GMT, Matthias Baesken  wrote:

>> We define the RESTARTABLE macro again and again at a lot of places in the 
>> JDK native codebase. This could be centralized to avoid repeating it again 
>> and again !
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   adjust COPYRIGHT year info

LGTM. Thanks

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18132#pullrequestreview-1922263102


Re: RFR: 8330205: Initial troff manpage generation for JDK 24

2024-05-09 Thread David Holmes
On Tue, 7 May 2024 11:53:19 GMT, Pavel Rappo  wrote:

> Please review this mechanical change to man pages. This PR should be 
> integrated after https://github.com/openjdk/jdk/pull/18787.

I think we may have to restore this to a standalone start-of-release change 
done after the other start-of-release changes. I had forgotten about the need 
to update the flags sections of java.1

src/java.base/share/man/java.1 line 3856:

> 3854: .SH REMOVED JAVA OPTIONS
> 3855: .PP
> 3856: These \f[V]java\f[R] options have been removed in JDK 24 and using them

This is incorrect. You can't just change 23 to 24 here as the actual set of 
flags listed below will be will be different.

-

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19119#pullrequestreview-2047411800
PR Review Comment: https://git.openjdk.org/jdk/pull/19119#discussion_r1595128505


Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v4]

2024-05-14 Thread David Holmes
On Tue, 14 May 2024 18:10:28 GMT, Maurizio Cimadamore  
wrote:

>> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting 
>> the use of JNI in the following ways:
>> 
>> * `System::load` and `System::loadLibrary` are now restricted methods
>> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods
>> * binding a JNI `native` method declaration to a native implementation is 
>> now considered a restricted operation
>> 
>> This PR slightly changes the way in which the JDK deals with restricted 
>> methods, even for FFM API calls. In Java 22, the single 
>> `--enable-native-access` was used both to specify a set of modules for which 
>> native access should be allowed *and* to specify whether illegal native 
>> access (that is, native access occurring from a module not specified by 
>> `--enable-native-access`) should be treated as an error or a warning. More 
>> specifically, an error is only issued if the `--enable-native-access flag` 
>> is used at least once.
>> 
>> Here, a new flag is introduced, namely 
>> `illegal-native-access=allow/warn/deny`, which is used to specify what 
>> should happen when access to a restricted method and/or functionality is 
>> found outside the set of modules specified with `--enable-native-access`. 
>> The default policy is `warn`, but users can select `allow` to suppress the 
>> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This 
>> aligns the treatment of restricted methods with other mechanisms, such as 
>> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`.
>> 
>> Some changes were required in the package-info javadoc for 
>> `java.lang.foreign`, to reflect the changes in the command line flags 
>> described above.
>
> Maurizio Cimadamore has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Address review comments
>Improve warning for JNI methods, similar to what's described in JEP 472
>Beef up tests
>  - Address review comments

Hotspot changes look good - notwithstanding discussion about properlty 
namespace placement. Manpage changes also look good.

-

PR Review: https://git.openjdk.org/jdk/pull/19213#pullrequestreview-2056696636


Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v3]

2024-05-14 Thread David Holmes
On Mon, 13 May 2024 15:32:27 GMT, Alan Bateman  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - Fix another typo
>>  - Fix typo
>>  - Add more comments
>
> src/hotspot/share/runtime/arguments.cpp line 2271:
> 
>> 2269: } else if (match_option(option, "--illegal-native-access=", 
>> &tail)) {
>> 2270:   if (!create_module_property("jdk.module.illegal.native.access", 
>> tail, InternalProperty)) {
>> 2271: return JNI_ENOMEM;
> 
> I think it would be helpful to get guidance on if this is the right way to 
> add this system property, only because this one not a "module property". The 
> configuration (WriteableProperty + InternalProperty) look right.

So my recollection/understanding is that we use this mechanism to convert 
module-related `--` flags passed to the VM into system properties that the Java 
code can then read, but we set them up such that you are not allowed to specify 
them directly via `-D`. Is the question here whether this new property should 
be in the `jdk.module` namespace?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1600819327


RFR: 8330205: Initial troff manpage generation for JDK 24

2024-06-09 Thread David Holmes
Sets the version to 24/24-ea and the copyright year to 2025.

Content changes related to the start of release (e.g. for removed options in 
java.1) are handled separately.

This initial generation also picks up the unpublished changes from: 

- JDK-8330807: Update Manpage for obsoletion of ScavengeBeforeFullGC
- JDK-8284500: Typo in Supported Named Extensions - BasicContraints
- JDK-8324342: Doclet should default @since for a nested class to that of its 
enclosing class

Thanks

-

Commit messages:
 - 8330205: Initial troff manpage generation for JDK 24

Changes: https://git.openjdk.org/jdk/pull/19617/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19617&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8330205
  Stats: 66 lines in 28 files changed: 12 ins; 13 del; 41 mod
  Patch: https://git.openjdk.org/jdk/pull/19617.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19617/head:pull/19617

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


Re: RFR: 8330205: Initial troff manpage generation for JDK 24

2024-06-10 Thread David Holmes
On Mon, 10 Jun 2024 07:15:51 GMT, Alan Bateman  wrote:

>> Sets the version to 24/24-ea and the copyright year to 2025.
>> 
>> Content changes related to the start of release (e.g. for removed options in 
>> java.1) are handled separately.
>> 
>> This initial generation also picks up the unpublished changes from: 
>> 
>> - JDK-8330807: Update Manpage for obsoletion of ScavengeBeforeFullGC
>> - JDK-8284500: Typo in Supported Named Extensions - BasicContraints
>> - JDK-8324342: Doclet should default `@since` for a nested class to that of 
>> its enclosing class
>> 
>> Thanks
>
> Marked as reviewed by alanb (Reviewer).

Thanks for the review @AlanBateman !

-

PR Comment: https://git.openjdk.org/jdk/pull/19617#issuecomment-2157606095


Re: RFR: 8330205: Initial troff manpage generation for JDK 24 [v2]

2024-06-10 Thread David Holmes
> Sets the version to 24/24-ea and the copyright year to 2025.
> 
> Content changes related to the start of release (e.g. for removed options in 
> java.1) are handled separately.
> 
> This initial generation also picks up the unpublished changes from: 
> 
> - JDK-8330807: Update Manpage for obsoletion of ScavengeBeforeFullGC
> - JDK-8284500: Typo in Supported Named Extensions - BasicContraints
> - JDK-8324342: Doclet should default `@since` for a nested class to that of 
> its enclosing class
> 
> Thanks

David Holmes has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains two commits:

 - Merge
 - 8330205: Initial troff manpage generation for JDK 24

-

Changes: https://git.openjdk.org/jdk/pull/19617/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19617&range=01
  Stats: 53 lines in 28 files changed: 12 ins; 3 del; 38 mod
  Patch: https://git.openjdk.org/jdk/pull/19617.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19617/head:pull/19617

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


Re: RFR: 8330205: Initial troff manpage generation for JDK 24 [v3]

2024-06-10 Thread David Holmes
> Sets the version to 24/24-ea and the copyright year to 2025.
> 
> Content changes related to the start of release (e.g. for removed options in 
> java.1) are handled separately.
> 
> This initial generation also picks up the unpublished changes from: 
> 
> - JDK-8330807: Update Manpage for obsoletion of ScavengeBeforeFullGC
> - JDK-8284500: Typo in Supported Named Extensions - BasicContraints
> - JDK-8324342: Doclet should default `@since` for a nested class to that of 
> its enclosing class
> 
> Thanks

David Holmes has updated the pull request incrementally with one additional 
commit since the last revision:

  Regenerated after merge

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19617/files
  - new: https://git.openjdk.org/jdk/pull/19617/files/8472c47d..e7563087

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19617&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19617&range=01-02

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

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


Re: RFR: 8330205: Initial troff manpage generation for JDK 24 [v3]

2024-06-10 Thread David Holmes
On Mon, 10 Jun 2024 17:27:18 GMT, Iris Clark  wrote:

>> David Holmes has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Regenerated after merge
>
> Marked as reviewed by iris (Reviewer).

Thanks for the review @irisclark .

I've merged in the latest updates that also brought in part of the missing 
changes. If anything goes wrong I have another PR in preparation that also 
updates java.1

-

PR Comment: https://git.openjdk.org/jdk/pull/19617#issuecomment-2159577044


Integrated: 8330205: Initial troff manpage generation for JDK 24

2024-06-10 Thread David Holmes
On Mon, 10 Jun 2024 02:31:00 GMT, David Holmes  wrote:

> Sets the version to 24/24-ea and the copyright year to 2025.
> 
> Content changes related to the start of release (e.g. for removed options in 
> java.1) are handled separately.
> 
> This initial generation also picks up the unpublished changes from: 
> 
> - ~~JDK-8330807: Update Manpage for obsoletion of ScavengeBeforeFullGC~~ 
> (covered by merge)
> - JDK-8284500: Typo in Supported Named Extensions - BasicContraints
> - JDK-8324342: Doclet should default `@since` for a nested class to that of 
> its enclosing class
> 
> Thanks

This pull request has now been integrated.

Changeset: 3a01b47a
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk/commit/3a01b47ac97714608356ce3faf797c37dc63e9af
Stats: 43 lines in 28 files changed: 2 ins; 3 del; 38 mod

8330205: Initial troff manpage generation for JDK 24

Reviewed-by: alanb, iris

-

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


[jdk23] RFR: 8325280: Update troff manpages in JDK 23 before RC

2024-07-18 Thread David Holmes
Before RC we need to update the man pages in the repo from their Markdown 
sources. All pages at a minimum have 23-ea replaced with 23, and publication 
year set to 2024 if needed.

This also picks up the unpublished changes to java.1 from:

- [JDK-8324836](https://bugs.openjdk.org/browse/JDK-8324836): Update Manpage 
for obsoletion of RAMFraction flags
- [JDK-8330807](https://bugs.openjdk.org/browse/JDK-8330807): Update Manpage 
for obsoletion of ScavengeBeforeFullGC

And a typo crept in to java.1 from:
- [JDK-8331670](https://bugs.openjdk.org/browse/JDK-8331670): Deprecate the 
Memory-Access Methods in sun.misc.Unsafe for Removal

This also picks up the unpublished change to keytool.1 from:

- [JDK-8284500](https://bugs.openjdk.org/browse/JDK-8284500): Typo in Supported 
Named Extensions - BasicContraints

This also picks up the unpublished change to javadoc.1 from:

- [JDK-8324342](https://bugs.openjdk.org/browse/JDK-8324342): Doclet should 
default @since for a nested class to that of its enclosing class

and some formatting changes (unclear why - perhaps pandoc version) from the 
combined changeset for:

- [JDK-8298405](https://bugs.openjdk.org/browse/JDK-8298405): Implement JEP 
467: Markdown Documentation Comments
- [JDK-8329296](https://bugs.openjdk.org/browse/JDK-8329296): Update Elements 
for '///' documentation comments

The javac.1 file has its copyright year reverted to match what is in the source 
file (which should have been updated but wasn't).

Thanks.

-

Commit messages:
 - 8325280: Update troff manpages in JDK 23 before RC

Changes: https://git.openjdk.org/jdk/pull/20248/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20248&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8325280
  Stats: 142 lines in 28 files changed: 51 ins; 52 del; 39 mod
  Patch: https://git.openjdk.org/jdk/pull/20248.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20248/head:pull/20248

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


Re: [jdk23] RFR: 8325280: Update troff manpages in JDK 23 before RC

2024-07-19 Thread David Holmes
On Fri, 19 Jul 2024 05:47:15 GMT, David Holmes  wrote:

> Before RC we need to update the man pages in the repo from their Markdown 
> sources. All pages at a minimum have 23-ea replaced with 23, and publication 
> year set to 2024 if needed.
> 
> This also picks up the unpublished changes to java.1 from:
> 
> - [JDK-8324836](https://bugs.openjdk.org/browse/JDK-8324836): Update Manpage 
> for obsoletion of RAMFraction flags
> - [JDK-8330807](https://bugs.openjdk.org/browse/JDK-8330807): Update Manpage 
> for obsoletion of ScavengeBeforeFullGC
> 
> And a typo crept in to java.1 from:
> - [JDK-8331670](https://bugs.openjdk.org/browse/JDK-8331670): Deprecate the 
> Memory-Access Methods in sun.misc.Unsafe for Removal
> 
> This also picks up the unpublished change to keytool.1 from:
> 
> - [JDK-8284500](https://bugs.openjdk.org/browse/JDK-8284500): Typo in 
> Supported Named Extensions - BasicContraints
> 
> This also picks up the unpublished change to javadoc.1 from:
> 
> - [JDK-8324342](https://bugs.openjdk.org/browse/JDK-8324342): Doclet should 
> default @since for a nested class to that of its enclosing class
> 
> and some formatting changes (unclear why - perhaps pandoc version) from the 
> combined changeset for:
> 
> - [JDK-8298405](https://bugs.openjdk.org/browse/JDK-8298405): Implement JEP 
> 467: Markdown Documentation Comments
> - [JDK-8329296](https://bugs.openjdk.org/browse/JDK-8329296): Update Elements 
> for '///' documentation comments
> 
> The javac.1 file has its copyright year reverted to match what is in the 
> source file (which should have been updated but wasn't).
> 
> Thanks.

Note this is simply a re-generation of the troff files from their sources. If 
there are any problems that need fixing then a new JBS issue has to be filed to 
update the source file and regenerate the troff file.

-

PR Comment: https://git.openjdk.org/jdk/pull/20248#issuecomment-2238248354


[jdk23] Integrated: 8325280: Update troff manpages in JDK 23 before RC

2024-07-21 Thread David Holmes
On Fri, 19 Jul 2024 05:47:15 GMT, David Holmes  wrote:

> Before RC we need to update the man pages in the repo from their Markdown 
> sources. All pages at a minimum have 23-ea replaced with 23, and publication 
> year set to 2024 if needed.
> 
> This also picks up the unpublished changes to java.1 from:
> 
> - [JDK-8324836](https://bugs.openjdk.org/browse/JDK-8324836): Update Manpage 
> for obsoletion of RAMFraction flags
> - [JDK-8330807](https://bugs.openjdk.org/browse/JDK-8330807): Update Manpage 
> for obsoletion of ScavengeBeforeFullGC
> 
> And a typo crept in to java.1 from:
> - [JDK-8331670](https://bugs.openjdk.org/browse/JDK-8331670): Deprecate the 
> Memory-Access Methods in sun.misc.Unsafe for Removal
> 
> This also picks up the unpublished change to keytool.1 from:
> 
> - [JDK-8284500](https://bugs.openjdk.org/browse/JDK-8284500): Typo in 
> Supported Named Extensions - BasicContraints
> 
> This also picks up the unpublished change to javadoc.1 from:
> 
> - [JDK-8324342](https://bugs.openjdk.org/browse/JDK-8324342): Doclet should 
> default @since for a nested class to that of its enclosing class
> 
> and some formatting changes (unclear why - perhaps pandoc version) from the 
> combined changeset for:
> 
> - [JDK-8298405](https://bugs.openjdk.org/browse/JDK-8298405): Implement JEP 
> 467: Markdown Documentation Comments
> - [JDK-8329296](https://bugs.openjdk.org/browse/JDK-8329296): Update Elements 
> for '///' documentation comments
> 
> The javac.1 file has its copyright year reverted to match what is in the 
> source file (which should have been updated but wasn't).
> 
> Thanks.

This pull request has now been integrated.

Changeset: 5473e9e4
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk/commit/5473e9e488c8fc7d99ea9d97fd0e7dd212636546
Stats: 142 lines in 28 files changed: 51 ins; 52 del; 39 mod

8325280: Update troff manpages in JDK 23 before RC

Reviewed-by: alanb, iris

-

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


Re: [jdk23] RFR: 8325280: Update troff manpages in JDK 23 before RC

2024-07-21 Thread David Holmes
On Fri, 19 Jul 2024 08:26:51 GMT, Alan Bateman  wrote:

>> Before RC we need to update the man pages in the repo from their Markdown 
>> sources. All pages at a minimum have 23-ea replaced with 23, and publication 
>> year set to 2024 if needed.
>> 
>> This also picks up the unpublished changes to java.1 from:
>> 
>> - [JDK-8324836](https://bugs.openjdk.org/browse/JDK-8324836): Update Manpage 
>> for obsoletion of RAMFraction flags
>> - [JDK-8330807](https://bugs.openjdk.org/browse/JDK-8330807): Update Manpage 
>> for obsoletion of ScavengeBeforeFullGC
>> 
>> And a typo crept in to java.1 from:
>> - [JDK-8331670](https://bugs.openjdk.org/browse/JDK-8331670): Deprecate the 
>> Memory-Access Methods in sun.misc.Unsafe for Removal
>> 
>> This also picks up the unpublished change to keytool.1 from:
>> 
>> - [JDK-8284500](https://bugs.openjdk.org/browse/JDK-8284500): Typo in 
>> Supported Named Extensions - BasicContraints
>> 
>> This also picks up the unpublished change to javadoc.1 from:
>> 
>> - [JDK-8324342](https://bugs.openjdk.org/browse/JDK-8324342): Doclet should 
>> default @since for a nested class to that of its enclosing class
>> 
>> and some formatting changes (unclear why - perhaps pandoc version) from the 
>> combined changeset for:
>> 
>> - [JDK-8298405](https://bugs.openjdk.org/browse/JDK-8298405): Implement JEP 
>> 467: Markdown Documentation Comments
>> - [JDK-8329296](https://bugs.openjdk.org/browse/JDK-8329296): Update 
>> Elements for '///' documentation comments
>> 
>> The javac.1 file has its copyright year reverted to match what is in the 
>> source file (which should have been updated but wasn't).
>> 
>> Thanks.
>
> Thanks for the tireless effort to update the man pages at the end of each 
> release.

Thanks for the reviews @AlanBateman  and @irisclark !

-

PR Comment: https://git.openjdk.org/jdk/pull/20248#issuecomment-2241805932


Re: RFR: 8342682: Errors related to unused code on Windows after 8339120

2024-10-21 Thread David Holmes
On Mon, 21 Oct 2024 14:34:30 GMT, Julian Waters  wrote:

> and whatever team is responsible for HotSpot debugging.

I don't see anything hotspot related here.

I think you would be better off splitting this up into distinct issues and PRs 
for different areas. I expect the client team in particular would prefer that.

-

PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2428037844


Re: RFR: 8342682: Errors related to unused code on Windows after 8339120

2024-10-22 Thread David Holmes
On Tue, 22 Oct 2024 01:43:50 GMT, Julian Waters  wrote:

> Aren't the dt_shmem and jdwp changes related to HotSpot? 

Nope. That's core-svc - the non-hotspot side of serviceability. :)

-

PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2428793636


Re: RFR: 8342868: Errors related to unused code on Windows after 8339120 in core libs

2024-10-29 Thread David Holmes
On Wed, 23 Oct 2024 04:40:50 GMT, Julian Waters  wrote:

> After 8339120, gcc began catching many different instances of unused code in 
> the Windows specific codebase. Some of these seem to be bugs. I've taken the 
> effort to mark out all the relevant globals and locals that trigger the 
> unused warnings and addressed all of them by commenting out the code as 
> appropriate. I am confident that in many cases this simplistic approach of 
> commenting out code does not fix the underlying issue, and the warning 
> actually found a bug that should be fixed. In these instances, I will be 
> aiming to fix these bugs with help from reviewers, so I recommend anyone 
> reviewing who knows more about the code than I do to see whether there is 
> indeed a bug that needs fixing in a different way than what I did

src/java.base/share/native/libzip/zip_util.c line 1:

> 1: /*

Not my area but this fix looks reasonable.

src/java.base/windows/native/libjava/HostLocaleProviderAdapter_md.c line 970:

> 968:  // int got = 0;
> 969: 
> 970: /*

Don't understand this one at all - what did gcc actually complain about here? 
This code all seems used.

src/java.base/windows/native/libjava/TimeZone_md.c line 235:

> 233: TziValue tempTzi;
> 234: WCHAR *stdNamePtr = tzi.StandardName;
> 235:  // int onlyMapID;

Looks like this became unused with JDK-8209167, so deleting it seems fine to me.

src/java.base/windows/native/libnet/NTLMAuthSequence.c line 50:

> 48: static jfieldID status_seqCompleteID;
> 49: 
> 50: // static HINSTANCE lib = NULL;

Looks like this became unused with JDK-7030256, so removal seems fine.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/21654#discussion_r1821924616
PR Review Comment: https://git.openjdk.org/jdk/pull/21654#discussion_r1821922770
PR Review Comment: https://git.openjdk.org/jdk/pull/21654#discussion_r1821929205
PR Review Comment: https://git.openjdk.org/jdk/pull/21654#discussion_r1821934247


Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage

2024-10-24 Thread David Holmes
On Thu, 24 Oct 2024 03:33:51 GMT, Julian Waters  wrote:

> the way I did it I'd have to force push

That should not be the case. You can just anti-delta changes.

-

PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2434475849


Re: RFR: 8344056: Use markdown format for man pages [v2]

2024-11-14 Thread David Holmes
On Thu, 14 Nov 2024 11:11:54 GMT, Magnus Ihse Bursie  wrote:

>> Currently, the man pages are stored as troff (a text format) in the open 
>> repo, and a content-wise identical copy is stored as markdown (another text 
>> format) in the closed repo.
>> 
>> Since markdown is preferred to troff in terms of editing, we make changes to 
>> the man pages in markdown and then convert it to troff.
>> 
>> This closed-markdown to open-troff processing needs to be done manually by 
>> an Oracle engineer. This is done regularly at the start and end of a new 
>> release cycle, adding to the burden of creating a new release. It is also 
>> done (if any of the reviewers knows about the process) whenever an Oracle 
>> engineer updates a man page. If a community contributor changes the behavior 
>> of a tool, an Oracle engineer needs to change the documentation for them, 
>> since they cannot do it themselves.
>
> Magnus Ihse Bursie has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Fix CheckManPageOptions test
>  - Remove classpath exception

LGTM!

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22081#pullrequestreview-2437489438


Re: RFR: 8344056: Use markdown format for man pages [v2]

2024-11-14 Thread David Holmes
On Thu, 14 Nov 2024 11:11:54 GMT, Magnus Ihse Bursie  wrote:

>> Currently, the man pages are stored as troff (a text format) in the open 
>> repo, and a content-wise identical copy is stored as markdown (another text 
>> format) in the closed repo.
>> 
>> Since markdown is preferred to troff in terms of editing, we make changes to 
>> the man pages in markdown and then convert it to troff.
>> 
>> This closed-markdown to open-troff processing needs to be done manually by 
>> an Oracle engineer. This is done regularly at the start and end of a new 
>> release cycle, adding to the burden of creating a new release. It is also 
>> done (if any of the reviewers knows about the process) whenever an Oracle 
>> engineer updates a man page. If a community contributor changes the behavior 
>> of a tool, an Oracle engineer needs to change the documentation for them, 
>> since they cannot do it themselves.
>
> Magnus Ihse Bursie has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Fix CheckManPageOptions test
>  - Remove classpath exception

> > Now `CheckManPageOptions` finds the `.md` file (good) and its checks fail 
> > (bad).
> 
> _sigh_
> 
> > A candidate for an ignore list as fixing it is out of scope of this PR?
> 
> Let me have a look at it first. It seems the test has the indention to handle 
> markdown files, so maybe there is an easy fix.

I'm somewhat surprised that the full src tree is available to this test when it 
runs. I was expecting it to examine the .1 files in the JDK image.

-

PR Comment: https://git.openjdk.org/jdk/pull/22081#issuecomment-2477764902


Re: RFR: 8344056: Use markdown format for man pages

2024-11-13 Thread David Holmes
On Wed, 13 Nov 2024 17:05:25 GMT, Magnus Ihse Bursie  wrote:

> Currently, the man pages are stored as troff (a text format) in the open 
> repo, and a content-wise identical copy is stored as markdown (another text 
> format) in the closed repo.
> 
> Since markdown is preferred to troff in terms of editing, we make changes to 
> the man pages in markdown and then convert it to troff.
> 
> This closed-markdown to open-troff processing needs to be done manually by an 
> Oracle engineer. This is done regularly at the start and end of a new release 
> cycle, adding to the burden of creating a new release. It is also done (if 
> any of the reviewers knows about the process) whenever an Oracle engineer 
> updates a man page. If a community contributor changes the behavior of a 
> tool, an Oracle engineer needs to change the documentation for them, since 
> they cannot do it themselves.

Great to finally see this happen!

Just one glitch with the GPL headers.

Thanks

src/java.base/share/man/java.md line 9:

> 7: # published by the Free Software Foundation.  Oracle designates this
> 8: # particular file as subject to the "Classpath" exception as provided
> 9: # by Oracle in the LICENSE file that accompanied this code.

Documentation files should not have the Classpath exception.

make/data/license-templates/gpl-header

-

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22081#pullrequestreview-2434474132
PR Review Comment: https://git.openjdk.org/jdk/pull/22081#discussion_r1841189067


Re: RFR: 8344191: Build code should not have classpath exception

2024-11-16 Thread David Holmes
On Fri, 15 Nov 2024 00:07:07 GMT, Magnus Ihse Bursie  wrote:

> The policy has long been to use Classpath Exception in the `src` and `make` 
> directories, but not in the `test` directories. If you're changing the 
> policy, you might want to check and update any documentation where the policy 
> might be written down.

What policy is that? The Classpath exception is for executing classfiles. It is 
meaningless in any other context.

-

PR Comment: https://git.openjdk.org/jdk/pull/22104#issuecomment-2480542305


Integrated: 8349511: [BACKOUT] Framework for tracing makefile inclusion and parsing

2025-02-05 Thread David Holmes
On Thu, 6 Feb 2025 01:32:51 GMT, David Holmes  wrote:

> This reverts commit 61465883b465a184e31e7a03e2603d29ab4815a4.
> 
> JDK-8348190: Framework for tracing makefile inclusion and parsing
> 
> The above issue caused problems in the Oracle closed builds and so needs to 
> be backed out until that is addressed.
> 
> Thanks.

This pull request has now been integrated.

Changeset: 64bd8d25
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk/commit/64bd8d2592d26e02a7f2f96caa47cba5e158aaa2
Stats: 2717 lines in 273 files changed: 501 ins; 1663 del; 553 mod

8349511: [BACKOUT] Framework for tracing makefile inclusion and parsing

Reviewed-by: darcy, mikael

-

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


Re: RFR: 8349511: [BACKOUT] Framework for tracing makefile inclusion and parsing

2025-02-05 Thread David Holmes
On Thu, 6 Feb 2025 02:01:47 GMT, Joe Darcy  wrote:

>> This reverts commit 61465883b465a184e31e7a03e2603d29ab4815a4.
>> 
>> JDK-8348190: Framework for tracing makefile inclusion and parsing
>> 
>> The above issue caused problems in the Oracle closed builds and so needs to 
>> be backed out until that is addressed.
>> 
>> Thanks.
>
> Approving clean backout.

Thanks @jddarcy !

-

PR Comment: https://git.openjdk.org/jdk/pull/23481#issuecomment-2638687813


Re: RFR: 8349511: [BACKOUT] Framework for tracing makefile inclusion and parsing

2025-02-05 Thread David Holmes
On Thu, 6 Feb 2025 02:48:21 GMT, Mikael Vidstedt  wrote:

>> This reverts commit 61465883b465a184e31e7a03e2603d29ab4815a4.
>> 
>> JDK-8348190: Framework for tracing makefile inclusion and parsing
>> 
>> The above issue caused problems in the Oracle closed builds and so needs to 
>> be backed out until that is addressed.
>> 
>> Thanks.
>
> Marked as reviewed by mikael (Reviewer).

Thanks @vidmik !

-

PR Comment: https://git.openjdk.org/jdk/pull/23481#issuecomment-2638701374


RFR: 8349511: [BACKOUT] Framework for tracing makefile inclusion and parsing

2025-02-05 Thread David Holmes
This reverts commit 61465883b465a184e31e7a03e2603d29ab4815a4.

JDK-8348190: Framework for tracing makefile inclusion and parsing

The above issue caused problems in the Oracle closed builds and so needs to be 
backed out until that is addressed.

Thanks.

-

Commit messages:
 - Revert "8348190: Framework for tracing makefile inclusion and parsing"

Changes: https://git.openjdk.org/jdk/pull/23481/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23481&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8349511
  Stats: 2717 lines in 273 files changed: 501 ins; 1663 del; 553 mod
  Patch: https://git.openjdk.org/jdk/pull/23481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23481/head:pull/23481

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


Re: RFR: 8345805: Update copyright year to 2024 for other files where it was missed [v2]

2024-12-10 Thread David Holmes
On Mon, 9 Dec 2024 21:02:03 GMT, Magnus Ihse Bursie  wrote:

>> Some files have been modified in 2024, but the copyright year has not been 
>> properly updated. This should be fixed. 
>> 
>> I have located these modified files using:
>> 
>> git log --since="Jan 1" --name-only --pretty=format: | sort -u > file.list
>> 
>> and then run a script to update the copyright year to 2024 on these files.
>> 
>> I have made a manual sampling of files in the list to verify that they have 
>> indeed been modified in 2024.
>> 
>> This is a "misc" bucket bug report, covering for all those files that has 
>> not been clearly assigned in some other issue. My strategy was to update the 
>> copyright year on all files in the JDK repo, and then try to the best of my 
>> ability to partition that huge chunk of files between groups. These are the 
>> remainder after I've done the large chunks. When you review, please state 
>> clearly what part of the code you are reviewing.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Revert mistaken changes to binary file

Looks fine. Thanks.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22645#pullrequestreview-2494322540


Re: RFR: 8347840: Fix testlibrary compilation warnings

2025-01-15 Thread David Holmes
On Wed, 15 Jan 2025 23:48:33 GMT, Leonid Mesnik  wrote:

> There few compiler warning disabled in the testlibary build.
> They should be fixed or localized and removed from build to prevent new 
> possible issues.
> 
> The main goal is to avoid new such issues in the testlibrary. 
> Tested with tier1-5 to ensure that all tests were passed.

Overall looks good - sometimes a bit of a puzzler understanding what warning is 
being addressed. :)

Only one thing I'm concerned may be an issue. Also a couple of suggestions.

Thanks

test/lib/jdk/test/lib/format/ArrayDiff.java line 110:

> 108:  * @return an ArrayDiff instance for the two arrays and formatting 
> parameters provided
> 109:  */
> 110: @SuppressWarnings({"rawtypes", "unchecked"})

Just wondering where the unchecked warning arises in the code?

test/lib/jdk/test/lib/hprof/model/JavaHeapObject.java line 42:

> 40:  *
> 41:  * @author  Bill Foote
> 42:  */

I would suggest deleting any comment blocks that just have the @author tag, and 
deleting the @author elsewhere. All these files (lib/hprof) already have an 
author attribution comment.

test/lib/jdk/test/lib/hprof/parser/ReadBuffer.java line 46:

> 44: public int   getInt(long pos) throws IOException;
> 45: public long  getLong(long pos) throws IOException;
> 46: public void  close() throws IOException;

Why was this redefined to throw IOException rather than just Exception?

test/lib/jdk/test/lib/hprof/parser/Reader.java line 96:

> 94: } else if ((i >>> 8) == GZIP_HEADER_MAGIC) {
> 95: // Possible gziped file, try decompress it and get the 
> stack trace.
> 96: in.close();

It is not obvious to me that there may not be a reason for closing all of the 
streams before opening new ones below.

test/lib/jdk/test/lib/hprof/parser/Reader.java line 169:

> 167: } else if ((i >>> 8) == GZIP_HEADER_MAGIC) {
> 168: // Possible gziped file, try decompress it and get the 
> stack trace.
> 169: in.close();

It is not obvious to me that there may not be a reason for closing all of the 
streams before opening new ones below.

test/lib/jdk/test/lib/thread/VThreadRunner.java line 100:

> 98: if ((X) ex == ex) {
> 99: throw (X) ex;
> 100: }

This doesn't make sense to me.

-

PR Review: https://git.openjdk.org/jdk/pull/23143#pullrequestreview-2554831705
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917773365
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917775999
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917780287
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917795069
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917795554
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917787207


Re: RFR: 8347840: Fix testlibrary compilation warnings [v3]

2025-01-16 Thread David Holmes
On Thu, 16 Jan 2025 17:53:48 GMT, Leonid Mesnik  wrote:

>> test/lib/jdk/test/lib/hprof/parser/ReadBuffer.java line 46:
>> 
>>> 44: public int   getInt(long pos) throws IOException;
>>> 45: public long  getLong(long pos) throws IOException;
>>> 46: public void  close() throws IOException;
>> 
>> Why was this redefined to throw IOException rather than just Exception?
>
> The javac complains about potential InterruptedException, so I changed type.
> See
> https://docs.oracle.com/javase/8/docs/api/java/lang/AutoCloseable.html
> `Implementers of this interface are also strongly advised to not have the 
> close method throw 
> [InterruptedException](https://docs.oracle.com/javase/8/docs/api/java/lang/InterruptedException.html).
>  This exception interacts with a thread's interrupted status, and runtime 
> misbehavior is likely to occur if an InterruptedException is 
> [suppressed](https://docs.oracle.com/javase/8/docs/api/java/lang/Throwable.html#addSuppressed-java.lang.Throwable-).
>  More generally, if it would cause problems for an exception to be 
> suppressed, the AutoCloseable.close method should not throw it.`

Ugggh! That is an annoyance.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1919602558


Re: RFR: 8347840: Fix testlibrary compilation warnings [v3]

2025-01-16 Thread David Holmes
On Thu, 16 Jan 2025 18:18:15 GMT, Leonid Mesnik  wrote:

>> There few compiler warning disabled in the testlibary build.
>> They should be fixed or localized and removed from build to prevent new 
>> possible issues.
>> 
>> The main goal is to avoid new such issues in the testlibrary. 
>> Tested with tier1-5 to ensure that all tests were passed.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   revert change

Looks good. Thanks

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23143#pullrequestreview-2558035645


Re: RFR: 8351322: Parameterize link option for pthreads [v2]

2025-03-16 Thread David Holmes
On Fri, 7 Mar 2025 00:18:14 GMT, David Holmes  wrote:

>>> What is the intended way of using this? Do you run make with 
>>> LIBPTHREAD=-pthread or do you apply a patch on libraries.m4 for the 
>>> specific way of linking to pthread?
>> 
>> This is in preparation of the upcoming BSD port, which uses `-pthread` 
>> instead of `-pthread`. It was me who suggested that this is done separately 
>> with the existing code, to minimize the patch of the BSD port.
>
> @magicus why can't we just use `-pthread` everywhere? My recollection is that 
> `-pthread` both sets compiler directives needed for pthread programming and 
> links to libpthread, so it seems to be what we should be using. ??

> Another follow-up is if it would hurt to include $LIBPTHREAD for _all_ 
> Hotspot tests, to avoid the huge list. @dholmes-ora Do you have anything 
> coming to mind directly that would make that infeasible, or is it just a 
> matter of testing to add it and see if any tests fail?

Sorry I was out of contact for a while. I can't imagine why any test would fail 
if linked with libpthread, given the VM is linking to it anyway.

> We can then check if we can turn -lpthread into -pthread on Linux as a follow 
> up.

I have a vague recollection that at one time `-pthread` set some _POSIX_SOURCE 
define (or something like that) which conflicted with our use of some gcc 
specific things. But that was long ago so I would try it and see. Of couise it 
then becomes hard to classify what kind of flag this is because it isn't 
strictly a library flag.

-

PR Comment: https://git.openjdk.org/jdk/pull/23930#issuecomment-2728361931


Re: RFR: 8351322: Parameterize link option for pthreads [v2]

2025-03-26 Thread David Holmes
On Thu, 20 Mar 2025 12:38:37 GMT, Magnus Ihse Bursie  wrote:

> Is there any gain then in changing away from -lpthread? That is clearly 
> defined, link with libpthread, with no side effects. "If it ain't broke..."

True - what we have works and using `-pthread` might subtly change things.

-

PR Comment: https://git.openjdk.org/jdk/pull/23930#issuecomment-2742145168


Re: RFR: 8351322: Parameterize link option for pthreads

2025-03-06 Thread David Holmes
On Thu, 6 Mar 2025 15:47:58 GMT, Magnus Ihse Bursie  wrote:

>> What is the intended way of using this? Do you run make with 
>> `LIBPTHREAD=-pthread` or do you apply a patch on `libraries.m4` for the 
>> specific way of linking to pthread?
>
>> What is the intended way of using this? Do you run make with 
>> LIBPTHREAD=-pthread or do you apply a patch on libraries.m4 for the specific 
>> way of linking to pthread?
> 
> This is in preparation of the upcoming BSD port, which uses `-pthread` 
> instead of `-pthread`. It was me who suggested that this is done separately 
> with the existing code, to minimize the patch of the BSD port.

@magicus why can't we just use `-pthread` everywhere? My recollection is that 
`-pthread` both sets compiler directives needed for pthread programming and 
links to libpthread, so it seems to be what we should be using. ??

-

PR Comment: https://git.openjdk.org/jdk/pull/23930#issuecomment-2705227213


Re: RFR: 8351322: Parameterize link option for pthreads

2025-03-06 Thread David Holmes
On Thu, 6 Mar 2025 10:39:27 GMT, snake66  wrote:

> Replace hardcoded instances of `-lpthread` with `$(LIBPTHREAD)`, so that it's 
> possible to parameterize this for platforms that use different flags for 
> enabling posix threads.
> 
> This work is a continuation of the work done by Greg Lewis in [1], but 
> generalized for the full JDK, and set at the configure stage.
> 
> Sponsored by: The FreeBSD Foundation
> Co-authored-by: Greg Lewis 
> 
> [1]: 
> https://github.com/battleblow/jdk23u/commit/dbd90aa8ab0b7f5e4865864a7c63d975daacabf4

Abstracting this out seems reasonable to me, though I should say I thought we 
already used `-pthread` rather than `-lpthread`.

Needs build team approval before integrating.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23930#pullrequestreview-2664340896


Re: RFR: 8356171: Increase timeout for testcases as preparation for change of default timeout factor

2025-05-08 Thread David Holmes
On Thu, 8 May 2025 14:51:24 GMT, Leo Korinth  wrote:

> This change tries to add timeout to individual testcases so that I am able to 
> run them with a timeout factor of 1 in the future (JDK-8260555).
> 
> The first commit changes the timeout factor to 0.7, so that I can run tests 
> and test the change (it will finally be changed to 1.0 in JDK-8260555). The 
> next commit excludes some junit/testng tests where I can currently not change 
> the timeout factor (CODETOOLS-7903961). Both these commits will be reverted 
> before integrating the change. I will also apply copyright updates after the 
> review.
> 
> In addition to changing the timeout factor, I am also using a library call to 
> parse the timeout factor from the java properties (I can not use the library 
> function everywhere as jtreg does not allow me to add @library notations to 
> non testcase files).
> 
> My approach has been to run all test, and afterwards updating those that 
> fails due to a timeout factor. The amount of updated testcases is huge, and 
> my strategy has been to quadruple the timeout if I could not directly see 
> that less was needed (thus the timeout will be the same after JDK-8260555 is 
> implemented). In a few places I have added a bit more timeout so that it will 
> work with the 0.7 timeout factor.
> 
> These fixes have been created when I have plown through testcases:
> JDK-8352719: Add an equals sign to the modules statement
> JDK-8352709: Remove bad timing annotations from WhileOpTest.java
> JDK-8352074: Test MemoryLeak.java seems not to test what it is supposed to 
> test
> CODETOOLS-7903937: JTREG uses timeout factor on socket timeout but not on 
> KEEPALIVE
> CODETOOLS-7903961: Make default timeout configurable
> 
> Sometime in the future I will also fix:
> 8260555: Change the default TIMEOUT_FACTOR from 4 to 1
> 
> for which I am awaiting:
> CODETOOLS-7903961 that is fixed in jtreg 7.6, but we are still running 7.5.1+1
> 
> *After the review I will revert the two first commits, and update the 
> copyrights*

My biggest concern with this change is that potentially any test that 
implicitly uses the default timeout, and which passes with no problem with a 
factor of 4, may now need to have an explicit timeout set due to the factor of 
1. I see this change in a number of tests (default timeout of 120s expressed as 
a new timeout of 480s to maintain equivalence**), but how many times did you 
run the tiers? I fear that the gatekeepers will be playing timeout whack-a-mole 
once these changes are put in.

** though another option would be to update the jtreg default timeout instead.

-

PR Comment: https://git.openjdk.org/jdk/pull/25122#issuecomment-2865099247


Re: RFR: 8356171: Increase timeout for testcases as preparation for change of default timeout factor

2025-05-09 Thread David Holmes
On Thu, 8 May 2025 14:51:24 GMT, Leo Korinth  wrote:

> This change tries to add timeout to individual testcases so that I am able to 
> run them with a timeout factor of 1 in the future (JDK-8260555).
> 
> The first commit changes the timeout factor to 0.7, so that I can run tests 
> and test the change (it will finally be changed to 1.0 in JDK-8260555). The 
> next commit excludes some junit/testng tests where I can currently not change 
> the timeout factor (CODETOOLS-7903961). Both these commits will be reverted 
> before integrating the change. I will also apply copyright updates after the 
> review.
> 
> In addition to changing the timeout factor, I am also using a library call to 
> parse the timeout factor from the java properties (I can not use the library 
> function everywhere as jtreg does not allow me to add @library notations to 
> non testcase files).
> 
> My approach has been to run all test, and afterwards updating those that 
> fails due to a timeout factor. The amount of updated testcases is huge, and 
> my strategy has been to quadruple the timeout if I could not directly see 
> that less was needed (thus the timeout will be the same after JDK-8260555 is 
> implemented). In a few places I have added a bit more timeout so that it will 
> work with the 0.7 timeout factor.
> 
> These fixes have been created when I have plown through testcases:
> JDK-8352719: Add an equals sign to the modules statement
> JDK-8352709: Remove bad timing annotations from WhileOpTest.java
> JDK-8352074: Test MemoryLeak.java seems not to test what it is supposed to 
> test
> CODETOOLS-7903937: JTREG uses timeout factor on socket timeout but not on 
> KEEPALIVE
> CODETOOLS-7903961: Make default timeout configurable
> 
> Sometime in the future I will also fix:
> 8260555: Change the default TIMEOUT_FACTOR from 4 to 1
> 
> for which I am awaiting:
> CODETOOLS-7903961 that is fixed in jtreg 7.6, but we are still running 7.5.1+1
> 
> *After the review I will revert the two first commits, and update the 
> copyrights*

I saw this PR as preparation for the change of the timeout factor so they could 
be reviewed distinctly and then integrated close together. So it is natural 
that people are querying how the proposed changes will work with that change - 
in particular if it will require explicit timeouts to be added to a lot of 
tests that don't presently have them.

-

PR Comment: https://git.openjdk.org/jdk/pull/25122#issuecomment-2866338479


Re: RFR: 8342868: Errors related to unused code on Windows after 8339120 in core libs [v2]

2025-07-07 Thread David Holmes
On Tue, 8 Jul 2025 01:28:04 GMT, Julian Waters  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove the got local
>
> Sorry for waiting so long. It's become clear that I won't be able to get awt 
> and accessibility up to speed for a long time, so I will go ahead with this 
> one first

@TheShermanTanker the commented out code really should have been deleted, not 
just left commented out. Please file anpther JBS issue to have this cleaned up 
so it is not forgotten. Thanks.

-

PR Comment: https://git.openjdk.org/jdk/pull/21654#issuecomment-3047084740


Re: RFR: 8260555: Change the default TIMEOUT_FACTOR from 4 to 1 [v4]

2025-08-20 Thread David Holmes
On Wed, 20 Aug 2025 15:21:39 GMT, Magnus Ihse Bursie  wrote:

> I realize that this is highly hardware dependent, but test times tend to be 
> Pareto distributed, so a very quick test maybe takes 1 second on fast 
> machines and 10 on slow, 

@magicus unfortunately that is often not the case in practice. We can see many 
tests that normally run very quickly but occasionally run very slow - minutes 
versus seconds.

> Right now it seems engineers is spending their valuable time giving 
> guesstimates for each individual test. That seems like poorly used time and 
> resources.

For this exercise existing explicit timeout values need to be multiplied by 4 
to keep the same absolute timeout value. In addition a number of tests that use 
the implicit default timeout (120*4) now need an explicit timeout (480 
generally).

-

PR Comment: https://git.openjdk.org/jdk/pull/26749#issuecomment-3208143195


Re: RFR: 8260555: Change the default TIMEOUT_FACTOR from 4 to 1 [v3]

2025-08-20 Thread David Holmes
On Tue, 19 Aug 2025 06:38:01 GMT, SendaoYan  wrote:

>> No change should be made to any explicit setting of the timeoutFactor in 
>> general as that could cause mass timeouts to occur (old default timeout = 
>> 120 * 10 = 1200 but new default = 120 * 2.5 = 300!). 
>> 
>> However I see the concern of @sendaoYan because individual tests may now get 
>> much larger timeout values when run with the non-default timeoutFactor 
>> because they have been adjusted for the new default. I don't see any 
>> solution to this dilemma.
>
> But what this PR do is change the timeoutFactor in general and find out all 
> the tests which may timeout after the timeoutFactor has been changed.
> 
> The old default timeout before this PR is 120 * 4, after this PR the new 
> default is 120 * 1

@sendaoYan this PR changes the default timeoutFactor and so also has to change 
quite a number of implicit and explicit timeouts. But that doesn't mean that 
test configs that already set their own timeoutFactor should adjust by the same 
factor! That just doesn't work for any test with an implicit default timeout.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2288072431


Re: RFR: 8260555: Change the default TIMEOUT_FACTOR from 4 to 1 [v5]

2025-08-24 Thread David Holmes
On Fri, 22 Aug 2025 15:41:21 GMT, Albert Mingkun Yang  wrote:

>> Leo Korinth has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   update testing.md, remove makefile link, fix bad text
>
> test/hotspot/jtreg/compiler/tiered/Level2RecompilationTest.java line 36:
> 
>> 34:  * @build jdk.test.whitebox.WhiteBox
>> 35:  * @run driver jdk.test.lib.helpers.ClassFileInstaller 
>> jdk.test.whitebox.WhiteBox
>> 36:  * @run main/othervm/timeout=960 -Xbootclasspath/a:. 
>> -XX:+TieredCompilation
> 
> Why not `480` in this case?

Leo also states in the description:

> In a few places, I have added a bit more timeout so that it will work with 
> the 0.7 timeout factor.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2296840413


Re: RFR: 8260555: Change the default TIMEOUT_FACTOR from 4 to 1 [v3]

2025-08-18 Thread David Holmes
On Tue, 19 Aug 2025 05:23:15 GMT, David Holmes  wrote:

>> Take test 
>> test/hotspot/jtreg/compiler/arraycopy/stress/TestStressArrayCopy.java as 
>> example.
>> If there is a bug in jvm with -Xcomp option which will cause this test run 
>> time outed. Before this PR, it will take `7200*10` seconds to run this test 
>> finish and report time outed failure. But after this PR, it will take 
>> `28800*10` seconds to run this test finish ang then report timed out 
>> failure. I think the `28800*10` senonds is too long and it's unacceptable.
>
> DELETED - I confused the timeout with the timeout-factor. New comment below.

No change should be made to any explicit setting of the timeoutFactor in 
general as that could cause mass timeouts to occur (old default timeout = 120 * 
10 = 1200 but new default = 120 * 2.5 = 300!). 

However I see the concern of @sendaoYan because individual tests may now get 
much larger timeout values when run with the non-default timeoutFactor because 
they have been adjusted for the new default. I don't see any solution to this 
dilemma.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2284161138


Re: RFR: 8260555: Change the default TIMEOUT_FACTOR from 4 to 1 [v3]

2025-08-18 Thread David Holmes
On Tue, 19 Aug 2025 03:31:55 GMT, SendaoYan  wrote:

>> It is also something that can be changed later, in a follow up fix.
>
> Take test 
> test/hotspot/jtreg/compiler/arraycopy/stress/TestStressArrayCopy.java as 
> example.
> If there is a bug in jvm with -Xcomp option which will cause this test run 
> time outed. Before this PR, it will take `7200*10` seconds to run this test 
> finish and report time outed failure. But after this PR, it will take 
> `28800*10` seconds to run this test finish ang then report timed out failure. 
> I think the `28800*10` senonds is too long and it's unacceptable.

> It is unclear to me if the author meant this to be 2.5 more than normal or 10 
> more than JTREG default, or a multiplier that seems to work. 

What matters is that the actual timeout, in seconds, remains unchanged, so 
please address this. Timeouts that are excessively long waste machine resources.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2284090715


  1   2   >