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

8275745: Reproducible copyright headers #1498

Closed
wants to merge 2 commits into from

Conversation

ebourg
Copy link
Contributor

@ebourg ebourg commented Nov 28, 2020

The copyright headers are generated at build time, and the year inserted in the template depends on the current date. This means the headers are not reproducible if the project is built a year later. The year in the headers could be derived from the SOURCE_DATE_EPOCH environment variable to make the build reproducible (this variable is already used in other parts of the build).


Progress

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

Issue

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/1498/head:pull/1498
$ git checkout pull/1498

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1498

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

Using diff file

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

@bridgekeeper bridgekeeper bot added the oca Needs verification of OCA signatory status label Nov 28, 2020
@bridgekeeper
Copy link

bridgekeeper bot commented Nov 28, 2020

Hi @ebourg, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user ebourg" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

@openjdk
Copy link

openjdk bot commented Nov 28, 2020

@ebourg The following labels will be automatically applied to this pull request:

  • build
  • core-libs
  • i18n

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org i18n i18n-dev@openjdk.org labels Nov 28, 2020
@ebourg
Copy link
Contributor Author

ebourg commented Nov 28, 2020

/signed

@bridgekeeper bridgekeeper bot added the oca-verify Needs verification of OCA signatory status label Nov 28, 2020
@bridgekeeper
Copy link

bridgekeeper bot commented Nov 28, 2020

Thank you! Please allow for up to two weeks to process your OCA, although it is usually done within one to two business days. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

@ebourg
Copy link
Contributor Author

ebourg commented Nov 28, 2020

/label remove core-libs,i18n

@openjdk openjdk bot removed core-libs core-libs-dev@openjdk.org i18n i18n-dev@openjdk.org labels Nov 28, 2020
@openjdk
Copy link

openjdk bot commented Nov 28, 2020

@ebourg
The core-libs label was successfully removed.

The i18n label was successfully removed.

@bridgekeeper bridgekeeper bot removed oca Needs verification of OCA signatory status oca-verify Needs verification of OCA signatory status labels Dec 9, 2020
@bridgekeeper
Copy link

bridgekeeper bot commented Dec 27, 2020

@ebourg 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!

@ebourg
Copy link
Contributor Author

ebourg commented Dec 27, 2020

Dummy comment to keep this issue alive (auto closing after 8 weeks of inactivity is a bad idea)

@bridgekeeper
Copy link

bridgekeeper bot commented Jan 24, 2021

@ebourg 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!

@ebourg
Copy link
Contributor Author

ebourg commented Feb 1, 2021

.

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 1, 2021

@ebourg 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!

@ebourg
Copy link
Contributor Author

ebourg commented Mar 5, 2021

.

@openjdk
Copy link

openjdk bot commented Mar 17, 2021

@ebourg Unknown command signed - for a list of valid commands use /help.

@y1yang0
Copy link
Member

y1yang0 commented Mar 18, 2021

You should link an issue to this patch so that a Request-For-Review email will be sent to the corresponding mailing list. Or they may not notice your patch.

@ebourg
Copy link
Contributor Author

ebourg commented Mar 18, 2021

Unfortunately I don't have a JBS account, so I'm stuck here bumping this PR every 4 weeks until someone notices it.

@bridgekeeper
Copy link

bridgekeeper bot commented Apr 21, 2021

@ebourg 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!

@ebourg
Copy link
Contributor Author

ebourg commented Apr 21, 2021

.

@y1yang0
Copy link
Member

y1yang0 commented Apr 21, 2021

Hi Emmanuel Bourg,

I'm not quite sure whether this PR is useful since I'm not familiar with the make system. I think you can just send an email to build-dev@openjdk.java.net(You may need to subscribe it before), the related folks/experts would review your change or give some meaningful feedbacks if they think it's worth doing it. Good Luck!

Thanks,
Yang

@ebourg
Copy link
Contributor Author

ebourg commented Apr 21, 2021

Yes, 5 months later I now understand that the GitHub issue tracker isn't the right entry point to contribute to OpenJDK, sadly.

@bridgekeeper
Copy link

bridgekeeper bot commented May 19, 2021

@ebourg 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!

@ebourg
Copy link
Contributor Author

ebourg commented Jun 7, 2021

.

@magicus
Copy link
Member

magicus commented Nov 11, 2021

@ebourg I have now modified this patch so it uses COPYRIGHT_YEAR, and sets COPYRIGHT_YEAR based on SOURCE_DATE_EPOCH, if it exists. The patch is in a branch in my personal fork, https://github.com/magicus/jdk/tree/reproducible-copyright-year.

If you think it looks good, we have two possible ways forward. Either you close this PR, and I open a new targeting the same JBS issue, and credit you as co-author. Or you integrate my branch into this PR, and credit me as co-author. Any of them is OK for me, but I think the former is simpler.

@ebourg
Copy link
Contributor Author

ebourg commented Nov 11, 2021

@magicus Thank you! I've integrated your branch

Copy link
Member

@magicus magicus left a comment

Choose a reason for hiding this comment

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

LGTM but then again I wrote most of it. :) Please await further reviewers before integrating.

@openjdk
Copy link

openjdk bot commented Nov 11, 2021

⚠️ @ebourg the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout reproducible-copyright-headers
$ git commit -c user.name='Preferred Full Name' --allow-empty -m 'Update full name'
$ git push

