8317370: JavaFX runtime version is wrong at runtime

[jgneff]

Please review these changes to the build timestamp and to the JavaFX runtime version available in the system properties.

---

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-8317370: JavaFX runtime version is wrong at runtime (Bug - P3)

Reviewers

• Kevin Rushforth (@kevinrushforth - Reviewer)
• Joeri Sykora (@tiainen - Author)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1253

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1253.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jgneff! 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.

[kevinrushforth]

I'll take a closer look, and also test this, when I can. This looks like the right approach to me. It should not cause any problems. with either the earlier fix for JDK-8317308 nor with a somewhat similar fix I did in our (outside the repo) CI build scripts a couple months ago, but it makes both of those fixes unnecessary.

/reviewers 2

[openjdk]

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[mlbridge]

Webrevs

• 01: Full - Incremental (70c0de1c)
• 00: Full (4d9a8ebe)

[jgneff]

This pull request makes three changes:

1. The ISO 8601 extended format of the build date and time is now used only in the --date option of the jmod command, as required. The historical custom format of "yyyy-MM-dd-HHmmss" is used everywhere else for the build timestamp.

2. The build file's short and long version strings, RELEASE_VERSION_SHORT and RELEASE_VERSION_LONG, are now added directly to the VersionInfo.java template file for use as the run-time values of the system properties javafx.version and javafx.runtime.version. This change prevents the values from getting out of sync again between their build-time and run-time values.

3. Four test cases are added to VersionInfoTest.java. Two of the tests verify that the JavaFX version and run-time version have the correct format. The other two tests verify that their values at run time are equal to their values in the static file javafx.properties created at build time and found in the JavaFX SDK and in the javafx.base modular JAR file. These test cases prevent this error from happening again.

[jgneff]

@FlorianKirmaier Florian, I would appreciate any comments you might have, too, or even a formal review if you have the time. Don't feel obligated, though, as you've already helped enough by discovering and patching the bug. 🏆

[kevinrushforth]

@arapte or @johanvos Can one of you also review this?

[johanvos]

@tiainen can you review?

[kevinrushforth]

The fix and new test both look good. Testing is green.

Can you also change the following in UPDATING-VERSION.md?

modify the testMajorVersion method to increment the feature version number
from `N` to `N+1`.

to something like:

modify the `FEATURE` variable to increment the feature version number
from `N` to `N+1`.

[jgneff]

Thanks, Kevin. I'll fix that.

To all reviewers: I added tests of the method VersionInfo.setupSystemProperties to the test cases in my personal repository. It's kind of overkill, and not necessary to prevent this bug, but let me know if I should add them anyway.

[kevinrushforth]

To all reviewers: I added tests of the method VersionInfo.setupSystemProperties to the test cases in my personal repository. It's kind of overkill, and not necessary to prevent this bug, but let me know if I should add them anyway.

Yeah, I probably wouldn't add them.

[kevinrushforth]

Looks good.

[FlorianKirmaier]

Based on the changes, it looks great - fewer variables, less code, more tests.
It's now also easier to follow, what the "no_hudson" flag does.

The ":" is now also removed from the version String - so my previous change would no longer be required.
But I guess it doesn't hurt either.

I tested making a build, and it works well!

So from my point of view, it looks quite great!

[tiainen]

Tested on our build infrastructure and build and tests all worked fine.

[openjdk]

@jgneff 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:

8317370: JavaFX runtime version is wrong at runtime

Reviewed-by: kcr, sykora

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 7 new commits pushed to the master branch:

• 6d2595a: 8318115: Webkit build fails after gradle 8.4
• f76437d: 8314906: [testbug] Create behavior tests for text input controls
• 4604e87: 8318059: Typo is javafx.scene.Node.usesMirroring comment
• e621d4b: 8314484: Update Gradle to 8.4
• 73e690f: 8314486: JavaFX build uses deprecated features that will be removed in gradle 8
• ec9a11b: 8316590: Rendering artifact after JDK-8311983
• 2ec3343: 8313628: Column drag header, overlay and line are not correctly aligned

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

[jgneff]

/integrate

[openjdk]

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

• 6d2595a: 8318115: Webkit build fails after gradle 8.4
• f76437d: 8314906: [testbug] Create behavior tests for text input controls
• 4604e87: 8318059: Typo is javafx.scene.Node.usesMirroring comment
• e621d4b: 8314484: Update Gradle to 8.4
• 73e690f: 8314486: JavaFX build uses deprecated features that will be removed in gradle 8
• ec9a11b: 8316590: Rendering artifact after JDK-8311983
• 2ec3343: 8313628: Column drag header, overlay and line are not correctly aligned

Your commit was automatically rebased without conflicts.

[openjdk]

@jgneff Pushed as commit 1616924.

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

[jgneff]

/backport jfx21u

[openjdk]

@jgneff Could not automatically backport 16169240 to openjdk/jfx21u due to conflicts in the following files:

• modules/javafx.base/src/test/java/test/com/sun/javafx/runtime/VersionInfoTest.java

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jfx21u. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jfx21u.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b jgneff-backport-16169240

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jfx.git 16169240667876633895b27464eb90033abb6166

# Backport the commit
$ git cherry-pick --no-commit 16169240667876633895b27464eb90033abb6166
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 16169240667876633895b27464eb90033abb6166'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jfx21u with the title Backport 16169240667876633895b27464eb90033abb6166.