[jdk11u-dev] RFR: 8344458: [11u] Add initial support for building with XCode 14 [v3]

[Andrew John Hughes]

On Tue, 19 Nov 2024 14:08:39 GMT, Antonio Vieiro <duke at openjdk.org> wrote:

>> An implementation of [JDK-8344458](https://bugs.openjdk.org/browse/JDK-8344458) that adds conditional support for building on `macos` with `XCode 14` while keeping compatibility with previous `XCode` versions.
>> 
>> The PR is separated in three commits for easier review:
>> 
>> - First commit adds a new `--enable-xcode14` configuration flag (which is currently disabled in GHA) and two additional variables:
>>     - `CFLAGS_XCODE14_DEPR_DECLARATIONS` (empty on current XCode versions, set to and `-Wno-deprecated-declarations` when `--enable-xcode14`is used)
>>     - And `CFLAGS_XCODE14_DEPR_NON_PROTOTYPE` (empty on current XCode versions, set to `-Wno-deprecated-non-prototype` when `--enable-xcode14` is used).
>> - The second commit applies `CFLAGS_XCODE14_DEPR_DECLARATIONS` to those parts of the codebase that use the deprecated `sprintf` function (i.e., avoiding `sprintf` usage errors in XCode 14).
>> - The third commit applies `CFLAGS_XCODE14_DEPR_NON_PROTOTYPE` to those parts of AWT lib that generate a compilation error (i.e., avoiding the `passing arguments to a function without prototype`).
>> 
>> Since the new flag `--enable-xcode14` is not set **the build should run exactly the same on the current XCode versions** and, consequently, the GitHub checks should pass on all platforms, including the current `macos-12` & `XCode 13.4.1`.
>
> Antonio Vieiro has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Removed doc instructions as per review

I don't think this is the right approach. This new flag is called `--enable-xcode14`, but its implementation has nothing to do with the compiler version or enabling XCode 14. It simply adds additional compiler flags to disable the warnings. Someone can add `--enable-xcode14` with 13 installed and these flags will be added (and presumably the build will fail, as you said these flags are not supported there). Alternatively, someone could have XCode 14 and not know about this flag, and it still fail to build. Also, I fail to see the point of the macosx check, because presumably this is due to a new version of clang that is part of XCode 14, and so the build would fail if someone uses that version of clang on Linux.

If these flags are needed for builds with XCode 14, we should detect that this newer compiler is being used and add them automatically. In fact, ideally, you should not depend on the version at all but test the compiler with the flag and add it if it works. Take a look at `FLAGS_SETUP_GCC6_COMPILER_FLAGS` also in `flags-cflags.m4`

You also don't need to define your own variables and pepper them all over every Makefile. This should be something handled in `FLAGS_SETUP_CFLAGS_HELPER`. Look at the area around line 587 where warnings are added to gcc and clang. They are already different there for Linux and MacOS for the JDK. Detecting whether these new flags work and then adding them to `WARNING_CFLAGS_JVM` and `WARNING_CFLAGS_JDK` looks the right way to go.

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

PR Review: https://git.openjdk.org/jdk11u-dev/pull/2966#pullrequestreview-2449203122