-
-
Notifications
You must be signed in to change notification settings - Fork 233
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 #274
Conversation
Just to show how powerful I tested this with a port I maintain, ioping. I ran Before this PR: File timestamps are different, so the archives are as well. $ gsha256sum ioping-1.3_0.darwin_21.arm64\ 2.tbz2
6135d8dff9c87a3074fbf1dbaeea13db4ab0ef7774bcd5072e1c5196246d7396 ioping-1.3_0.darwin_21.arm64 2.tbz2
$ gsha256sum ioping-1.3_0.darwin_21.arm64.tbz2
6958d0826cc9c0d2775f3d9f9c20372f2c2fa5369caa44f80b7fbc4746504743 ioping-1.3_0.darwin_21.arm64.tbz2 Afterwards: The archives are completely identical. $ gsha256sum ioping-1.3_0.darwin_21.arm64\ 2.tbz2
2406a76abc668c91ef9ec066ab9735dbee55ceba615ea6ec15aff6537d6d71ce ioping-1.3_0.darwin_21.arm64 2.tbz2
$ gsha256sum ioping-1.3_0.darwin_21.arm64.tbz2
2406a76abc668c91ef9ec066ab9735dbee55ceba615ea6ec15aff6537d6d71ce ioping-1.3_0.darwin_21.arm64.tbz2 Below is the diffoscope output showing how the timestamps in the original archives are different: @@ -1,14 +1,14 @@
-drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:31.000000 ./
--rw-r--r-- 0 root (0) wheel (0) 287 2023-02-25 12:13:31.000000 ./+STATE
+drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:03.000000 ./
+-rw-r--r-- 0 root (0) wheel (0) 287 2023-02-25 12:13:03.000000 ./+STATE
-rw-r--r-- 0 root (0) wheel (0) 1039 2022-09-04 11:18:25.000000 ./+PORTFILE
--rw-r--r-- 0 root (0) wheel (0) 405 2023-02-25 12:13:31.000000 ./+CONTENTS
--rw-r--r-- 0 root (0) wheel (0) 152 2023-02-25 12:13:31.000000 ./+DESC
--rw-r--r-- 0 root (0) wheel (0) 39 2023-02-25 12:13:31.000000 ./+COMMENT
-drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:31.000000 ./opt/
-drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:31.000000 ./opt/local/
-drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:31.000000 ./opt/local/bin/
-drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:31.000000 ./opt/local/share/
-drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:31.000000 ./opt/local/share/man/
-drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:31.000000 ./opt/local/share/man/man1/
--r--r--r-- 0 root (0) wheel (0) 3603 2023-02-25 12:13:31.000000 ./opt/local/share/man/man1/ioping.1.gz
--rwxr-xr-x 0 root (0) wheel (0) 73539 2023-02-25 12:13:31.000000 ./opt/local/bin/ioping
+-rw-r--r-- 0 root (0) wheel (0) 405 2023-02-25 12:13:03.000000 ./+CONTENTS
+-rw-r--r-- 0 root (0) wheel (0) 152 2023-02-25 12:13:03.000000 ./+DESC
+-rw-r--r-- 0 root (0) wheel (0) 39 2023-02-25 12:13:03.000000 ./+COMMENT
+drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:03.000000 ./opt/
+drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:03.000000 ./opt/local/
+drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:03.000000 ./opt/local/bin/
+drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:03.000000 ./opt/local/share/
+drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:03.000000 ./opt/local/share/man/
+drwxr-xr-x 0 root (0) wheel (0) 0 2023-02-25 12:13:03.000000 ./opt/local/share/man/man1/
+-r--r--r-- 0 root (0) wheel (0) 3603 2023-02-25 12:13:03.000000 ./opt/local/share/man/man1/ioping.1.gz
+-rwxr-xr-x 0 root (0) wheel (0) 73539 2023-02-25 12:13:03.000000 ./opt/local/bin/ioping |
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.
Thanks for working on this, this is and important step and a good thing to have. I have a few concerns, though.
src/port1.0/portinstall.tcl
Outdated
# Set standardised file modification times | ||
fs-traverse file $destpath { | ||
# Keep the original times for files that have not been modified during the build | ||
if {[file mtime $file] > $source_date_epoch} { | ||
file mtime $file $source_date_epoch | ||
} | ||
} |
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.
I'm not sure that will work for all ports as we expect it to. As far as I recall, Python scripts check that the timestamps of their associated .pyc files is strictly larger than the timestamp of the script itself.
For python scripts generated during the build (e.g., because of some autotools replacement magic), this would mean that we might end up shipping a pre-compiled .pyc file that ends up being ignored by all users.
Maybe we should provide an option to turn this behavior off for ports where it would have adverse effects, or add a method to override the time for certain files?
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.
Sorry it's taken me this long to get to this @neverpanic. Appreciate the feedback.
As far as I recall, Python scripts check that the timestamps of their associated .pyc files is strictly larger than the timestamp of the script itself.
Just looking it up, it seems like it should be fine for all supported python versions (see PEP 552). Setting SOURCE_DATE_EPOCH triggers using hash-based pyc files (python/cpython@ccbe581).
However, I completely agree that there might be other scenarios where this has adverse effects.
Maybe we should provide an option to turn this behavior off for ports where it would have adverse effects, or add a method to override the time for certain files?
I've added a reproducible_timestamps
option. It's default is set to true, but this can be changed if that's the general consensus.
0d7ec86
to
0e6ae65
Compare
EDIT: Fix incoming. |
0e6ae65
to
fce6e4b
Compare
src/port1.0/portmain.tcl
Outdated
@@ -113,6 +114,7 @@ default destroot {${destpath}} | |||
default filesdir files | |||
default revision 0 | |||
default epoch 0 | |||
default reproducible_timestamps 1 |
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.
Although using 1
probably works, MacPorts convention is to use yes
and no
for Booleans.
default reproducible_timestamps 1 | |
default reproducible_timestamps yes |
src/port1.0/portinstall.tcl
Outdated
@@ -308,6 +308,19 @@ proc portinstall::create_archive {location archive.type} { | |||
puts $fd "@cxx_stdlib_overridden ${portinstall::cxx_stdlib_overridden}" | |||
close $fd | |||
|
|||
if {[get_statefile_value "SOURCE_DATE_EPOCH" $target_state_fd source_date_epoch] != 0 && $reproducible_timestamps == 1} { |
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.
This will need a corresponding change.
src/port1.0/portinstall.tcl
Outdated
fs-traverse file $destpath { | ||
if {[file type $file] == "link"} { | ||
# file mtime doesn't work on symbolic links | ||
exec touch -ht $source_date_epoch $file |
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.
I would rather see the MacPorts touch
procedure be used than using the external touch
command. The MacPorts touch
procedure doesn't support -h
yet but perhaps that could be added.
Took me a minute of reading the code, but I see now that you implemented:
In the ticket I was only proposing doing (1) and I don't know what the consequences are of doing (2). |
fce6e4b
to
3d5a913
Compare
Thank you for the review Ryan, you make a very valid point. I think the easiest thing for now is to simply add SOURCE_DATE_EPOCH to base as was discussed in the original ticket. Standardising the timestamps can be discussed at a later date. |
Closes: https://trac.macports.org/ticket/59672
This PR allows base to define the
SOURCE_DATE_EPOCH
environment variable to assist with reproducible builds. The value can be manually set in a portfile via thesource_date_epoch
variable.If the variable isn't assigned, its value is set to the greatest Unix timestamp of the files present in the source code (credit to Ryan for implementation details).