On Wed, 26 Apr 2023 04:17:23 GMT, John Neffenger <jgn...@openjdk.org> wrote:

>> 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`][1] page. For more 
>> information on the command to recreate the JMOD files, see the 
>> [`strip-nondeterminism`][2] 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.
>> 
>> [1]: https://reproducible-builds.org/docs/source-date-epoch/
>> [2]: https://salsa.debian.org/reproducible-builds/strip-nondeterminism
>
> John Neffenger has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 23 commits:
> 
>  - Merge branch 'master' into allow-reproducible-builds
>  - Revert format of timestamp in version OPT field
>  - Merge branch 'master' into allow-reproducible-builds
>    
>    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
>  - Merge branch 'master' into allow-reproducible-builds
>  - Support JDK 17 GA or later for building JavaFX
>  - Merge branch 'master' into allow-reproducible-builds
>  - Add '--date' argument for deterministic JMOD files
>  - Merge branch 'master' into allow-reproducible-builds
>  - Merge branch 'master' into allow-reproducible-builds
>  - Comment out 'jmod --date' until building on JDK 19
>    
>    Support for the 'jmod --date' option was added to JDK 19 starting
>    with the 19+2 early-access build, and it was backported to JDK 17
>    starting with release 17.0.3. It is not available in JDK 18.
>  - ... and 13 more: https://git.openjdk.org/jfx/compare/559ad113...842470ae

I finished all my testing. This looks good to me, and I have no further 
questions or concerns. There is some follow-up work that is needed, but that 
can be done after we get this in.

Here are the results of my testing. I did 3 CI builds as well as a few local 
builds on all three desktop OS platforms. All of my testing was done using the 
latest compiler devkits and the 
[kevinrushforth:test-pr-446](/kevinrushforth/jfx/tree/test-pr-446) branch, 
which is the latest branch from this PR with the latest upstream master merged 
in.

For the first test, I ran without setting `SOURCE_DATE_EPOCH` and compared the 
results of a build from `test-pr-446` with a build from `master`. All is as 
expected.

For the second test, I compared the results of two builds using the same 
`SOURCE_DATE_EPOCH` timestamp on the same platform. Except for the native 
WebKit library on Windows and macOS, everything was identical.

For the third test, I compared the results of a local build with the results of 
a CI build using the same `SOURCE_DATE_EPOCH` timestamp. There were some 
differences on Windows and Linux in the native binaries that should be 
followed-up on.

#### macOS

Two different CI builds on the same system:
* Only `libjfxwebkit.dylib` is different

Two different builds (CI and local) each on a different macOS 13.x system:
* `libjfxwebkit.dylib` is different
* `.bss` files differ due to known issue where the absolute file path is 
recorded in the `.bss` files

#### Linux

Two different CI builds on the same system:
    * All artifacts are identical

Two different builds (CI and local) each on a different OS platform (Oracle 
Linux 7.x versus Ubuntu 16.04) with devkit:
    * All `.so` files are different
    * `.bss` files differ due to file path issues

#### Windows

Two different builds on the same system:
    * Only `jfxwebkit.dll` is different

Two different builds (CI and local) each on a different OS platform (Windows 10 
versus Windows Server 2016) with devkit:
    * All `.dll` files are different
    * `.bss` files differ due to file path issues


NOTE: Please merge in the latest master and I'll reapprove.

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

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/446#pullrequestreview-1471918709

Reply via email to