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
Set SOURCE_DATE_EPOCH env var if not provided. #2882
Conversation
This comment has been minimized.
This comment has been minimized.
5be84d1
to
557fd7b
Compare
It now has a passing test for reproducibility that can be made to fail by changing: ENV["SOURCE_DATE_EPOCH"] = new_epoch to: ENV["SOURCE_DATE_EPOCH"] = (new_epoch.to_i + 1).to_s So I am a lot more confident that it actually works properly, now. |
557fd7b
to
60ca549
Compare
So, assuming CI passes, the current state of this is: there is a test proving generated .gem files for the same source with the same SOURCE_DATE_EPOCH are equivalent, but I have had difficulty doing this manually for some reason. By screwing with SOURCE_DATE_EPOCH on the second build in the test, I can make the test fail, so I don't think it's the test that's wrong this time. |
|
||
Time.at(ENV["SOURCE_DATE_EPOCH"].to_i).utc.freeze | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you think of:
def self.source_date_epoch
source_date_epoch = ENV["SOURCE_DATE_EPOCH"]
source_date_epoch = Time.now.to_i if source_date_epoch.strip.empty?
Time.at(source_date_epoch).utc.freeze
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire purpose of this PR is to set ENV["SOURCE_DATE_EPOCH"]
if it hasn't been set, which your variant doesn't do.
Thanks for the feedback @bronzdoc. I'll go through and update the code in a bit. 👍 |
60ca549
to
e993811
Compare
@bronzdoc I just pushed changes which should resolve most of what you mentioned. Let me know what you think. 🙂 |
Fixes rubygems#2290. 1. `Gem::Specification.date` returns SOURCE_DATE_EPOCH when defined, 2. this commit makes RubyGems set it _persistently_ when not provided. This combination means that you can build a gem, check the build time, and use that value to generate a new build -- and then verify they're the same.
e993811
to
d830d53
Compare
Just rebased this off latest master. |
LGTM 👍 |
@bundlerbot r=djberg96 |
2882: Set SOURCE_DATE_EPOCH env var if not provided. r=djberg96 a=duckinator # Description: Set SOURCE_DATE_EPOCH env var if not provided. Fixes #2290. 1. `Gem::Specification.date` returns SOURCE_DATE_EPOCH when defined, 2. this commit makes RubyGems set it _persistently_ when not provided. This combination means that you can build a gem, check the build time, and use that value to generate a new build -- and then verify they're the same. # Tasks: - [x] Describe the problem / feature - [x] Write tests - [x] Write code to solve the problem - [ ] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md). Co-authored-by: Ellen Marie Dash <the@smallest.dog>
Build succeeded |
Description:
Set SOURCE_DATE_EPOCH env var if not provided.
Fixes #2290.
Gem::Specification.date
returns SOURCE_DATE_EPOCH when defined,This combination means that you can build a gem, check the build time,
and use that value to generate a new build -- and then verify they're
the same.
Tasks:
I will abide by the code of conduct.