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

Set SOURCE_DATE_EPOCH #1418

Merged
merged 8 commits into from
Apr 20, 2022
Merged

Set SOURCE_DATE_EPOCH #1418

merged 8 commits into from
Apr 20, 2022

Conversation

natalieparellano
Copy link
Member

@natalieparellano natalieparellano commented Apr 11, 2022

pack should set SOURCE_DATE_EPOCH in exporter's env if --creation-time flag is provided

Also bumps default lifecycle version

Before

Image create time is always Jan 1, 1980

After

Image create time is configurable

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #1281

…ag is provided

Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano natalieparellano requested a review from a team as a code owner April 11, 2022 22:22
@github-actions github-actions bot added this to the 0.25.0 milestone Apr 11, 2022
@github-actions github-actions bot added type/chore Issue that requests non-user facing changes. type/enhancement Issue that requests a new feature or improvement. labels Apr 11, 2022
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #1418 (34ff550) into main (b9be13a) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

❗ Current head 34ff550 differs from pull request most recent head 26c6f36. Consider uploading reports for the commit 26c6f36 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1418      +/-   ##
==========================================
- Coverage   81.27%   81.27%   -0.00%     
==========================================
  Files         151      151              
  Lines        9715     9745      +30     
==========================================
+ Hits         7895     7919      +24     
- Misses       1346     1350       +4     
- Partials      474      476       +2     
Flag Coverage Δ
os_linux 80.02% <80.00%> (-<0.01%) ⬇️
os_macos 77.40% <80.00%> (-0.02%) ⬇️
os_windows 81.14% <80.00%> (-<0.01%) ⬇️
unit 81.27% <80.00%> (-<0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@sambhav
Copy link
Member

sambhav commented Apr 11, 2022

Should the flag instead be --creation-time?

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano natalieparellano changed the title pack should set SOURCE_DATE_EPOCH in exporter's env if --date-time flag is provided pack should set SOURCE_DATE_EPOCH in exporter's env if --creation-time flag is provided Apr 12, 2022
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano
Copy link
Member Author

Phew! Finally green...

Copy link
Member

@jromero jromero left a comment

Choose a reason for hiding this comment

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

Two minor points:

  • We should probably cary the term CreationTime across the entire code base instead of having an ambiguous DateTime
  • Isn't this variable only respected on newer Platform API versions? If so, it seems like we should only be passing it to the container in those cases and warning the user if it's not applicable.

internal/commands/build.go Show resolved Hide resolved
Copy link
Member

@dfreilich dfreilich left a comment

Choose a reason for hiding this comment

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

Overall, as always, beautiful! just a few tweaks would be awesome

internal/commands/build.go Outdated Show resolved Hide resolved
internal/commands/build.go Show resolved Hide resolved
Signed-off-by: Natalie Arellano <narellano@vmware.com>
…en platform api is 0.9

Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano
Copy link
Member Author

The tests are green but I don't understand why - https://github.com/buildpacks/pack/runs/6009113466?check_suite_focus=true uses a lifecycle that doesn't support this feature. Investigating.

Copy link
Member

@dfreilich dfreilich left a comment

Choose a reason for hiding this comment

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

Fantastic!

})

when("not provided", func() {
it("image has create time of Jan 1, 1980", func() {
Copy link
Member

Choose a reason for hiding this comment

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

This test could actually run in all compatibility cases, but I'm not sure whether it's worth the time cost of having the tests do that.

…n time

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Jan 1, 1980 is always going to be less than a time of about now

Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano
Copy link
Member Author

The tests are green but I don't understand why - https://github.com/buildpacks/pack/runs/6009113466?check_suite_focus=true uses a lifecycle that doesn't support this feature. Investigating.

I found the issue - fixed in c23e970 (it was a bad assertion).

@jromero jromero changed the title pack should set SOURCE_DATE_EPOCH in exporter's env if --creation-time flag is provided Set SOURCE_DATE_EPOCH Apr 13, 2022
@jromero jromero mentioned this pull request Apr 13, 2022
4 tasks
@dfreilich dfreilich modified the milestones: 0.25.0, 0.26.0 Apr 15, 2022
@jromero jromero merged commit 8921b1b into main Apr 20, 2022
@jromero jromero deleted the source-date-epoch branch April 20, 2022 17:21
@thedevelopnik
Copy link

Thank you so much! This is a long-awaited feature and I am so grateful it landed!

@jromero jromero removed the type/chore Issue that requests non-user facing changes. label Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable configuring the date used by reproducible builds
5 participants