Skip to content

fix(build-go-attest): harden auto-build-timestamp (fail on empty)#78

Merged
CybotTM merged 1 commit intomainfrom
fix/auto-build-timestamp-hard-fail
Apr 21, 2026
Merged

fix(build-go-attest): harden auto-build-timestamp (fail on empty)#78
CybotTM merged 1 commit intomainfrom
fix/auto-build-timestamp-hard-fail

Conversation

@CybotTM
Copy link
Copy Markdown
Member

@CybotTM CybotTM commented Apr 21, 2026

Copilot review on #77:

  1. Silent failures: git show … 2>/dev/null || true ate failures and let the build continue with main.buildTime unset, defeating the opt-in. Remove the fallback — if resolution returns empty, emit ::error:: + exit 1.
  2. Input description tightened: consumers MUST declare var buildTime string. Go 1.21+ linkers silently ignore unknown -X targets, but older toolchains reject with 'symbol not found' — state the requirement explicitly instead of 'silent no-op'.
  3. Matching tightening in the inline step comment.

Copilot review on #77:

1. 'git show ... 2>/dev/null || true' silently ate failures — the
   build continued with main.buildTime unset, defeating the point of
   opting in. Remove the fallback: if resolution returns empty, emit
   a ::error:: annotation and exit 1. Stderr no longer silenced so
   the underlying failure reason reaches the log too.

2. Input description tightened: consumers MUST declare
   'var buildTime string' in package main. Go 1.21+ linkers silently
   ignore unknown -X targets, but older toolchains reject with
   'symbol not found' — noting the requirement explicitly instead
   of saying 'silent no-op'.

3. Matching tightening in the inline step comment.

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
Copilot AI review requested due to automatic review settings April 21, 2026 04:57
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@CybotTM CybotTM merged commit 84e4c90 into main Apr 21, 2026
@CybotTM CybotTM deleted the fix/auto-build-timestamp-hard-fail branch April 21, 2026 04:57
CybotTM added a commit to netresearch/raybeam that referenced this pull request Apr 21, 2026
Copilot on [#227](#227): the
comment listed `-X main.buildTime=<commit-timestamp>` as a direct
release.yml ldflag, but
[netresearch/.github#77](netresearch/.github#77)
moved that injection into `build-go-attest.yml`'s `auto-build-timestamp`
step. Update the comment to:

- attribute `buildTime` to the `auto-build-timestamp` source
- describe `var buildTime string` as a contract (not optional
insurance), since
[netresearch/.github#78](netresearch/.github#78)
hardens `auto-build-timestamp` to fail when the ldflag target is missing
on older toolchains
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the reusable Go build/attestation workflow’s auto-build-timestamp behavior so opting in can’t silently produce binaries with an unset/empty main.buildTime, and clarifies the consumer-side contract in the input documentation.

Changes:

  • Tighten auto-build-timestamp input docs to require var buildTime string and to describe fail-hard behavior on empty resolution.
  • Remove the silent fallback around git show, and fail the step when timestamp resolution yields an empty value.
  • Update the inline build-step comments to match the new contract/behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/build-go-attest.yml
Comment thread .github/workflows/build-go-attest.yml
CybotTM added a commit that referenced this pull request Apr 21, 2026
)

Copilot on [#78](#78):

1. **`set -e` short-circuit**: `BUILD_TS=$(git show …)` under `set -euo
pipefail` aborts before the custom `::error::` diagnostic runs. Wrap in
`if ! BUILD_TS=$(...); then` so the non-zero exit is captured locally;
merge stderr into stdout (`2>&1`) so git's actual error message appears
in the annotation.
2. **fetch-depth wording**: `fetch-depth: 0` also produces a valid HEAD.
Reword to 'fetch-depth 0 or >=1 both work'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants