On Wed, 8 Mar 2023 19:15:16 GMT, Roger Riggs <rri...@openjdk.org> 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 wonder if `jdk.internal.platform` would be a better home for
`OperatingSystem` class?
make/modules/java.base/gensrc/GensrcMisc.gmk line 57:
> 55: OUTPUT_FILE :=
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/jdk/internal/misc/OperatingSystemProps.java,
> \
> 56: REPLACEMENTS := \
> 57: @@OPENJDK_TARGET_OS@@ => $(OPENJDK_TARGET_OS), \
The replacement string can be as simple as `@@OS_NAME@@` that represents the
current OS.
Suggestion:
@@OS_NAME@@ => $(OPENJDK_TARGET_OS), \
src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 85:
> 83: ;
> 84:
> 85: // Cache a copy of the array for lightweight indexing
This is a reference to the Enum values array (not a copy).
src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java line 98:
> 96: @ForceInline
> 97: public static boolean isLinux() {
> 98: return OperatingSystemProps.TARGET_OS_IS_LINUX;
Suggestion:
return OperatingSystemProps.CURRENT_OS_ORDINAL == Linux.ordinal();
This will also simplify the template file as `TARGET_OS_IS_XXX` constants are
not needed.
Also suggest to rename `TARGET_OS_ORDINAL` to `CURRENT_OS_ORDINAL` since it
represents the current OS (vs the build target).
src/java.base/share/classes/jdk/internal/misc/OperatingSystemProps.java.template
line 29:
> 27: * @see OperatingSystem
> 28: */
> 29: class OperatingSystemProps {
Have you considered to include OS architecture here for future use? So this
file be `PlatformProps.java.template` instead.
`jdk.tools.jlink.internal.Platform` needs to get the runtime OS and
architecture.
src/java.base/share/classes/jdk/internal/misc/OperatingSystemProps.java.template
line 39:
> 37:
> 38: // Index/ordinal of the current OperatingSystem enum as substituted
> by the build
> 39: static final int TARGET_OS_ORDINAL = TARGET_OS_@@OPENJDK_TARGET_OS@@;
This represents the current OS. What about renaming it to:
Suggestion:
static final int CURRENT_OS_ORDINAL = @@OS_NAME@@;
-------------
PR: https://git.openjdk.org/jdk/pull/12931
