fix: upload canister wasms under their intended CDN filenames#10666
fix: upload canister wasms under their intended CDN filenames#10666basvandijk wants to merge 1 commit into
Conversation
`copy_file` defaults `allow_symlink` to True (for non-executable outputs), so each `.wasm.gz` in publish/canisters was a symlink to the underlying canister artifact. `artifact_bundle` resolves inputs with `realpath` and the uploader (ci/src/artifacts/upload.sh) names the uploaded object after the resolved basename, so files were uploaded under the underlying target's name (e.g. `governance-canister-test.wasm.gz`) instead of the intended CDN name (e.g. `governance-canister_test.wasm.gz`). This was latent until #10641 pinned `artifact_bundle` to run locally (`no-remote`): running remotely materialized inputs by content at their declared (correct) path, whereas running locally stages the copy_file symlink, so `realpath` now follows it to the underlying dash-named artifact. Force a hard copy (`allow_symlink = False`), mirroring the existing `.did` copy, so the resolved filename keeps the intended name. This restores `governance-canister_test.wasm.gz` and other affected canisters without reverting #10641.
There was a problem hiding this comment.
Pull request overview
This PR fixes a CDN publishing regression where canister .wasm.gz artifacts were uploaded under the underlying target’s resolved (dash) filename instead of the intended (underscore) filename by ensuring the published .wasm.gz outputs are hard copies rather than symlinks.
Changes:
- Set
allow_symlink = Falsefor thecopy_filerules that produce published*.wasm.gzoutputs. - Document why hard-copying is required to preserve the intended CDN object name (aligning behavior with existing
.didhandling).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # the uploader (ci/src/artifacts/upload.sh) names the uploaded object | ||
| # after the *resolved* basename. A symlink therefore resolves to the |
There was a problem hiding this comment.
the uploader [...] names the uploaded object after the resolved basename
Seems like THIS is the bug, no? I mean, maybe this is more difficult to fix, but if we put that issue aside for a moment, it seems to me that if I ask for a/b/c.d to be uploaded to s3/some/path, then, I would expect the result to be a copy at s3/some/path/a/b/c.d, not some random path that a/b/c.d happens to symbolically link to (e.g. s3/some/x/y.z). I mean, who would ever want s3/some/x/y.z?? At least to me, it seems to violate The Principle of Least Astonishment, but maybe there are some "obvious" pieces that I'm missing here...
Anyway, I don't own this -> I'm not trying to block. Just suggest. (Hence, I approve.)
There was a problem hiding this comment.
I'm actually working on a fix for the root cause but that might take a bit longer.
Problem
governance-canister_test.wasm.gz(and other canisters) stopped being available on the CDN under their intended names, e.g. for a recent commit:…/canisters/governance-canister_test.wasm.gz→ 404…/canisters/governance-canister-test.wasm.gz→ present (note the dash)…/canisters/governance-canister_test.wasm.gz.did→ present (underscore)The upload log even reports uploading the underscore name, yet the object lands under the underlying target's dash name.
Root cause
publish/canisters/BUILD.bazel, each.wasm.gzis produced bycopy_filewithoutallow_symlink = False. bazel_skylib defaultsallow_symlinktoTruefor non-executable outputs, so the output is a symlink to the underlying canister artifact (e.g.//rs/nns/governance:governance-canister-test→governance-canister-test.wasm.gz, dash).artifact_bundle(publish/defs.bzl) recordsln -s "$(realpath "$input")"— the fully-resolved (dash) path.ci/src/artifacts/upload.shnames the uploaded object afterbasename "$(readlink -f …)"— the resolved (dash) basename — not the bundle entry (underscore) shown in the log line andbucket_path.The
.didfiles useallow_symlink = False(hard copy), which is exactly why they keep uploading under the correct underscore name.This was latent until #10641 pinned
artifact_bundleto run locally (no-remote). Running remotely materialized inputs by content at their declared (correct) path; running locally stages thecopy_filesymlink, sorealpathnow follows it through to the dash-named artifact.Fix
Force a hard copy (
allow_symlink = False) for the.wasm.gzcopies, mirroring the existing.didcopy. The bundle then resolves to the real file carrying the intended name, restoringgovernance-canister_test.wasm.gzand the other affected canisters — without reverting #10641.Notes
The deeper issue is that
ci/src/artifacts/upload.shnames objects by the resolved basename rather than the bundle-relative name (rclone copyto "$bucket_path"would be the general fix). This PR is the minimal, low-risk fix consistent with the existing.didhandling.