@openjdk
Copy link

openjdk bot commented Nov 11, 2021

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

8275745: Reproducible copyright headers

Reviewed-by: ihse, erikj

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

  • b15e6f0: 8277449: compiler/vectorapi/TestLongVectorNeg.java fails with release VMs
  • 11d819d: 8277439: G1: Correct include guard name in G1EvacFailureObjectsSet.hpp
  • 7a046e0: 8277371: Remove unnecessary DefNewGeneration::ref_processor_init()
  • 3a76d39: 8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule
  • 2f20b0d: 8273039: JShell crashes when naming variable or method "abstract" or "strictfp"
  • f34f119: 8277213: CompileTask_lock is acquired out of order with MethodCompileQueue_lock
  • 47564ca: 8275643: C2's unaryOp vector intrinsic does not properly handle LongVector.neg
  • 2f0bde1: 8277102: Dubious PrintCompilation output
  • 839033b: 8276976: Rename LIR_OprDesc to LIR_Opr
  • 89b125f: 8275527: Refactor forward pointer access
  • ... and 624 more: https://git.openjdk.java.net/jdk/compare/5e4b514e6e7e1b9f51fac1983b6c12a988f7f5a8...master

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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@magicus, @erikj79) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Nov 11, 2021
@ebourg
Copy link
Contributor Author

ebourg commented Nov 11, 2021

I can rebase the branch with your changes only, my commit just adds noise.

@ebourg ebourg force-pushed the reproducible-copyright-headers branch from 9c4efbd to 618d28a Compare November 11, 2021 15:33
@magicus
Copy link
Member

magicus commented Nov 11, 2021

In the future, please refrain from force pushing to a PR. It makes history hard to follow for reviewers, and is generally strongly discouraged. OpenJDK uses the Skara tools which will automatically squash and rebase the commits in the PR.

@ebourg
Copy link
Contributor Author

ebourg commented Nov 11, 2021

Thank you for the advice, I'm not familiar with the Skara workflow yet.

@magicus
Copy link
Member

magicus commented Nov 19, 2021

@ebourg To finish integrating this PR, you need to type /integrate as a PR comment. This will allow the Skara bots to move the bug on to the state where I can sponsor it.

@ebourg
Copy link
Contributor Author

ebourg commented Nov 19, 2021

/integrate

@ebourg
Copy link
Contributor Author

ebourg commented Nov 19, 2021

@magicus Thank you for the hint!

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 19, 2021
@openjdk
Copy link

openjdk bot commented Nov 19, 2021

@ebourg
Your change (at version 618d28a) is now ready to be sponsored by a Committer.

@magicus
Copy link
Member

magicus commented Nov 19, 2021

/sponsor

@openjdk
Copy link

openjdk bot commented Nov 19, 2021

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

  • b1a1bf4: 8277427: Update jib-profiles.js to use JMH 1.33 devkit
  • 03debf2: 8276774: Cookie stored in CookieHandler not sent if user headers contain cookie
  • b15e6f0: 8277449: compiler/vectorapi/TestLongVectorNeg.java fails with release VMs
  • 11d819d: 8277439: G1: Correct include guard name in G1EvacFailureObjectsSet.hpp
  • 7a046e0: 8277371: Remove unnecessary DefNewGeneration::ref_processor_init()
  • 3a76d39: 8277324: C2 compilation fails with "bad AD file" on x86-32 after JDK-8276162 due to missing match rule
  • 2f20b0d: 8273039: JShell crashes when naming variable or method "abstract" or "strictfp"
  • f34f119: 8277213: CompileTask_lock is acquired out of order with MethodCompileQueue_lock
  • 47564ca: 8275643: C2's unaryOp vector intrinsic does not properly handle LongVector.neg
  • 2f0bde1: 8277102: Dubious PrintCompilation output
  • ... and 626 more: https://git.openjdk.java.net/jdk/compare/5e4b514e6e7e1b9f51fac1983b6c12a988f7f5a8...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot closed this Nov 19, 2021
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Nov 19, 2021
@openjdk
Copy link

openjdk bot commented Nov 19, 2021

@magicus @ebourg Pushed as commit a022796.

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

@magicus
Copy link
Member

magicus commented Nov 19, 2021

@ebourg Thank you for your contribution! It's been a long and bumpy ride for something that seemed like a simple patch. I hope you have not been scared away from contributing to the JDK in the future. :-)

@magicus
Copy link
Member

magicus commented Nov 20, 2021

@ebourg You might be interested in watching what @andrew-m-leonard is doing in the area, for example in #6395.

@mlbridge
Copy link

mlbridge bot commented Nov 21, 2021

Mailing list message from Jonathan Gibbons on build-dev:

Emmanual,

If you're interested, please contact the javadoc-dev at openjdk.java.net
list regarding the javadoc fix. While the diff looks promising, it would
require a corresponding regression test.

-- Jon

On 11/19/21 6:24 AM, Emmanuel Bourg wrote:

On Thu, 11 Nov 2021 15:37:09 GMT, Emmanuel Bourg <duke at openjdk.java.net> wrote:

[snip]

https://salsa.debian.org/openjdk-team/openjdk/-/blob/openjdk-17/debian/patches/reproducible-javadoc-timestamp.diff

[snip]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org integrated Pull request has been integrated
4 participants