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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8238650: Allow to override buildDate with SOURCE_DATE_EPOCH #422

Closed
wants to merge 2 commits into from

Conversation

@jgneff
Copy link
Member

@jgneff jgneff commented Mar 9, 2021

This is a continuation of the pull request started by @bmwiedemann in January 2020. After this change is integrated, I can follow up immediately with additional pull requests that get us much closer to providing fully reproducible builds.

Motivation

The only conclusive way to verify a software package is to reproduce it. That's the main point of the Linux Foundation article Preventing Supply Chain Attacks like SolarWinds by David Wheeler, Director of Open Source Supply Chain Security. "In the longer term," he writes, "I know of only one strong countermeasure for this kind of attack: verified reproducible builds."

It's not enough anymore to trust the person, organization, or company that publishes a software package. David Wheeler explains, "Assuming a system can 'never be broken into' is a failing strategy." As I see it, any project that doesn't yet allow for reproducible builds is on a list of possible attack vectors. I'd like to get OpenJFX off that list.

This is a huge undertaking involving the entire open-source community. Just Debian, for example, has over 30,000 source packages to build in a reproducible manner. Remarkably, it's almost 96 percent complete. The OpenJFX package is one of three percent still failing. Our first step towards helping in this goal is to support the SOURCE_DATE_EPOCH specification.

Implementation

When you want to build 30,000 packages in a reproducible manner, command-line flags unique to each package aren't so helpful. The environment variable needs to be set, anyway, for the tools invoked by Gradle to pick it up. We could allow for a Gradle property in addition to the environment variable. The Gradle property would override the default current date and export the environment variable, and the environment variable would override the command-line property. I think it makes more sense in the OpenJFX build to support the environment variable directly.

With these considerations, I added the support just as recommended by the example for "Java / gradle" on the Reproducible Builds SOURCE_DATE_EPOCH page. For comparison, the OpenJDK build does the reverse, using the configure script option --with-source-date to export the SOURCE_DATE_EPOCH environment variable.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • 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.java.net/jfx pull/422/head:pull/422
$ git checkout pull/422

Update a local copy of the PR:
$ git checkout pull/422
$ git pull https://git.openjdk.java.net/jfx pull/422/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 422

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/422.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Mar 9, 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 label Mar 9, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Mar 9, 2021

Webrevs

@kevinrushforth kevinrushforth self-requested a review Mar 9, 2021
@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Mar 9, 2021

/reviewers 2

@openjdk
Copy link

@openjdk openjdk bot commented Mar 9, 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

@kevinrushforth kevinrushforth commented Mar 9, 2021

This is a simple enough change by itself, but since it is intended as the start of a larger effort, I'd like @johanvos (or someone else he designates from Gluon) to be a second reviewer.

@jgneff
Copy link
Member Author

@jgneff jgneff commented Mar 24, 2021

/contributor add @bmwiedemann

@openjdk
Copy link

@openjdk openjdk bot commented Mar 24, 2021

@jgneff Could not parse bmwiedemann as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>

Copy link
Member

@kevinrushforth kevinrushforth left a comment

This looks good to me. I tested it and it does what I would expect.

I generally prefer gradle properties to env vars, but supporting the SOURCE_DATE_EPOCH env variable directly makes sense for the reasons you pointed out. We could always add a property later to override it, but I suspect we won't ever feel the need to do that.

@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Mar 24, 2021

@jgneff Could not parse bmwiedemann as a valid contributor.

You might retry the "contributor" command with the @ before their GitHub username (I'm not 100% sure that will work, though).

@jgneff
Copy link
Member Author

@jgneff jgneff commented Mar 25, 2021

You might retry the "contributor" command with the @ before their GitHub username.

Thanks. I'll try that now. I checked and Bernhard is on the list of "OCAs submitted prior to 2021," but not on the OpenJDK Census list, in case that matters.

By the way, should I be adding my own name, too? It looks as if sometimes people do that and sometimes not. That would put me as "Author" and also on a line with "Co-authored-by" in the commit message.

/contributor add @bmwiedemann

@openjdk
Copy link

@openjdk openjdk bot commented Mar 25, 2021

@jgneff Could not parse @bmwiedemann as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>

@jgneff
Copy link
Member Author

@jgneff jgneff commented Mar 30, 2021

This is a simple enough change by itself, but since it is intended as the start of a larger effort, I'd like @johanvos (or someone else he designates from Gluon) to be a second reviewer.

I jumped the gun and created the follow-up pull request. I thought it might help when reviewing this pull request to have a full picture of that larger effort. Although these two changes are part of a huge undertaking in the open-source community, it turns out that the changes required on our part are quite small.

@kevinrushforth kevinrushforth requested review from johanvos and tiainen Mar 31, 2021
@jgneff
Copy link
Member Author

@jgneff jgneff commented Apr 1, 2021

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

@openjdk
Copy link

@openjdk openjdk bot commented Apr 1, 2021

@jgneff
Contributor Bernhard M. Wiedemann <javabmw@lsmod.de> successfully added.

@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Apr 16, 2021

Pending review by @johanvos

@jgneff
Copy link
Member Author

@jgneff jgneff commented Jun 14, 2021

Closing this pull request in favor of #446.

The changes in this pull request are included as the first commit of pull request #446.

@jgneff jgneff closed this Jun 14, 2021
@jgneff jgneff deleted the source-date-epoch branch Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants