fix: make zip task compatible with other systems #277
Conversation
The zip task was relying on 'touch -d @seconds' to set the timestamp of output files to allow reproducible builds. 'touch -d @seconds' is not POSIX compliant and the standard only supports a subset of formats: https://man7.org/linux/man-pages/man1/touch.1p.html The standard way of doing it would be to use one of those format or just use 'touch -t TIME'. Since SOURCE_DATE_EPOCH is providing seconds since epoch and we don't have a standard way to convert them to a standard format used by touch, we rely on 'date' and use a fallback mechanism to try two different syntax: - Busybox and GNU coreutils date - BSD date
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
There was a problem hiding this comment.
Thanks for doing this! WFM on macos, and two runs of make build zip result in an identical "extension.zip".
The quote from https://www.gnu.org/software/coreutils/manual/html_node/Date-input-formats.html applies here:
It is like a set of trapezoidal building blocks, with no vertical or horizontal surfaces, like a language in which the simplest thought demands ornate constructions, useless particles and lengthy circumlocutions.
working with time is such a pain.
| SOURCE_DATE_EPOCH ?= $(shell git log -1 --pretty=%ct) | ||
| DATE_FMT = +%Y%m%d%H%M.%S | ||
| # Fallback mechanism to support other systems: | ||
| # 1. 'date -d': Busybox and GNU coreutils. | ||
| # 2. 'date -r': BSD date. It does not support '-d'. | ||
| BUILD_DATE = $(shell date -u -d "@${SOURCE_DATE_EPOCH}" "${DATE_FMT}" 2>/dev/null || date -u -r "${SOURCE_DATE_EPOCH}" "${DATE_FMT}") |
There was a problem hiding this comment.
I wonder if it's worth trying to adhere to SOURCE_DATE_EPOCH. This would be simpler by using the --date flag of git log:
BUILD_DATE = $(shell git log -1 --pretty=%cd --date=format:%Y%m%d%H%M.%S)
There was a problem hiding this comment.
I think we should still support SOURCE_DATE_EPOCH but that's a valid point.
Moreover avoiding to call date would increase compatibility since touch is POSIX compliant and git is required to build the extension.
I will merge this for now but we can iterate on it!
3 participants
The zip task was relying on 'touch -d @seconds' to set the timestamp of
output files to allow reproducible builds.
'touch -d @seconds' is not POSIX compliant and the standard only supports
a subset of formats:
https://man7.org/linux/man-pages/man1/touch.1p.html
The standard way of doing it would be to use one of those format or just use
'touch -t TIME'.
Since SOURCE_DATE_EPOCH is providing seconds since epoch and we don't have a
standard way to convert them to a standard format used by touch, we rely on
'date' and use a fallback mechanism to try two different syntax: