RFR: 8306678: Replace use of os.version with an internal Version record

[ExE Boss]

On Fri, 21 Apr 2023 16:44:13 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

> Create an internal Version record to hold and compare versions of the form (major, minor, micro).
> Add `OperatingSystem.version()` to return the version of the running OS.
> Replace uses of os.version in java.base.
> Subsequent PRs will apply to uses in other modules including, jdk.jlink, jdk.jpackage, and java.desktop.

src/java.base/macosx/classes/sun/nio/fs/BsdFileStore.java line 103:

> 101:                 // fgetxattr broken on APFS prior to 10.14
> 102:                 return OperatingSystem.version()
> 103:                         .compareTo(new Version(10, 14)) >= 0;

The `Version(10, 14)` record should probably be stored in a static constant.

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java line 150:

> 148: 
> 149:     // Parse and save the current version
> 150:     private static Version osVersion = initVersion();

The version field should probably be `final`:
Suggestion:

    public static Version version() {
        return CURRENT_VERSION;
    }

    // Parse and save the current version
    private static final Version CURRENT_VERSION = initVersion();

src/java.base/share/classes/jdk/internal/util/OperatingSystem.java line 157:

> 155:             return Version.parse(osVer);
> 156:         } catch (IllegalArgumentException ile) {
> 157:             throw new AssertionError("os.version malformed: " + osVer);

This should probably preserve the `IllegalArgumentException` stack trace:
Suggestion:

        } catch (IllegalArgumentException iae) {
            throw new AssertionError("os.version malformed: " + osVer, iae);


And maybe throw `InternalError` instead:
Suggestion:

        } catch (IllegalArgumentException iae) {
            throw new InternalError("os.version malformed: " + osVer, iae);

test/jdk/jdk/internal/util/VersionTest.java line 96:

> 94:                 Arguments.of(new Version(3, 3, 1), new Version(3, 3, 1), 0),
> 95:                 Arguments.of(new Version(3, 3, 1), new Version(3, 3, 0), 1),
> 96:                 Arguments.of(new Version(3, 3, 0), new Version(3, 3, 1), -1)

This should probably include tests for differing `major` versions as well:
Suggestion:

                Arguments.of(new Version(2, 1), new Version(2, 1), 0),
                Arguments.of(new Version(2, 1), new Version(2, 0), +1),
                Arguments.of(new Version(2, 0), new Version(2, 1), -1),
                Arguments.of(new Version(3, 3, 1), new Version(3, 3, 1), 0),
                Arguments.of(new Version(3, 3, 1), new Version(3, 3, 0), +1),
                Arguments.of(new Version(3, 3, 0), new Version(3, 3, 1), -1),
                Arguments.of(new Version(2, 0), new Version(3, 0), -1),
                Arguments.of(new Version(3, 0), new Version(2, 0), +1)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13584#discussion_r1174347678
PR Review Comment: https://git.openjdk.org/jdk/pull/13584#discussion_r1174346831
PR Review Comment: https://git.openjdk.org/jdk/pull/13584#discussion_r1174345782
PR Review Comment: https://git.openjdk.org/jdk/pull/13584#discussion_r1174347449