Repository navigation
fix(release): make archive checks locale-independent - #984
Merged
mergify[bot] merged 1 commit intoOct 13, 2026
Merged
Conversation
XuPeng-SH
approved these changes
Oct 13, 2026
XuPeng-SH
left a comment
Collaborator
There was a problem hiding this comment.
结论:Approve。未发现需要阻止合并的明确问题。
审查范围:base / merge-base 2873a950f2c9fea00343757933e96070d1631596 → head 83a7c2cd25ef72aa472fa8e21f35a86922ac1fa1。完整阅读三个文件的修改及相关安装、发布校验、契约测试入口;提交前确认 head 未变。本轮未使用 subagent。
实现与用户影响
- 两个 archive-member 排序使用命令局部的
LC_ALL=C,与固定的LICENSE\nastra\nastra-edge字节序比较契约一致。LC_ALL 能覆盖调用者的 LANG / LC_COLLATE 等设置,不会改变整个安装进程或调用 shell 的环境;installer 保持 POSIX sh 写法。 checksums.txt的排序同样固定为 C locale,使发布清单的排序规则明确。没有引入新依赖、网络请求或运行时分支。- 校验仍是精确成员集合比较,未使用去重或模糊匹配;现有 checksum、binary version、安装暂存与 rollback 路径保持原样。
- 新测试按实际排序行为选择 locale,使用已有 fake-curl fixture 经过完整安装路径并检查两个可执行文件的版本;不依赖在线 release,也没有修改机器全局 locale。
本轮实际验证
在 Linux 本地环境独立执行,非仅引用 PR 正文:
- 原始 sort 在 C 下产生
LICENSE, astra, astra-edge,在 en_US.utf8 下产生astra, astra-edge, LICENSE,确认测试环境具有相关排序差异。 LC_ALL=C bash scripts/ci/test_release_contract.sh:通过。LC_ALL=en_US.utf8 bash scripts/ci/test_release_contract.sh:通过。完整套件包含精确 checksum sidecar、意外发布资产、被修改的 archive、非法版本及模拟安装提交失败后的 rollback 检查。- 仅临时撤回 installer 的成员排序修复、保留新测试,并在外层 C locale 运行完整套件:按预期失败,报 archive member mismatch。这确认新增 locale 测试实际执行并能检测原问题。
- 仅临时撤回 verifier 的成员排序修复,在 en_US.utf8 下运行套件:按预期失败,报合法四平台发布集的 archive member mismatch。
- 使用实际 reproducible archiver 创建四平台 fixture,再分别在 C / en_US.utf8 下调用实际 verifier:
checksums.txt和checksums.txt.sha256均逐字节相同。 - POSIX
sh -n、Bash 语法检查、git diff --check均通过;临时反向验证修改已恢复,worktree 干净。
当前 head 的 Static Checks / Test Suite 均成功;检查了 static lint 的 repository validation 执行入口。以上本地结果是 Linux 证据,不宣称本轮实际执行了 macOS 测试。
测试与复杂度评价
净增 31 行由 25 行测试和 6 行注释组成,生产修改为三条现有 sort 命令增加局部 locale 设置,复杂度与修复价值匹配。
一个非阻塞测试改进:新增的显式 UTF-8 分支目前只重跑 installer,verifier 仍取决于套件的外层 locale。后续可复用已选 locale 同时验证 verifier,并比较两份 manifest;这样普通 C-locale CI 也能持续覆盖发布端修复。本轮已经对发布端进行了上述独立正反验证,当前实现正确。
Contributor
Merge Queue Status
This pull request spent 19 minutes 29 seconds in the queue, including 19 minutes 8 seconds running CI. Required conditions to merge
|
The installer and the release artifact verifier compare 'tar -tzf | sort' output against a hard-coded C byte-order expectation (LICENSE, astra, astra-edge), but sort follows the caller's locale collation. Under UTF-8 locales (macOS en_US.UTF-8/zh_CN.UTF-8, glibc en_US.UTF-8) collation is case-insensitive, so every real install and any UTF-8-locale release verification failed with 'must contain exactly astra, astra-edge, and LICENSE at its root'. CI passed only because GitHub runners use C/C.UTF-8. Pin LC_ALL=C on both member sorts and on the checksums.txt manifest ordering so published artifacts are byte-identical regardless of locale. The release contract test now probes for a case-reordering UTF-8 locale and re-runs the installer member check under it, skipping gracefully where no such locale exists. Fixes matrixorigin#982
mergify
Bot
force-pushed
the
fix/install-sort-locale-982
branch
from
October 13, 2026 11:59
83a7c2c to
d33b4c7
Compare
8 of 10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The release archive member checks compared
tar -tzf | sortoutput against a hard-coded C byte-order expectation (LICENSE,astra,astra-edge), butsortfollows the caller's locale collation. Under UTF-8 locales (macOS defaulten_US.UTF-8,zh_CN.UTF-8, glibcen_US.UTF-8) collation is case-insensitive, so the members sort asastra astra-edge LICENSEand:scripts/install-astra.shfailed every real install withrelease archive must contain exactly astra, astra-edge, and LICENSE at its root.scripts/verify-release-artifacts.sh(the pre-publication gate) rejected valid artifact sets with the equivalent error, and itssort -k2ordering ofchecksums.txtdepended on the publishing machine's locale, threatening non-reproducible manifests.Fix: pin
LC_ALL=Con both member sorts and on thechecksums.txtmanifest sort, so archive validation and published manifest bytes are locale-independent.scripts/ci/test_release_contract.shnow probes for a case-reordering UTF-8 locale (en_US/zh_CN candidates) and re-runs the installer's member-check path under it, skipping gracefully on runners that provide no such locale, so the regression stays covered where the locale exists without failing locale-poor environments.Related issue
Fixes #982
Change type
User and compatibility impact
curl | shinstalls now succeed on machines running UTF-8 locales (all default macOS setups, many Linux desktops) instead of aborting at the archive check.verify-release-artifacts.shno longer rejects valid release sets on such machines, andchecksums.txtis now byte-identical regardless of the publishing locale (unchanged for CI, which already ran under a C-locale). No configuration, flags, or API surface changed.Architecture and complexity delta
sort/comm/join/uniqunderscripts/*.shandscripts/ci/*.shwas audited (nocomm/joinuses exist).LC_ALL=Cpin plus one regression block.Inspected and deliberately left alone:
scripts/reconcile-docker-manifest.sh:53,153,158,191—sort -uover lowercase-ASCII-only platform names (linux/amd64,linux/arm64) and hex digests; no case mixing, and the candidate/published files compared viacmpare both produced by the same sort, so ordering is self-consistent.scripts/reconcile-docker-manifest.sh:105,scripts/validate-release-version.sh:157— Pythonsorted(), which is codepoint order and locale-independent by definition.scripts/phase0-production-baseline.sh:139,143—sort -zonly stabilizes input enumeration for harness assembly; not compared against a fixed C-order literal.scripts/ci/test_interactive_setup_contract.sh:411—sort -u | wc -l; a count-only assertion where order is never observed.Verification
LANG=en_US.UTF-8,LC_COLLATE=en_US.UTF-8, no outerLC_ALLoverride unless noted):install-astra.shback to| sort):PYTHONPATH="$PWD/.pyshim-982" bash scripts/ci/test_release_contract.sh→ exit 1,error: release archive must contain exactly astra, astra-edge, and LICENSE at its rootfrom the new UTF-8-locale installer run.verify-release-artifacts.shback to| sort):PYTHONPATH="$PWD/.pyshim-982" bash scripts/ci/test_release_contract.sh→ exit 1,astra-v0.1.0-linux-amd64.tar.gz must contain exactly astra, astra-edge, and LICENSE at its root.PYTHONPATH="$PWD/.pyshim-982" bash scripts/ci/test_release_contract.sh→ exit 0.PYTHONPATHonly supplied a throwawaytomli→tomllibshim because the local dev host's defaultpython3is 3.9; CI runners use Python ≥ 3.11 wheretomllibis stdlib. The shim was never committed and was deleted after the runs.scripts/install-astra.shfull download→verify→install path via the suite's fake-curlrelease fixture, both under the ambient locale and under the case-reordering UTF-8 locale; plusscripts/verify-release-artifacts.shover a full four-platform dist set.CI never caught this because GitHub-hosted runners execute under C.UTF-8/POSIX locale, where unpinned
sortalready produces byte order; only developer machines with UTF-8 default locales hit it.Final checklist