8303485: Replacing os.name for operating system customization #12931

RogerRiggs · 2023-03-08T19:15:16Z

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.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8303485: Replacing os.name for operating system customization

Reviewers

Naoto Sato (@naotoj - Reviewer) ⚠️ Review applies to 8372978f
Erik Joelsson (@erikj79 - Reviewer) ⚠️ Review applies to 736ab3cc
Alan Bateman (@AlanBateman - Reviewer) ⚠️ Review applies to 8372978f

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/12931/head:pull/12931
$ git checkout pull/12931

Update a local copy of the PR:
$ git checkout pull/12931
$ git pull https://git.openjdk.org/jdk.git pull/12931/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12931

View PR using the GUI difftool:
$ git pr show -t 12931

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/12931.diff

Improvements should 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

bridgekeeper · 2023-03-08T19:16:42Z

👋 Welcome back rriggs! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-03-08T19:19:55Z

@RogerRiggs The following labels will be automatically applied to this pull request:

build
core-libs
net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-03-08T19:24:31Z

Webrevs

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java

src/java.base/share/classes/jdk/internal/misc/OperatingSystemProps.java.template

mlchung

I wonder if jdk.internal.platform would be a better home for OperatingSystem class?

src/java.base/share/classes/jdk/internal/misc/OperatingSystemProps.java.template

make/modules/java.base/gensrc/GensrcMisc.gmk

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java

src/java.base/share/classes/jdk/internal/misc/OperatingSystemProps.java.template

naotoj

Looks good. Some minor nits follow.

src/java.base/share/classes/jdk/internal/util/StaticProperty.java

src/java.base/unix/classes/java/lang/ProcessImpl.java

test/jdk/java/foreign/TestUnsupportedLinker.java

dholmes-ora

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

erikj79

Build changes look good.

openjdk · 2023-03-09T15:20:53Z

@RogerRiggs This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8303485: Replacing os.name for operating system customization

Reviewed-by: naoto, erikj, alanb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 230 new commits pushed to the master branch:

87b314a: 7093691: Nimbus LAF: disabled JComboBox using renderer has bad font color
2f34687: 8304689: Add hidden option to disable external spec page
6b2f34f: 8304718: GetIntArrayElements should not be passed JNI_FALSE
138cdc9: 8304694: Runtime exception thrown when break stmt is missing
46b0602: 8304547: Remove checking of -Djava.compiler in src/jdk.jdi/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java
8d423f7: 8304723: Statically allocate global mutexes
2083088: 8304828: Lots of constant static data not declared static const in cpu/x86
0712adc: 7169951: SwingSet2 throws NullPointerException with Nimbus L&F
de1c12e: 8301012: [vectorapi]: Intrinsify CompressBitsV/ExpandBitsV and add the AArch64 SVE backend implementation
10fa7d1: 8304804: Remove develop flag G1VerifyCTCleanup
... and 220 more: https://git.openjdk.org/jdk/compare/f813dc71836e002814622fead8a2b0464b49c83a...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

RogerRiggs · 2023-03-09T16:11:19Z

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.

I haven't found any use of BSD and I don't think the build supports a BSD build.

mlbridge · 2023-03-09T18:27:36Z

Mailing list message from Justin King on build-dev:

Let's please not kill generic BSD support if at all possible. There is
NetBSD, OpenBSD, FreeBSD, and DragonflyBSD. I know FreeBSD and NetBSD have
OpenJDK 19 and 17 respectively.

On Wed, Mar 8, 2023, 6:54 PM David Holmes <dholmes at openjdk.org> wrote:

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/build-dev/attachments/20230308/980d7009/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3999 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://mail.openjdk.org/pipermail/build-dev/attachments/20230308/980d7009/smime-0001.p7s>

mlbridge · 2023-03-09T18:27:38Z

Mailing list message from Roger Riggs on build-dev:

Hi Justin,

How would I go about building one of those? Or knowing what the
dependencies are?

Thanks, Roger

On 3/8/23 11:02 PM, Justin King wrote:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/build-dev/attachments/20230309/1f8ce865/attachment.htm>

RogerRiggs · 2023-03-09T20:32:13Z

