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

Mandy Chung mchung at openjdk.org
Wed Mar 8 23:26:17 UTC 2023


On Wed, 8 Mar 2023 19:15:16 GMT, Roger Riggs <rriggs at 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



More information about the build-dev mailing list