[jdk11u-dev] RFR: 8299254: Support dealing with standard assert macro [v4]

[Severin Gehwolf]

On Thu, 26 Sep 2024 13:30:09 GMT, Antonio Vieiro <duke at openjdk.org> wrote:

>> Backport of  [JDK-8299254 Support dealing with standard assert macro](https://bugs.openjdk.org/browse/JDK-8299254) on top of the previous PR [`macos-12` github actions ](https://github.com/openjdk/jdk11u-dev/pull/2940).
>> 
>> This will make the github actions in `jdk11u-dev`  run again on top of `macos-12` / `xcode-13.4.1` .
>> 
>> The change is minimal, and should work properly with no changes on any existing `macos-11` CI pipelines.
>
> Antonio Vieiro has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'master' of github.com:openjdk/jdk11u-dev into backports/JDK-8299254-macos12
>  - Backport 89dd23f2fab0d98879e68f817923656e113087e3
>  - 8340671: GHA: Bump macOS and Xcode versions to macos-12 and XCode 13.4.1

It would be helpful to explain what the differences of the patch to, say the JDK 17u backport was. As far as I can tell these are:

- Omitted hunk in test/hotspot/gtest/gc/shenandoah/test_shenandoahNumberSeq.cpp since the test isn't in JDK 11u
- You didn't remove the `gtest/gtest.h` related [hunk](https://github.com/openjdk/jdk17u/commit/89dd23f2fab0d98879e68f817923656e113087e3#diff-ff1dd449619fcf441d7a63b01c6c4b6bf3e6fd5dc1149ffa48f38d5e6d82e5baL63-L74) as was done for the 17u version. Why?

What testing have you done? Did you run gtests to verify they still work?

test/hotspot/gtest/gc/shared/test_memset_with_concurrent_readers.cpp line 2:

> 1: /*
> 2:  * Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.

Suggestion:

 * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved.


So as to match the JDK 17u version of the patch.

test/hotspot/gtest/jfr/test_networkUtilization.cpp line 2:

> 1: /*
> 2:  * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.

Suggestion:

 * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.

test/hotspot/gtest/unittest.hpp line 2:

> 1: /*
> 2:  * Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.

This copyright update isn't in the JDK 17u version of the patch, please remove.

https://github.com/openjdk/jdk17u/commit/89dd23f2fab0d98879e68f817923656e113087e3#diff-ff1dd449619fcf441d7a63b01c6c4b6bf3e6fd5dc1149ffa48f38d5e6d82e5ba

For backports we try to avoid copyright updates, only use what was done in the backports of newer code-lines.

test/hotspot/gtest/unittest.hpp line 36:

> 34: #include "utilities/vmassert_reinstall.hpp"
> 35: 
> 36: // gtest/gtest.h includes assert.h which will define the assert macro, but hotspot has its

This was removed in the 17u version. Why not here as well?

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

Changes requested by sgehwolf (Reviewer).

PR Review: https://git.openjdk.org/jdk11u-dev/pull/2937#pullrequestreview-2333280003
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2937#discussion_r1778346956
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2937#discussion_r1778347314
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2937#discussion_r1778350401
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2937#discussion_r1778359911