I wonder if jdk.internal.platform would be a better home for OperatingSystem class?

jdk.internal.platform seems to be focused on information about container environments such as Docker.

jdk.internal.misc already contains other classes that are shared out of java.base to other modules.

RogerRiggs · 2023-03-09T20:34:12Z

To see the knock-on effects of using the API in other modules, see the Draft PR #12961

jcking · 2023-03-10T00:08:57Z

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.

I haven't found any use of BSD and I don't think the build supports a BSD build.

It looks like FreeBSD and NetBSD have their own separate ports. They fork the OpenJDK repos and then add changes on-top to make it work for BSD. That is kind of disappointing...one more thing for them to patch I suppose. I wonder how big the diff is...it looks like github.com/battleblow/jdk19u is what FreeBSD uses.

AlanBateman · 2023-03-10T13:57:36Z

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.

The macOS port in 7u4 was based on a BSD port and there was a time when it was possible to build. Right now, we have src/hotspot/os/bsd and native code in the libs that is compiled in with _ALLBSD_SOURCE. It's sad to see the BSD port bit rot but if it's not maintained or tested in the OpenJDK project then the changes proposed here may be okay. For porters, they need to know which code needs to be ported is important, ideally there would be a compile or build time if something significant is missing.

src/java.base/share/classes/java/util/zip/ZipFile.java

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java

jcking · 2023-03-10T14:22:17Z

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.

The macOS port in 7u4 was based on a BSD port and there was a time when it was possible to build. Right now, we have src/hotspot/os/bsd and native code in the libs that is compiled in with _ALLBSD_SOURCE. It's sad to see the BSD port bit rot but if it's not maintained or tested in the OpenJDK project then the changes proposed here may be okay. For porters then know which code needs to be ported is important, ideally there would be a compile or build time if someone significant is missing.

Yes it is unfortunate. It would be nice if NetBSD, FreeBSD, OpenBSD, and friends would collaborate. I think the majority of their changes are fixing build issues and making AWT/Swing use X11 instead of macOS-specific stuff (Quartz or Cocoa or whatever). Maybe I'll go poke them eventually.

Rename Mac -> MacOS; isMac() -> isMacOS()

michael-o · 2023-03-12T13:57:21Z

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.

The macOS port in 7u4 was based on a BSD port and there was a time when it was possible to build. Right now, we have src/hotspot/os/bsd and native code in the libs that is compiled in with _ALLBSD_SOURCE. It's sad to see the BSD port bit rot but if it's not maintained or tested in the OpenJDK project then the changes proposed here may be okay. For porters then know which code needs to be ported is important, ideally there would be a compile or build time if someone significant is missing.

Yes it is unfortunate. It would be nice if NetBSD, FreeBSD, OpenBSD, and friends would collaborate. I think the majority of their changes are fixing build issues and making AWT/Swing use X11 instead of macOS-specific stuff (Quartz or Cocoa or whatever). Maybe I'll go poke them eventually.

I bet @battleblow has tried, but that wasn't fruitful with the OpenJDK team.

michael-o · 2023-03-12T13:58:17Z

This looks wrong because it would leave out a lot of ports which this solution cannot cover. Rephrased: An exhaustive list of values to an non-exhaustive amount of systems just feels wrong.

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java

src/java.base/unix/classes/java/lang/ProcessImpl.java

RogerRiggs · 2023-03-22T20:49:56Z

Please review the revisions.

src/java.base/unix/classes/java/lang/ProcessImpl.java

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java

michael-o · 2023-03-24T16:04:18Z

@battleblow FYI

naotoj

LGTM

mlbridge · 2023-03-27T03:30:11Z

Mailing list message from David Holmes on core-libs-dev:

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

On Fri, 24 Mar 2023 16:53:05 GMT, Michael Osipov <duke at openjdk.org> 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

src/java.base/unix/classes/java/lang/ProcessImpl.java

turbanoff · 2023-03-27T15:59:54Z

src/java.base/unix/classes/java/lang/ProcessImpl.java

+     * {@return the default or requested launch mechanism}
+     * @throws Error if the requested launch mechanism is not found or valid
+     */
+    @SuppressWarnings("removal")


