CHECK_EPOCH freshness is host-conditional: busybox date silently drops %N #125
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Follow-up to PR #122 (closes #115), from the re-review. Non-blocking there, worth closing properly.
script/cibuildpasses--build-arg CHECK_EPOCH="$(date +%s%N)".%Nis a GNU/uutils extension, not POSIX.The reviewer checked it against this repo's own pinned alpine image: busybox
date +%s%Nprints1786257437— it silently drops%Nand exits 0. So on a busybox host the epoch degrades from nanosecond to second granularity.Why this is worth four characters
The degradation itself is benign — second granularity is exactly where the PR stood before the
%Nchange, and sequential builds take ~45s so they cannot collide. The problem is the mismatch between guarantee and description:script/cibuild's header comment and the README describe the freshness unconditionally.dateimplementation, and the degradation is silent — no warning, exit 0, nothing in the build output distinguishes a nanosecond epoch from a second-granular one except the digit count.Two concurrent invocations within the same second on a busybox host would produce identical epochs, and the second would be served from the first's layer. That is the original #115 false green, narrowed to a race window rather than eliminated.
The reviewer also chased the catastrophic path rather than assuming it away, and the result is worth recording: a constant
CHECK_EPOCHwould requiredateto exit non-zero, which no strftime implementation does for an unknown conversion specifier — so it cannot realistically happen. But they verified indashthat if it ever did,set -euwould not catch it: a failing command substitution inside a command's arguments does not tripset -e. The build would silently receiveCHECK_EPOCH=empty and constant, and the false green would return by a different door.Definition of done
CHECK_EPOCHis unique per invocation on every host, regardless of whetherdatesupports%N. Appending the shell PID —$(date +%s%N)$$— achieves this in four characters:$$differs between concurrent invocations even when the seconds field is identical, and it is POSIX.datesubstitution fails. Consider whether to assert non-emptiness explicitly rather than relying ondatenever failing.script/cibuild's header comment and the README describe the actual guarantee, without overclaiming.script/cibuildremains POSIXsh(#!/bin/sh,set -eu, no bashisms);sh -nclean.%Nis honoured and when it is dropped — the pinned alpine image in this repo'sDockerfilegives you a genuine busyboxdateto test against, so this can be checked for real rather than reasoned about.script/cibuildstill actually runs the suite: two consecutive runs on an unchanged tree, both executing the check step with distinct epochs, neitherCACHEDnor sub-second. Report both timings.make checkgreen;TODO.mdupdated in the same commit.The finishing commit's title must end with
(closes #N)referencing this issue.Hard constraints
golang/alpinesha256digests, golangci-lintc0d3ddc9cf3faa61a4e378e879ece580256d76e5, goimports009367f5c17a8d4c45a961a3a509277190a9a6f0..golangci.ymlunmodified — sha256021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.make checkstill runs in full.parallel golangci-lint is running, or reports any path starting with../, or any absolute path outside your worktree. Retry; never record a void run.Sequencing
Land after PR #122, which introduces the line being changed.
Upstream
script/cibuildis byte-identical across repos and this fix is being adopted as the canonical template change (prompts#26), so the$$refinement should go upstream too rather than living only here.