jfx icon indicating copy to clipboard operation
jfx copied to clipboard

Published 20 hours ago • verified publisher icon openjdk

8264449: Enable reproducible builds with SOURCE_DATE_EPOCH

Open jgneff opened this issue 4 years ago • 50 comments

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

  • [x] Change must not contain extraneous whitespace
  • [x] 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

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx 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.java.net/jfx 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.java.net/jfx/pull/446.diff

jgneff avatar Mar 30 '21 16:03 jgneff

:wave: 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.

bridgekeeper[bot] avatar Mar 30 '21 16:03 bridgekeeper[bot]

Webrevs

mlbridge[bot] avatar Mar 30 '21 16:03 mlbridge[bot]

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 avatar Mar 30 '21 16:03 jgneff

I think there might be a Skara bug.

No, no bug. Sorry about that. I just don't know how GitHub works. :frowning_face: 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.

jgneff avatar Mar 30 '21 19:03 jgneff

/reviewers 2

kevinrushforth avatar Mar 31 '21 13:03 kevinrushforth

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

openjdk[bot] avatar Mar 31 '21 13:03 openjdk[bot]

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.

kevinrushforth avatar Mar 31 '21 13:03 kevinrushforth

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 avatar Mar 31 '21 23:03 jgneff

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 avatar Apr 01 '21 15:04 jgneff

@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?

jgneff avatar Apr 01 '21 16:04 jgneff

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 avatar Apr 01 '21 17:04 kevinrushforth

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.

kevinrushforth avatar Apr 01 '21 17:04 kevinrushforth

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

arun-joseph avatar Apr 01 '21 17:04 arun-joseph

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

kevinrushforth avatar Apr 01 '21 17:04 kevinrushforth

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.

bmwiedemann avatar Apr 01 '21 19:04 bmwiedemann

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).

kevinrushforth avatar Apr 01 '21 20:04 kevinrushforth

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.

jgneff avatar Apr 01 '21 20:04 jgneff

/contributor add Bernhard M. Wiedemann [email protected]

bmwiedemann avatar Apr 01 '21 20:04 bmwiedemann

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

openjdk[bot] avatar Apr 01 '21 20:04 openjdk[bot]

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

kevinrushforth avatar Apr 01 '21 21:04 kevinrushforth

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)

mlbridge[bot] avatar Apr 02 '21 13:04 mlbridge[bot]

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?

That's a good question. I'm hoping the answer is no. So far, the difference with Windows is the linker. The other systems use the GNU linker where it's enough to sort the input object files. The Microsoft linker, though, creates reproducible builds with the flag /experimental:deterministic (or -experimental:deterministic). So on Windows, we just need to figure out where to put the flag. I'm finding that to be a challenge for the WebKit library.

jgneff avatar Apr 02 '21 17:04 jgneff

/contributor add ...

@bmwiedemann I added you as co-author of pull request #422, which is my continuation of your pull request #99 from last year. This pull request #446 temporarily includes our co-authored commit, but those changes will disappear when #422 is integrated and I merge the master branch into this one. Sorry for the confusion.

jgneff avatar Apr 03 '21 01:04 jgneff

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

Thank you, Arun, for that helpful suggestion. I updated this pull request for the WebKit library on Windows and ran my tests again, as described below.

I saved the output of the first build in the build1.log file and the build1 directory:

$ export SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct)
$ 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 \
  | tee build1.log
$ mv build build1

Then I ran another clean build, this time saving the output in the build2.log file and the build2 directory:

$ bash gradlew cleanAll
$ 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 \
  | tee build2.log
$ mv build build2

I copied the builds to my Linux workstation and ran the following command on the JMOD files for each pair of builds:

$ strip-nondeterminism -v -T $SOURCE_DATE_EPOCH build?/jmods/*.jmod
Normalizing build1/jmods/javafx.base.jmod
Normalizing build1/jmods/javafx.controls.jmod
Normalizing build1/jmods/javafx.fxml.jmod
Normalizing build1/jmods/javafx.graphics.jmod
Normalizing build1/jmods/javafx.media.jmod
Normalizing build1/jmods/javafx.swing.jmod
Normalizing build1/jmods/javafx.web.jmod
Normalizing build2/jmods/javafx.base.jmod
Normalizing build2/jmods/javafx.controls.jmod
Normalizing build2/jmods/javafx.fxml.jmod
Normalizing build2/jmods/javafx.graphics.jmod
Normalizing build2/jmods/javafx.media.jmod
Normalizing build2/jmods/javafx.swing.jmod
Normalizing build2/jmods/javafx.web.jmod

After normalizing the JMOD files, the output of the two builds on each of the three operating systems is identical:

$ diff -qr linux/build1 linux/build2
$ diff -qr macos/build1 macos/build2
$ diff -qr win10/build1 win10/build2
$

The following commands show the count of identical output files for each system:

$ diff -sr linux/build1 linux/build2 | wc -l
10676
$ diff -sr macos/build1 macos/build2 | wc -l
10680
$ diff -sr win10/build1 win10/build2 | wc -l
10633

Considering the time it takes to run all these builds, I'd like to take a week to get feedback and mull over the code changes before anyone spends a lot of time testing. I wish the changes for Windows were not so scattered about, for example.

jgneff avatar Apr 04 '21 17:04 jgneff

Thank you, Alexander and Arun, for looking over the Makefiles and verifying the builds.

I'd like to take a week to get feedback and mull over the code changes ...

After realizing how little I knew about Gradle, I did some research last week and took four courses on Groovy, Gradle, and the Gradle Java Plugin. Although I now see a couple things I could change, I'm still happy with the commits as they are. I welcome any comments or questions.

Once #422 and this pull request are integrated, I can follow up with changes to make the all task reproducible (fixing javafx-sdk-17.zip) and to remove the build path from the .bss files (fixing number 4 in the first comment).

jgneff avatar Apr 12 '21 18:04 jgneff

IEEE Software just published a good article that describes the problems solved in part by this pull request. The article is called "Reproducible Builds: Increasing the Integrity of Software Supply Chains," by Chris Lamb and Stefano Zacchiroli. It's an easy read of 10 pages, available at the links below:

jgneff avatar Apr 14 '21 20:04 jgneff

Pending review by @johanvos -- I bumped the number of reviewers, so he will have a change to review before it is approved.

/reviewers 3

kevinrushforth avatar Apr 16 '21 19:04 kevinrushforth

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

openjdk[bot] avatar Apr 16 '21 19:04 openjdk[bot]

Note that this PR is dependent on PR #422 .

@jgneff As an alternative, you could close that PR (since all of the changes from that one are currently included here), and list both issues in this PR with Skara's /issue add ... command. It's up to you.

kevinrushforth avatar Apr 16 '21 19:04 kevinrushforth

As an alternative, you could close that PR ... and list both issues in this PR ...

I prefer to keep them separate because they have distinct attributions and descriptions. Few developers will read #422 if we merge them.

They also pose distinct questions for reviewers. The implicit question in #422 is, should we define an external environment variable or just remove the build timestamp entirely? The implicit question in this pull request is, given the environment variable, should we also use it as a flag for reproducible builds? Furthermore, which sources of non-determinism should be removed unconditionally, and which should be conditional on the flag?

I chose conservative answers to those questions in my changes, but I'm open to other suggestions.

jgneff avatar Apr 17 '21 17:04 jgneff