So, we can remove it

Suggested change

@SuppressWarnings("removal")

RogerRiggs · 2023-03-27T17:44:36Z

/integrate

openjdk · 2023-03-27T17:45:35Z

Going to push as commit 6c3b10f.
Since your change was applied there have been 230 commits pushed to the master branch:

87b314a: 7093691: Nimbus LAF: disabled JComboBox using renderer has bad font color
2f34687: 8304689: Add hidden option to disable external spec page
6b2f34f: 8304718: GetIntArrayElements should not be passed JNI_FALSE
138cdc9: 8304694: Runtime exception thrown when break stmt is missing
46b0602: 8304547: Remove checking of -Djava.compiler in src/jdk.jdi/share/classes/com/sun/tools/jdi/SunCommandLineLauncher.java
8d423f7: 8304723: Statically allocate global mutexes
2083088: 8304828: Lots of constant static data not declared static const in cpu/x86
0712adc: 7169951: SwingSet2 throws NullPointerException with Nimbus L&F
de1c12e: 8301012: [vectorapi]: Intrinsify CompressBitsV/ExpandBitsV and add the AArch64 SVE backend implementation
10fa7d1: 8304804: Remove develop flag G1VerifyCTCleanup
... and 220 more: https://git.openjdk.org/jdk/compare/f813dc71836e002814622fead8a2b0464b49c83a...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-03-27T17:45:56Z

@RogerRiggs Pushed as commit 6c3b10f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

RogerRiggs marked this pull request as ready for review March 8, 2023 19:16

openjdk bot added the rfr Pull request is ready for review label Mar 8, 2023

openjdk bot added build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org net net-dev@openjdk.org labels Mar 8, 2023

rgiulietti reviewed Mar 8, 2023

View reviewed changes

mlchung reviewed Mar 8, 2023

View reviewed changes

naotoj reviewed Mar 9, 2023

View reviewed changes

src/java.base/share/classes/jdk/internal/util/StaticProperty.java Outdated Show resolved Hide resolved

src/java.base/unix/classes/java/lang/ProcessImpl.java Outdated Show resolved Hide resolved

test/jdk/java/foreign/TestUnsupportedLinker.java Outdated Show resolved Hide resolved

dholmes-ora reviewed Mar 9, 2023

View reviewed changes

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java Outdated Show resolved Hide resolved

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java Outdated Show resolved Hide resolved

erikj79 approved these changes Mar 9, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 9, 2023

openjdk bot mentioned this pull request Mar 9, 2023

Use OperatingSystem.isXXX instead of os.name checks for modules outside of java.base #12961

Closed

3 tasks

AlanBateman reviewed Mar 10, 2023

View reviewed changes

src/java.base/share/classes/java/util/zip/ZipFile.java Outdated Show resolved Hide resolved

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java Outdated Show resolved Hide resolved

AlanBateman reviewed Mar 10, 2023

View reviewed changes

src/java.base/share/classes/jdk/internal/misc/OperatingSystem.java Outdated Show resolved Hide resolved

Move OperatingSystem from jdk.internal.misc to jdk.internal.util

342f30f

Rename Mac -> MacOS; isMac() -> isMacOS()

AlanBateman reviewed Mar 14, 2023

View reviewed changes

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java Show resolved Hide resolved

src/java.base/unix/classes/java/lang/ProcessImpl.java Show resolved Hide resolved

Cover default case in switch on OS.

ead8437

naotoj reviewed Mar 22, 2023

View reviewed changes

src/java.base/unix/classes/java/lang/ProcessImpl.java Outdated Show resolved Hide resolved

Group imports for jdk.internal.util

32ee9d4

dfuch reviewed Mar 23, 2023

View reviewed changes

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java Outdated Show resolved Hide resolved

Rename OperatingSystem enum values to uppercase

8372978

AlanBateman approved these changes Mar 24, 2023

View reviewed changes

michael-o reviewed Mar 24, 2023

View reviewed changes

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Mar 24, 2023

michael-o reviewed Mar 24, 2023

View reviewed changes

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java Outdated Show resolved Hide resolved

naotoj approved these changes Mar 24, 2023

View reviewed changes

clarify Linux

bfc3de9

turbanoff reviewed Mar 27, 2023

View reviewed changes

src/java.base/unix/classes/java/lang/ProcessImpl.java Outdated Show resolved Hide resolved

turbanoff reviewed Mar 27, 2023

View reviewed changes

src/java.base/unix/classes/java/lang/ProcessImpl.java Outdated Show resolved Hide resolved

Correct javadoc throws keyword

3b88f23

turbanoff suggested changes Mar 27, 2023

View reviewed changes

Remove unused SuppressWarnings

574e37b

openjdk bot added the integrated Pull request has been integrated label Mar 27, 2023

openjdk bot closed this Mar 27, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 27, 2023

RogerRiggs mentioned this pull request Apr 4, 2023

8304006: jlink should create the jimage file in the native endian for the target platform #11943

Closed

3 tasks

andy-goryachev-oracle mentioned this pull request May 9, 2023

8278422: Replace use of deprecated single string variant of Runtime.exec method openjdk/jfx#1130

Closed

3 tasks

RogerRiggs deleted the 8303485-os-enum branch December 11, 2023 17:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8303485: Replacing os.name for operating system customization #12931

8303485: Replacing os.name for operating system customization #12931

RogerRiggs commented Mar 8, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 8, 2023

openjdk bot commented Mar 8, 2023

mlbridge bot commented Mar 8, 2023 •

edited

Loading

mlchung left a comment

naotoj left a comment

dholmes-ora left a comment

erikj79 left a comment

openjdk bot commented Mar 9, 2023 •

edited

Loading

RogerRiggs commented Mar 9, 2023

mlbridge bot commented Mar 9, 2023

mlbridge bot commented Mar 9, 2023

RogerRiggs commented Mar 9, 2023

RogerRiggs commented Mar 9, 2023

jcking commented Mar 10, 2023

AlanBateman commented Mar 10, 2023 •

edited

Loading

jcking commented Mar 10, 2023

michael-o commented Mar 12, 2023

michael-o commented Mar 12, 2023

RogerRiggs commented Mar 22, 2023

michael-o commented Mar 24, 2023

naotoj left a comment

mlbridge bot commented Mar 27, 2023

turbanoff Mar 27, 2023

RogerRiggs commented Mar 27, 2023

openjdk bot commented Mar 27, 2023

openjdk bot commented Mar 27, 2023

8303485: Replacing os.name for operating system customization #12931

8303485: Replacing os.name for operating system customization #12931

Conversation

RogerRiggs commented Mar 8, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Mar 8, 2023

openjdk bot commented Mar 8, 2023

mlbridge bot commented Mar 8, 2023 • edited Loading

Webrevs

mlchung left a comment

Choose a reason for hiding this comment

naotoj left a comment

Choose a reason for hiding this comment

dholmes-ora left a comment

Choose a reason for hiding this comment

erikj79 left a comment

Choose a reason for hiding this comment

openjdk bot commented Mar 9, 2023 • edited Loading

RogerRiggs commented Mar 9, 2023

mlbridge bot commented Mar 9, 2023

mlbridge bot commented Mar 9, 2023

RogerRiggs commented Mar 9, 2023

RogerRiggs commented Mar 9, 2023

jcking commented Mar 10, 2023

AlanBateman commented Mar 10, 2023 • edited Loading

jcking commented Mar 10, 2023

michael-o commented Mar 12, 2023

michael-o commented Mar 12, 2023

RogerRiggs commented Mar 22, 2023

michael-o commented Mar 24, 2023

naotoj left a comment

Choose a reason for hiding this comment

mlbridge bot commented Mar 27, 2023

turbanoff Mar 27, 2023

Choose a reason for hiding this comment

RogerRiggs commented Mar 27, 2023

openjdk bot commented Mar 27, 2023

openjdk bot commented Mar 27, 2023

RogerRiggs commented Mar 8, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 8, 2023 •

edited

Loading

openjdk bot commented Mar 9, 2023 •

edited

Loading

AlanBateman commented Mar 10, 2023 •

edited

Loading