Skip to content

Commit b59008c

Browse files
committed
ci(onnx): build a pinned commit, not a movable tag, and drop the token
Two review findings applied, one declined on the measurement. **Build the commit, not the tag.** `v1.27.1` upstream is a *lightweight* tag: it points straight at a commit and can be moved by anyone with push rights there. Building from it means attesting an artifact to "whatever that tag meant this morning" — which is exactly the property this workflow exists to provide, so the tag undercut its own point. `SOURCE_COMMIT` now sits beside `VERSION` in `fetch-onnxruntime.mjs`, so the single source of truth stays single, and a step fails the build if the tag no longer resolves to it. That catches both a repointed tag and a `VERSION` bump whose commit was forgotten. The source commit is printed in the job summary beside the digest, so whoever adopts the artifact can see what it was built from. **`persist-credentials: false` on both checkouts.** This job compiles third-party source in the same workspace, and leaving the token in `.git/config` puts it within reach of ONNX Runtime's own build scripts — in a workflow holding `attestations: write`. `build.yml`, `docs.yml` and `nix-build.yml` already set this; the omission was mine. **Raising `timeout-minutes` above 227 is declined.** That number is from the run with bare `--parallel`, which swapped instead of compiling. With the parallelism bounded the whole job takes **30 minutes**, verified end to end. 150 is already five times the observed duration, and a cap set above a known-pathological run cannot do the job a cap is for.
1 parent 4f1d918 commit b59008c

2 files changed

Lines changed: 43 additions & 2 deletions

File tree

‎.github/workflows/build-onnxruntime-macos.yml‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ jobs:
6666
steps:
6767
- name: Checkout
6868
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
69+
with:
70+
# This job compiles third-party source in the same workspace. Leaving the
71+
# token in .git/config would put it within reach of ONNX Runtime's own build
72+
# scripts — and this workflow holds `attestations: write`. Same setting
73+
# build.yml, docs.yml and nix-build.yml already use.
74+
persist-credentials: false
6975

7076
- name: Read the pinned version from fetch-onnxruntime.mjs
7177
id: pin
@@ -77,8 +83,11 @@ jobs:
7783
set -euo pipefail
7884
VERSION="$(sed -n 's/^const VERSION = "\(.*\)";$/\1/p' scripts/fetch-onnxruntime.mjs)"
7985
[ -n "$VERSION" ] || { echo "::error::VERSION not found in scripts/fetch-onnxruntime.mjs"; exit 1; }
86+
COMMIT="$(sed -n 's/^const SOURCE_COMMIT = "\(.*\)";$/\1/p' scripts/fetch-onnxruntime.mjs)"
87+
[ -n "$COMMIT" ] || { echo "::error::SOURCE_COMMIT not found in scripts/fetch-onnxruntime.mjs"; exit 1; }
8088
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
81-
echo "Building ONNX Runtime v$VERSION"
89+
echo "commit=$COMMIT" >> "$GITHUB_OUTPUT"
90+
echo "Building ONNX Runtime v$VERSION at $COMMIT"
8291
8392
- name: Read the deployment floor from electron-builder.json5
8493
id: floor
@@ -95,10 +104,28 @@ jobs:
95104
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
96105
with:
97106
repository: microsoft/onnxruntime
98-
ref: v${{ steps.pin.outputs.version }}
107+
# The COMMIT, not the tag. `v1.27.1` upstream is a lightweight tag — it points
108+
# straight at a commit and can be moved. Building from a tag would mean
109+
# attesting an artifact to "whatever that tag meant this morning", which is
110+
# precisely the property this workflow exists to provide.
111+
ref: ${{ steps.pin.outputs.commit }}
99112
path: onnxruntime-src
100113
submodules: recursive
101114
fetch-depth: 1
115+
persist-credentials: false
116+
117+
- name: Check the pinned commit is still what the tag names
118+
# Not fatal to the build — the commit above is what gets built either way — but a
119+
# tag that has moved means the pin and the version string no longer describe the
120+
# same thing, and somebody should look before adopting the artifact.
121+
run: |
122+
set -euo pipefail
123+
TAGGED="$(git -C onnxruntime-src ls-remote https://github.com/microsoft/onnxruntime "refs/tags/v${{ steps.pin.outputs.version }}" | cut -f1)"
124+
if [ "$TAGGED" != "${{ steps.pin.outputs.commit }}" ]; then
125+
echo "::error::v${{ steps.pin.outputs.version }} now resolves to ${TAGGED:-nothing}, not the pinned ${{ steps.pin.outputs.commit }}"
126+
exit 1
127+
fi
128+
echo "v${{ steps.pin.outputs.version }} still resolves to ${{ steps.pin.outputs.commit }}"
102129
103130
- name: Read the runner image
104131
id: image
@@ -241,6 +268,8 @@ jobs:
241268
{
242269
echo "## ONNX Runtime ${V}, macOS arm64, deployment target ${{ steps.floor.outputs.floor }}"
243270
echo ""
271+
echo "- Source: microsoft/onnxruntime@\`${{ steps.pin.outputs.commit }}\`"
272+
echo ""
244273
echo "- Result: ${{ job.status }}"
245274
} >> "$GITHUB_STEP_SUMMARY"
246275
if [ "${{ steps.attest.outcome }}" != "success" ]; then

‎scripts/fetch-onnxruntime.mjs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,18 @@ const ROOT = path.join(__dirname, "..");
5252
* direction to fail in.
5353
*/
5454
const VERSION = "1.27.1";
55+
56+
/**
57+
* The upstream commit `v${VERSION}` pointed at when it was reviewed.
58+
*
59+
* `v1.27.1` is a LIGHTWEIGHT tag — it points straight at a commit and can be moved by
60+
* anyone with push rights upstream. Nothing here fetches source at install time, so this
61+
* does not affect `npm run fetch:onnxruntime`; it matters to
62+
* `.github/workflows/build-onnxruntime-macos.yml`, which builds the library and would
63+
* otherwise attest an artifact to "whatever that tag meant that morning". The workflow
64+
* resolves the tag and refuses to build if it no longer resolves here.
65+
*/
66+
const SOURCE_COMMIT = "df2ba1cf8108aa63627cf4cdf8f807880b938616";
5567
const BASE = `https://github.com/microsoft/onnxruntime/releases/download/v${VERSION}`;
5668

5769
/**

0 commit comments

Comments
 (0)