Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

8264449: Enable reproducible builds with SOURCE_DATE_EPOCH #446

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

jgneff
Copy link
Member

@jgneff jgneff commented Mar 30, 2021

This pull request allows for reproducible builds of JavaFX on Linux, macOS, and Windows by defining the SOURCE_DATE_EPOCH environment variable. For example, the following commands create a reproducible build:

$ export SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct)
$ bash gradlew sdk jmods javadoc
$ strip-nondeterminism -v -T $SOURCE_DATE_EPOCH build/jmods/*.jmod

The three commands:

  1. set the build timestamp to the date of the latest source code change,
  2. build the JavaFX SDK libraries, JMOD archives, and API documentation, and
  3. recreate the JMOD files with stable file modification times and ordering.

The third command won't be necessary once Gradle can build the JMOD archives or the jmod tool itself has the required support. For more information on the environment variable, see the SOURCE_DATE_EPOCH page. For more information on the command to recreate the JMOD files, see the strip-nondeterminism repository. I'd like to propose that we allow for reproducible builds in JavaFX 17 and consider making them the default in JavaFX 18.

Fixes

There are at least four sources of non-determinism in the JavaFX builds:

  1. Build timestamp

    The class com.sun.javafx.runtime.VersionInfo in the JavaFX Base module stores the time of the build. Furthermore, for builds that don't run on the Hudson continuous integration tool, the class adds the build time to the system property javafx.runtime.version.

  2. Modification times

    The JAR, JMOD, and ZIP archives store the modification time of each file.

  3. File ordering

    The JAR, JMOD, and ZIP archives store their files in the order returned by the file system. The native shared libraries also store their object files in the order returned by the file system. Most file systems, though, do not guarantee the order of a directory's file listing.

  4. Build path

    The class com.sun.javafx.css.parser.Css2Bin in the JavaFX Graphics module stores the absolute path of its .css input file in the corresponding .bss output file, which is then included in the JavaFX Controls module.

This pull request modifies the Gradle and Groovy build files to fix the first three sources of non-determinism. A later pull request can modify the Java files to fix the fourth.


Progress

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

Issues

  • JDK-8264449: Enable reproducible builds with SOURCE_DATE_EPOCH
  • JDK-8238650: Allow to override buildDate with SOURCE_DATE_EPOCH

Reviewers

Contributors

  • Bernhard M. Wiedemann <javabmw@lsmod.de>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 446

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

Using diff file

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

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 30, 2021

馃憢 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.

@openjdk openjdk bot added the rfr Ready for review label Mar 30, 2021
@mlbridge
Copy link

mlbridge bot commented Mar 30, 2021

Webrevs

@jgneff
Copy link
Member Author

jgneff commented Mar 30, 2021

I think there might be a Skara bug. The pre-submit builds on Linux, macOS, and Windows completed immediately. I think that's because the first of the two commits in this pull request includes the Java Bug ID from another pending pull request, because this pull request is a continuation of that one. I can squash the two commits and force-push the changes, if that would help.

@jgneff
Copy link
Member Author

jgneff commented Mar 30, 2021

I think there might be a Skara bug.

No, no bug. Sorry about that. I just don't know how GitHub works. 鈽癸笍 The pre-submit tests ran two days ago when I pushed the branch to GitHub, so by the time I submitted the pull request, they had finished long ago.

@kevinrushforth
Copy link
Member

/reviewers 2

@openjdk
Copy link

openjdk bot commented Mar 31, 2021

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

@kevinrushforth
Copy link
Member

I recommend trying this with the following gradle flags, to match the settings for production builds:

-PCONF=Release -PPROMOTED_BUILD_NUMBER=NNN -PHUDSON_BUILD_NUMBER=MMM -PHUDSON_JOB_NAME=jfx -PCOMPILE_WEBKIT=true -PCOMPILE_MEDIA=true -PBUILD_LIBAV_STUBS=true

where NNN is the promoted build number that is being built (usually taken from the repo tag) and MMM is the CI build number. You can just pick any two positive numbers for your test builds.

Note that this will build the native media libraries and the native webkit library.

@jgneff
Copy link
Member Author

jgneff commented Mar 31, 2021

I recommend trying this with the following gradle flags, to match the settings for production builds:

Thanks, Kevin. Good news so far. I'm posting the Linux results while I run the macOS and Windows builds.

Linux

I ran the following commands twice, moving the build directory to build1 and then build2 to save their output:

$ bash gradlew clean
$ bash gradlew -PCONF=Release -PPROMOTED_BUILD_NUMBER=5 \
    -PHUDSON_BUILD_NUMBER=101 -PHUDSON_JOB_NAME=jfx \
    -PCOMPILE_WEBKIT=true -PCOMPILE_MEDIA=true \
    -PBUILD_LIBAV_STUBS=true sdk jmods javadoc

Then I changed the Hudson job number with -PHUDSON_BUILD_NUMBER=102, ran the build a third time, and moved build to build3. I also ran strip-nondeterminism as shown in the first comment of this pull request.

The first result is as hoped, and the second result is as expected:

$ diff -qr build1 build2
$ diff -qr build2 build3
Files build2/jmods/javafx.base.jmod
   and build3/jmods/javafx.base.jmod
   differ
Files build2/modular-sdk/modules/javafx.base/com/sun/javafx/runtime/VersionInfo.class
  and build3/modular-sdk/modules/javafx.base/com/sun/javafx/runtime/VersionInfo.class
  differ
Files build2/modular-sdk/modules_src/javafx.base/com/sun/javafx/runtime/VersionInfo.java
  and build3/modular-sdk/modules_src/javafx.base/com/sun/javafx/runtime/VersionInfo.java
  differ
Files build2/publications/javafx.base-linux.jar
  and build3/publications/javafx.base-linux.jar
  differ
Files build2/sdk/lib/javafx.base.jar
  and build3/sdk/lib/javafx.base.jar
  differ
Files build2/sdk/lib/src.zip
  and build3/sdk/lib/src.zip
  differ

You have to appreciate the irony of adding all this information to the build 鈥 the time, the path, even the job number 鈥 so that we can uniquely identify a build by its output. Meanwhile, if we didn't add this information, our builds could be uniquely identified by a single Git tag.

@jgneff
Copy link
Member Author

jgneff commented Apr 1, 2021

There's good news and bad news. Good news first.

macOS

The two comparisons of the three builds on macOS were similar to those on Linux:

$ diff -qr build1 build2
$ diff -qr build2 build3
Files build2/jmods/javafx.base.jmod
  and build3/jmods/javafx.base.jmod
  differ
Files build2/modular-sdk/modules/javafx.base/com/sun/javafx/runtime/VersionInfo.class
  and build3/modular-sdk/modules/javafx.base/com/sun/javafx/runtime/VersionInfo.class
  differ
Files build2/modular-sdk/modules_src/javafx.base/com/sun/javafx/runtime/VersionInfo.java
  and build3/modular-sdk/modules_src/javafx.base/com/sun/javafx/runtime/VersionInfo.java
  differ
Files build2/publications/javafx.base-mac.jar
  and build3/publications/javafx.base-mac.jar
  differ
Files build2/sdk/lib/javafx.base.jar
  and build3/sdk/lib/javafx.base.jar
  differ
Files build2/sdk/lib/src.zip
  and build3/sdk/lib/src.zip
  differ

Windows

The Windows build, on the other hand, failed to produce identical copies of the media and WebKit shared libraries:

$ diff -qr build1 build2
Files build1/jmods/javafx.media.jmod
  and build2/jmods/javafx.media.jmod
  differ
Files build1/jmods/javafx.web.jmod
  and build2/jmods/javafx.web.jmod
  differ
Files build1/modular-sdk/modules_libs/javafx.media/fxplugins.dll
  and build2/modular-sdk/modules_libs/javafx.media/fxplugins.dll
  differ
Files build1/modular-sdk/modules_libs/javafx.media/glib-lite.dll
  and build2/modular-sdk/modules_libs/javafx.media/glib-lite.dll
  differ
Files build1/modular-sdk/modules_libs/javafx.media/gstreamer-lite.dll
  and build2/modular-sdk/modules_libs/javafx.media/gstreamer-lite.dll
  differ
Files build1/modular-sdk/modules_libs/javafx.media/jfxmedia.dll
  and build2/modular-sdk/modules_libs/javafx.media/jfxmedia.dll
  differ
Files build1/modular-sdk/modules_libs/javafx.web/jfxwebkit.dll
  and build2/modular-sdk/modules_libs/javafx.web/jfxwebkit.dll
  differ
Files build1/publications/javafx.media-win.jar
  and build2/publications/javafx.media-win.jar
  differ
Files build1/publications/javafx.web-win.jar
  and build2/publications/javafx.web-win.jar
  differ
Files build1/sdk/bin/fxplugins.dll
  and build2/sdk/bin/fxplugins.dll
  differ
Files build1/sdk/bin/glib-lite.dll
  and build2/sdk/bin/glib-lite.dll
  differ
Files build1/sdk/bin/gstreamer-lite.dll
  and build2/sdk/bin/gstreamer-lite.dll
  differ
Files build1/sdk/bin/jfxmedia.dll
  and build2/sdk/bin/jfxmedia.dll
  differ
Files build1/sdk/bin/jfxwebkit.dll
  and build2/sdk/bin/jfxwebkit.dll
  differ

I didn't bother running the third build with -PHUDSON_BUILD_NUMBER=102. I assume CMake would be the same across platforms. I'm hoping it's just a missing /experimental:deterministic somewhere for the Windows linker. I'll track it down.

@jgneff
Copy link
Member Author

jgneff commented Apr 1, 2021

@kevinrushforth I found the Makefiles building the media libraries for Windows. I can fix those, but there's also libxml and libxslt that invoke the Windows linker here:

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/win32/Makefile.msvc
modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/src/win32/Makefile.msvc

They both state at the top, "There should never be a need to modify anything below this line." Are those files directly from their upstream sources? If so, what's our policy on patching them while waiting for fixes?

@kevinrushforth
Copy link
Member

kevinrushforth commented Apr 1, 2021

The WebKit build is complicated (to say the least). All of the WebKit components, including the additional third-party dependencies in /Source/ThirdParty, are built using cmake. I think any changes would involve passing flags to cmake in build.gradle.

@arun-Joseph might be able to comment on that.

@kevinrushforth
Copy link
Member

I should add that if changes are needed to the Makefile.msvc files to accept extra link flags to be passed in, then it would be fine to modify them.

@arun-Joseph
Copy link
Member

It would be better to add the flag in libxml/CMakeLists.txt or in build.gradle via cmakeArgs.

@kevinrushforth
Copy link
Member

Ideally the latter, if feasible, so that the logic to handle SOURCE_DATE_EPOCH is more localized.

Enable reproducible builds of the native media shared libraries for
Windows when SOURCE_DATE_EPOCH is defined. The libraries are:

  fxplugins.dll
  glib-lite.dll
  gstreamer-lite.dll
  jfxmedia.dll
@bmwiedemann
Copy link

IMHO, it would make sense to merge any clear improvement of the status-quo and debug+fix more in a later PR. "Perfect is the enemy of good."

Regarding Webkit, I know of
https://bugs.webkit.org/show_bug.cgi?id=174540
https://bugs.webkit.org/show_bug.cgi?id=188738
https://build.opensuse.org/request/show/667729
but your version is probably not that old.

@kevinrushforth
Copy link
Member

Given the level of effort to test this, I would recommend minimizing the number of separate fixes for this enhancement. If WebKit turns out to be too big a can of worms, then it might make sense to do everything else with this fix and file a follow-on for WebKit.

Regarding Webkit...but your version is probably not that old.

Right, we aren't nearly that old. We just updated WebKit a little over 2 months ago with recent sources (we update WebKit every six months).

@jgneff
Copy link
Member Author

jgneff commented Apr 1, 2021

IMHO, it would make sense to merge any clear improvement of the status-quo and debug+fix more in a later PR.

I agree that we will have to draw the line somewhere, but right now I'm down to just one file on one operating system out of 10,633 files in the build output! (It's just jfxwebkit.dll on Windows.) It would be a shame to give up now.

I would, though, like to postpone tests of static builds and builds for Android and iOS. My priorities for this pull request are:

  1. Linux distributions building JavaFX
  2. Developers building JavaFX from the repository source with the build defaults
  3. Oracle, Gluon, and BellSoft building JavaFX for their customers and for downloading

I would be happy to satisfy just the first group, but we might be close to getting all three.

@bmwiedemann
Copy link

/contributor add Bernhard M. Wiedemann javabmw@lsmod.de

@openjdk
Copy link

openjdk bot commented Apr 1, 2021

@bmwiedemann Only the author (@jgneff) is allowed to issue the contributor command.

@kevinrushforth
Copy link
Member

@sashamatveev Can you look at the changes to the media Makefiles?

Copy link
Member

@sashamatveev sashamatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Media makefiles look fine.

@mlbridge
Copy link

mlbridge bot commented Apr 2, 2021

Mailing list message from Eric Bresie on openjfx-dev:

Silly question, is the difference with Windows just the nature of the native support on each platform (Unix based vs Windows) and libraries used as part of that?

Eric Bresie
Ebresie at gmail.com (mailto:Ebresie at gmail.com)

@jgneff
Copy link
Member Author

jgneff commented Jan 3, 2023

The results of my latest tests are shown below for each type of build:

System Develop Actions Release Notes
Linux Release build failed
macOS Some shared libraries differ
Windows Release build failed

where:

means that the unit tests ran successfully and that the files in the build directory were identical between two builds in the same project directory on the same system,

means there were random differences in the native shared libraries between the two builds, such as libjavafx_iio.dylib, libglib-lite.dylib, and libjfxwebkit.dylib, and

means the build failed due to an error unrelated to this pull request.

The build types are defined by the Shell scripts in the repository jgneff/jfxbuild. The Release build failed on Linux because libav.org is not resolving. The Release build failed on Windows due to compiler error C2327.

@jgneff
Copy link
Member Author

jgneff commented Jan 3, 2023

I also confirmed that the builds are reproducible on all of the following Linux architectures running Ubuntu 18.04 LTS when built from the same project path:

Architecture Develop
amd64
arm64
armhf
i386
ppc64el
s390x

where the Develop build script includes:

SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct)
export SOURCE_DATE_EPOCH
bash gradlew --no-daemon -PPROMOTED_BUILD_NUMBER=14 \
    -PRELEASE_SUFFIX="-ea" -PJDK_DOCS="$JDK_DOCS" \
    sdk jmods javadoc

I just discovered that the --date argument for the jmod tool was back-ported to JDK 17, so I didn't need to wait for JDK 19 after all! Oh well. I think this pull request is now ready for a final review. I welcome any other comments or suggestions.

@johanvos
Copy link
Collaborator

johanvos commented Jan 5, 2023

I think this pull request is now ready for a final review. I welcome any other comments or suggestions.

Great. I'll review tomorrow.

@jgneff
Copy link
Member Author

jgneff commented Jan 5, 2023

Great. I'll review tomorrow.

Thanks, Johan. I'll rerun my failing Linux build once your pull request #989 is integrated. (Thank you for doing that!) Are you seeing the error I'm getting on Windows with the full production release build?

@jgneff
Copy link
Member Author

jgneff commented Jan 10, 2023

I tested the Linux "Release" build again now that pull request #989 is integrated. The updated results are shown below:

System Develop Actions Release Notes
Linux
macOS Some shared libraries differ
Windows Release build failed

where , , and have the meanings described earlier. I'll test the Windows "Release" build next to see whether it's still failing, and I'll update this comment directly if there are any changes.

Edit: The Windows "Release" build is still failing for me with the following compiler error, even after upgrading to Microsoft Visual Studio Community 2022 version 17.4.4 (from version 17.4.3):

modules\javafx.web\src\main\native\Source\WebCore\Modules/mediacapabilities/MediaCapabilities.cpp(323): error C2327: 'WebCore::MediaCapabilities::m_encodingTasks': is not a type name, static, or enumerator

Copy link
Collaborator

@johanvos johanvos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a good step towards reproducible builds.
At the very least, I can't spot any regression.

@jgneff
Copy link
Member Author

jgneff commented Jan 11, 2023

I just discovered that the --date argument for the jmod tool was back-ported to JDK 17, so I didn't need to wait for JDK 19 after all! Oh well.

Actually, it turns out I did have to wait for JDK 19 after all. Before reaching version 19.0.1, the boot JDK for building JavaFX (jfx.build.jdk.version) went from version 17.0.2 to 18, 18.0.1, and 18.0.2, none of which had the feature. The feature was back ported from version 19 to 17.0.4 or later. See JDK-8283182. That makes me feel a little better about the delay. 馃槃

Copy link
Member

@kevinrushforth kevinrushforth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll need a bit more time to run CI test builds again. I'm thinking it might be better to get this in early in JavaFX 21 rather than late in JavaFX 20 anyway.

// Requires OpenJDK 19
// https://github.com/openjdk/jdk/pull/6481
// args("--date", buildTimestamp)
args("--date", buildTimestamp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the minimum boot JDK for building JavaFX is still JDK 17 GA, you will need to qualify this with a check for if (jdk19OrLater). See the following for a pattern to use to define that flag:

jfx/build.gradle

Lines 699 to 704 in 1e4c083

// For example, we could define a "jdk18OrLater" property as
// follows that could then be used to implement conditional build
// logic based on whether we were running on JDK 18 or later,
// should the need arise.
// def status = compareJdkVersion(jdkVersion, "18")
// ext.jdk18OrLater = (status >= 0)

Btw, the comment in that block should read "based on whether we are building on JDK 18 or later".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The jmodTask checks for JDK 19 or later. Let me know if you think we should add the --date argument also for versions of JDK 17 later than 17.0.4. My tests indicate that JDK 17.0.5, for example, has enough back-ported features to get reproducible builds of JavaFX in some limited circumstances. I figured it wasn't worth it, though, considering we'll need JDK 20 eventually, anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it isn't worth it. The goal is to make it still buildable with JDK 17, and what you've done does that. Thanks.

@jgneff
Copy link
Member Author

jgneff commented Jan 11, 2023

I'll need a bit more time to run CI test builds again. I'm thinking it might be better to get this in early in JavaFX 21 rather than late in JavaFX 20 anyway.

I agree, especially considering the change required for JDK 17 GA. Besides, we're not going to have full cross-system reproducible builds until we start building JavaFX with JDK 20 due to Bug JDK-8292892.

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 31, 2023

@jgneff This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@kevinrushforth
Copy link
Member

I'd like to see this get in soon. I'll do some test builds next week.

@kevinrushforth
Copy link
Member

@jgneff Can you merge in the latest master? I'll do this in my test branch, but it would still be helpful.

Include two commits that fix WebKit build failures on Windows and macOS:

  8282359: Intermittent WebKit build failure on Windows:
           C1090: PDB API call failed, error code 23
  8286089: Intermittent WebKit build failure on macOS in JavaScriptCore
@kevinrushforth
Copy link
Member

I started doing some testing today. I verified that without SOURCE_DATE_EPOCH set, the builds are as expected with a couple minor diffs.

  1. The jar index in javafx-swt.jar is gone. This is fine (as mentioned earlier), since jar indexing is not useful for modular jars. There is an in-progress RFE remove some support for it in the JDK with JDK-8302819, so I was going to file an issue for us to stop using it, but now I don't have to.
  2. The format of VersionInfo.BUILD_TIMESTAMP, which is used in constructing the javafx.runtime.version System property for dev builds, has changed to an ISO date -- 2023-04-04T15:11:59Z rather than 2023-04-04-151159. Since the : is not legal for Java version strings, it is possible (though unlikely), that some app is parsing this in a way that might run into porblems. This should probably be fixed.

I then did a build with SOURCE_DATE_EPOCH. On each of three machines I tried (one each of Windows, Linux, and Mac / x64), a pair of builds with the same SOURCE_DATE_EPOCH were identical except for the native WebKit library, which was different between the two builds on Windows and Linux (Mac was fine). All other artifacts (except, by extension, javafx.web.jmod) were identical.

Unless there is an easy solution, I think addressing the jfxwebkit native library differences on Windows and Linux could be handled via a follow-on issue.

I'll do a review of the changes later this week along with some more testing.

@jgneff
Copy link
Member Author

jgneff commented Apr 5, 2023

I started doing some testing today.

Thank you, Kevin. I'm almost done with another round of my own testing and will post my results, too.

  1. The format of VersionInfo.BUILD_TIMESTAMP, which is used in constructing the javafx.runtime.version System property for dev builds, has changed to an ISO date -- 2023-04-04T15:11:59Z rather than 2023-04-04-151159. Since the : is not legal for Java version strings, it is possible (though unlikely), that some app is parsing this in a way that might run into problems. This should probably be fixed.

It always bothered me that developer builds get a timestamp suffix while the official builds do not, even when built from the same Git tag. For example:

$ cat opt/javafx-sdk-20/lib/javafx.properties 
javafx.version=20
javafx.runtime.version=20+19
javafx.runtime.build=19
$ cat /snap/openjfx/current/sdk/lib/javafx.properties 
javafx.version=20
javafx.runtime.version=20+19-2023-03-07-164252
javafx.runtime.build=19

That makes it impossible for a developer to reproduce an identical copy of the official build.

Would it be more appropriate to use the output of git describe? For example, below is the command's output for this pull request branch and for the 21+11 tag:

$ git describe
21+11-25-ge42a070947
$ git describe allow-reproducible-builds 
21+11-25-ge42a070947
$ git describe 21+11
21+11

The JDK does something similar for its release file:

$ cat opt/jdk-20/release 
IMPLEMENTOR="Oracle Corporation"
JAVA_VERSION="20"
JAVA_VERSION_DATE="2023-03-21"
LIBC="gnu"
  ...
OS_ARCH="x86_64"
OS_NAME="Linux"
SOURCE=".:git:82749901b149"

@kevinrushforth
Copy link
Member

  1. The format of VersionInfo.BUILD_TIMESTAMP, which is used in constructing the javafx.runtime.version System property for dev builds, has changed to an ISO date -- 2023-04-04T15:11:59Z rather than 2023-04-04-151159. Since the : is not legal for Java version strings, it is possible (though unlikely), that some app is parsing this in a way that might run into problems. This should probably be fixed.

It always bothered me that developer builds get a timestamp suffix while the official builds do not, even when built from the same Git tag. For example:
...

We could consider a future RFE to adjust the format of the version string, although I note that the JDK also adds the date code to the version string only for developer builds.

$ cat opt/javafx-sdk-20/lib/javafx.properties 
javafx.version=20
javafx.runtime.version=20+19
javafx.runtime.build=19
$ cat /snap/openjfx/current/sdk/lib/javafx.properties 
javafx.version=20
javafx.runtime.version=20+19-2023-03-07-164252
javafx.runtime.build=19

That makes it impossible for a developer to reproduce an identical copy of the official build.

No it doesn't. What it does mean is that they would need to set the following properties to the same values that were used for the official build:

HUDSON_JOB_NAME
HUDSON_BUILD_NUMBER
PROMOTED_BUILD_NUMBER
MILESTONE_FCS

and, of course:

CONF=Release

Would it be more appropriate to use the output of git describe? For example, below is the command's output for this pull request branch and for the 21+11 tag:

$ git describe
21+11-25-ge42a070947
$ git describe allow-reproducible-builds 
21+11-25-ge42a070947
$ git describe 21+11
21+11

Not for the version string, but we could consider a future RFE to add something along these lines to the lib/javafx.properties file (which is the closest thing we have to the JDK's release file).

@jgneff
Copy link
Member Author

jgneff commented Apr 6, 2023

Thanks for finding this, Kevin. For my own reference, the format of the Java version string is explained in the API specification of Runtime.Version, which defines the $OPT additional build information as matching the regular expression ([-a-zA-Z0-9.]+). No colon characters are permitted.

We could consider a future RFE to adjust the format of the version string, although I note that the JDK also adds the date code to the version string only for developer builds.

The JDK removed the timestamp in version 9 for JDK-8170632, so now they look something like 20-internal+36-adhoc.root.build. More to your point, I now see that the JDK also adds additional information by default, just like JavaFX.

No it doesn't. What it does mean is that they would need to set the following properties to the same values that were used for the official build:

Right. Sorry, I should have known that after all the "release" builds I do for this pull request!

I would like to replace the current non-standard build timestamp with one that conforms to ISO 8601. This pull request uses the ISO 8601 extended format, but the standard also defines a basic format that does result in a valid Java version OPT field. The consensus seems to be that both the date and time must use the same format type, either extended or basic, which makes the current JavaFX build timestamp non-standard.

What do you think about simply replacing this pull request's extended format with the basic format?

See the following sample program for details:

import java.text.SimpleDateFormat;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import java.util.Date;

public class Timestamp {

    private static final String OPT = "([-a-zA-Z0-9.]+)";
    private static final String OK = " (okay)";
    private static final String NOT_OK = " (NOT okay)";

    public static void main(String[] args) {
        // ISO 8601 extended format
        var buildInstant = Instant.now().truncatedTo(ChronoUnit.SECONDS);
        String extended = buildInstant.toString();

        // Non-standard format used in the current JavaFX release
        var buildDate = Date.from(buildInstant);
        var format = new SimpleDateFormat("yyyy-MM-dd-HHmmss");
        String current = format.format(buildDate);

        // ISO 8601 basic format
        var localTime = LocalDateTime.ofInstant(buildInstant, ZoneOffset.UTC);
        var formatter = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'");
        String basic = localTime.format(formatter);

        System.out.print("ISO 8601 extended format = " + extended);
        System.out.println(extended.matches(OPT) ? OK : NOT_OK);

        System.out.print("Current JavaFX format = " + current);
        System.out.println(current.matches(OPT) ? OK : NOT_OK);

        System.out.print("ISO 8601 basic format = " + basic);
        System.out.println(basic.matches(OPT) ? OK : NOT_OK);
    }
}

An example of its output is shown below:

$ java Timestamp
ISO 8601 extended format = 2023-04-06T04:20:16Z (NOT okay)
Current JavaFX format = 2023-04-05-212016 (okay)
ISO 8601 basic format = 20230406T042016Z (okay)

@jgneff
Copy link
Member Author

jgneff commented Apr 7, 2023

I reverted the version timestamp to the original format. The version looks like this for "release" early-access builds:

$ cat build5/sdk/lib/javafx.properties
javafx.version=21-ea
javafx.runtime.version=21-ea+12
javafx.runtime.build=12

and like this for developer builds:

$ cat build1/sdk/lib/javafx.properties
javafx.version=21-internal
javafx.runtime.version=21-internal+0-2023-04-06-232926
javafx.runtime.build=0

The only difference is that the timestamp is localized to UTC, while before it was in the local time of the build machine.

The jmod --date option requires the ISO 8601 extended format, so we need at least two different formats for the timestamp. Meanwhile, Java has no built-in support for parsing an ISO 8601 string in basic format, so both the current non-standard format and the standard basic format need a DateTimeFormatter with a custom pattern. There's no advantage from a parsing perspective in using the standard format:

    // Parses the format in the current JavaFX release
    var currentFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd-HHmmss");
    var localTime = LocalDateTime.parse(timestamp, currentFormatter);

    // Parses the ISO 8601 basic format
    var basicFormatter = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmssX");
    var zonedTime = ZonedDateTime.parse(timestamp, basicFormatter);

@jgneff
Copy link
Member Author

jgneff commented Apr 7, 2023

For the record, here's the latest version of my program that tests the timestamp formats:

import java.time.Instant;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;

public class Timestamp {

    private static final String OPT = "([-a-zA-Z0-9.]+)";
    private static final String OK = " (OK)";
    private static final String NOT_OK = " (FAILED)";

    public static void main(String[] args) {
        var buildInstant = Instant.now().truncatedTo(ChronoUnit.SECONDS);

        // Creates the timestamp in the ISO 8601 extended format
        String extended = buildInstant.toString();

        // Creates the timestamp in the current non-stnadard format
        var zonedTime = ZonedDateTime.ofInstant(buildInstant, ZoneOffset.UTC);
        var currentFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd-HHmmss");
        String current = zonedTime.format(currentFormatter);

        // Creates the timestamp in the ISO 8601 basic format
        zonedTime = ZonedDateTime.ofInstant(buildInstant, ZoneOffset.UTC);
        var basicFormatter = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmssX");
        String basic = zonedTime.format(basicFormatter);

        // Prints the timestamps and checks them for the version OPT field
        System.out.print("ISO 8601 extended format = " + extended);
        System.out.println(extended.matches(OPT) ? OK : NOT_OK);
        System.out.print("Current JavaFX format    = " + current);
        System.out.println(current.matches(OPT) ? OK : NOT_OK);
        System.out.print("ISO 8601 basic format    = " + basic);
        System.out.println(basic.matches(OPT) ? OK : NOT_OK);
        System.out.println();

        // Parses the timestamp in the ISO 8601 extended format
        zonedTime = ZonedDateTime.parse(extended);
        System.out.println("Parsed extended time = " + zonedTime);

        // Parses the timestamp in the current non-standard format
        var localTime = LocalDateTime.parse(current, currentFormatter);
        System.out.println("Parsed current time  = " + localTime);

        // Parses the timestamp in the ISO 8601 basic format
        zonedTime = ZonedDateTime.parse(basic, basicFormatter);
        System.out.println("Parsed basic time    = " + zonedTime);
    }
}

Below is a sample of its output:

$ java Timestamp
ISO 8601 extended format = 2023-04-07T16:01:16Z (FAILED)
Current JavaFX format    = 2023-04-07-160116 (OK)
ISO 8601 basic format    = 20230407T160116Z (OK)

Parsed extended time = 2023-04-07T16:01:16Z
Parsed current time  = 2023-04-07T16:01:16
Parsed basic time    = 2023-04-07T16:01:16Z

@kevinrushforth
Copy link
Member

Your solution using two formatted date strings looks good to me. Thanks for the additional detail.

@jgneff
Copy link
Member Author

jgneff commented Apr 26, 2023

I solved the build failure occurring on Windows by downgrading to Visual Studio 2022 Build Tools version 17.1.0, dated February 15, 2022, from the prior version 17.5.4 that I was using. I opened a bug report, linked below, as a heads-up for when we upgrade Visual Studio.

I'm now using the following versions of the tools:

Toolchains

  • Linux: gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0
  • macOS: Command Line Tools for Xcode 14.2 version 14.2.0.0.1.1668646533
  • Windows: Visual Studio Build Tools 2022 version 17.1.0, build number 17.1.32210.238

Build Tools

  • JDK: OpenJDK Runtime Environment (build 19.0.2+7-44)
  • Ant: Apache Ant(TM) version 1.10.13 compiled on January 4 2023
  • CMake: cmake version 3.26.3

My latest test results are:

System Develop Actions Release Notes
Linux libjfxwebkit.so differs
macOS Just luck, I suspect.
Windows jfxwebkit.dll differs

where:

  • means that all of the files in the build directory were identical between the two consecutive builds, and the unit tests ran successfully, while
  • means that there were differences in the native libraries between the two builds.

I created bug reports to track each of the remaining potential problems:

  • JDK-8306884: Building WebKit on Linux is not deterministic
  • JDK-8306885: Building WebKit on Windows is not deterministic
  • JDK-8306886: Building macOS libraries can be non-deterministic
  • JDK-8306887: Error C2327 while compiling WebKit on Windows

@jgneff
Copy link
Member Author

jgneff commented Apr 28, 2023

I created bug reports to track each of the remaining potential problems:

I forgot the original remaining problem! It's listed as item 4 under Fixes in the description at the top of this pull request and now described by the following bug report:

  • JDK-8307082: Build path is recorded in JavaFX Controls module

@bridgekeeper
Copy link

bridgekeeper bot commented May 26, 2023

@jgneff This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@kevinrushforth
Copy link
Member

Now that the compiler updates are in, I'll resume reviewing and testing this. I'd like to get it in soon.

@jgneff Can you merge in the latest master? This will pick up the recent compiler updates along with any other recent changes. I'll do that anyway when testing, but that way we'll get a GHA run on Mac and Linux with the latest compilers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfr Ready for review
6 participants