Conversation
Both CI jobs currently fail two seconds after starting, before any build
step runs:
##[error]This request has been automatically failed because it uses a
deprecated version of `actions/cache: v2`.
GitHub has closed down actions/cache v1 and v2 and now auto-fails runs
that reference them at action-download time, so the workflow cannot get
as far as `make test` or the bazel e2e target.
Bump actions/cache to v4 to unblock that, and bump the remaining actions
still on the Node 12 runtime at the same time:
- actions/checkout v2 -> v4
- actions/setup-node v2.1.4 -> v4
- haskell/actions/setup v1 -> haskell-actions/setup v2 (repository moved)
Tool versions are left as they are (GHC 8.8.3, stack 2.5.1, Node 12.x) so
this change is limited to action versions. haskell-actions/setup v2 still
exposes the stack-path output the cache step depends on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
With the deprecated actions bumped, the Haskell job now reaches `make test` and fails there instead: `make test` depends on the `schema` target, which fetches the EDItEUR archives, and editeur.org now answers bazel's request with `202 Accepted` instead of the zip. The test suite does not need those archives. Everything under test/ reads fixtures/test_*.xsd; the only reader of ./schema is schemaRoot in src/Lib.hs, which is the code-generation path. The dependency was an over-specification that tied the whole feedback loop for the parser to the availability of an external service. Drop it from `test` and keep it on `build`, where generating code really does need the schema. This does not fix the 202 itself: generation and tracking new schema releases still need the download. Also introduce docs/adr/ to record decisions like this one, with a template, an index, and the two decisions made here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
The previous commit dropped the `schema` prerequisite from `make test` on the claim that nothing under test/ reads schema/. That claim was wrong, and review caught it: fixtures/test_mixed_html.xsd included ../schema/v2/ONIX_XHTML_Subset.xsd directly. Xsd.getSchema follows includes recursively and resolves that relative path with readFile, which throws when the file is absent, and schema/ is gitignored — so on a clean checkout the suite would have failed instead of running. Commit 9352123 confirms the dependency was deliberate: it added `test: schema` in the same change that deleted the vendored 2_1_rev03_schema/ tree and repointed this fixture at ../schema/v2/. Rather than restore the prerequisite, move what the fixture needs into the repository. test_mixed_html_xhtml_subset.xsd declares the 40 element names the fixture refers to, each as a mixed complex type. That is the property the assertions actually rest on: TestModel expects Model.collectElements to come back empty, and that filter keeps only elements with complexMixed = False, so the test means "XHTML elements do not leak into models". Deleting the include instead would have made the assertion vacuous. The stand-in is written here rather than copied from the EDItEUR distribution, so it raises no redistribution question. ADR-0002 is rewritten around what is actually true, including why the dependency was real and why a stand-in is enough. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
BZL_BIN used `:=`, so `$(shell npx bazel info bazel-bin)` ran while make parsed the file — on every target, `make test` included. The Haskell job runs `make test` without ever installing node_modules, so that expansion just fails there. Measured in this environment: `make -n test` took 18.1s and printed a bazel download error before doing anything, and 0.011s with the assignment deferred to `=`. Nothing outside the schema recipes reads BZL_BIN, so deferring it costs nothing. Also drop the `ls -lah` step that printed the stack tool path; it was debugging output for the cache setup, not a check anything depends on. Both spotted in review of this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
build_bazel_rules_nodejs 3.1.0 was pulled in for two things. One of them, the @npm repository from npm_install, is referenced by no BUILD target at all — //e2e/go:snapshot_test only consumes a Go binary and two JSON files. The other, generated_file_test, compares two files. That is not worth an external repository, and it actively blocks updating Bazel: 3.1.0 predates current Bazel releases, and generated_file_test does not exist in rules_nodejs 5.x/6.x, so bumping the dependency would mean rewriting the comparison anyway and keeping the dependency for nothing. Compare with diff -u in a sh_test instead. It has no Bazel version constraints of its own, so it stops being a factor in the version bump, and it prints what actually differs rather than only that something does. Snapshot refresh instructions are in the script. bazel_skylib's diff_test was the obvious alternative, but it swaps one dependency for another, and which skylib version pairs with Bazel 3.7.0 cannot be checked from here (releases.bazel.build is blocked by egress policy). Removing the dependency leaves nothing to verify. npx bazelisk is unaffected — that comes from the npm devDependency, not from rules_nodejs. Recorded as ADR-0004. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
総評Bazel の配線は正しく、意図どおり動きます。 指摘必須1.
推奨2. ADR L65-66 は「更新は手作業になった、手順は json: fixtures/20201200.json
fixtures/20201200.json: run
go run github.com/kogai/onix-codegen/go/helper
任意3. CI ログに毎回出ています。
4. echo "If the change is intended, refresh the snapshot as described in $0." >&2テスト実行時の 5. ADR の「結果」に、旧 旧定義は引数が逆でした。 generated_file_test(
name = "snapshot_test",
src = "snapshot", # ← ルールの定義では src = ワークスペース側のソース
generated = "//:fixtures/20201200.json", # ← generated = Bazel が生成した出力
)rules_nodejs 3.1.0 の docstring は 6.
確認したこと
結論:
実行権限:
ADR の引用行番号: base ( CI が壊れないこと: ADR のその他の記述: 「WORKSPACE の外部依存が 1 つ減る」(http_archive 5 → 4)、「3.1.0 は新しい Bazel との組み合わせが保証されない」(3.1.0 の 確認できなかったこと
Generated by Claude Code |
ADR-0004 said generated_file_test does not exist in rules_nodejs 5.x/6.x. That is only true of 6.x; 4.7.0 and 5.8.5 still ship it. The actual blocker on 5.8.5 is SUPPORTED_BAZEL_VERSIONS = ["4.2.2", "5.0.0"], which does not include the 3.7.0 this repo pins — so upgrading rules_nodejs alone was never possible, and going all the way to 6.x means rewriting the comparison regardless. The decision is unchanged; the reason it gave was wrong. The ADR declared snapshot updates manual while leaving Makefile's `json` target in place. That target was broken twice over: it depends on a `run` target that does not exist, and runs github.com/kogai/onix-codegen/go/helper, which is not a real package — the helper lives in e2e/go. Remove it, and record in the ADR that the old generated_file_test had src and generated the wrong way round, so its .update path wrote in the wrong direction too. Also give the sh_test size = "small", which silences the "test size is too big" warning Bazel prints, and make the failure message name the script by path rather than $0, which resolves to a sandbox path at test time. All from review of this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
|
レビューありがとうございます。必須・推奨・任意すべて反映しました (7a6d7de)。 必須: ADR-0004 の事実誤認ご指摘のとおりです。「 より重要なのは、ご提示いただいた本当の行き止まりの理由です。5.8.5 の
結論（外す）は変わりませんが、理由が間違っていたので直しました。ADR は後から読む人が信頼する記録なので、ここが誤っているのは実質的な欠陥だと思います。 推奨: Makefile の死んだ更新経路確認しました。二重に壊れています。 json: fixtures/20201200.json
fixtures/20201200.json: run ← run ターゲットは存在しない
go run github.com/kogai/onix-codegen/go/helper ← 実体は e2e/goADR で「更新は手作業」と宣言しながらこれを残すのは矛盾なので、削除しました。 任意
検証について
なお e2e ジョブは 71d2457 で実際に緑になっています（ Generated by Claude Code |
First CI run on this branch:
ERROR: e2e/go/BUILD.bazel:35:1: name 'sh_test' is not defined
(did you mean 'cc_test'?)
Bazel 9 finished moving the native shell rules out of the global
namespace and into rules_shell. Add the bazel_dep and load sh_test from
@rules_shell//shell:sh_test.bzl.
This falsifies the claim ADR-0006 and the pull request both made, that no
BUILD file changes — one load line does. Corrected there, along with a
note that further Bazel bumps can be expected to need the same treatment
as more native rules are Starlarkified.
The Makefile conflict from merging #61 is resolved by taking both
deletions: that branch removed the dead `json` target and this one
removed the `WORKSPACE` target, and neither should come back.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
1 participant
Bazel 側のライブラリ更新の第一歩です。base は #56。
実態
build_bazel_rules_nodejs3.1.0 が使われているのは 2 箇所だけでした。このうち
npm_installが定義する@npmは、どの BUILD ターゲットからも参照されていません（#57 のレビューでも指摘された点です）。//e2e/go:snapshot_testが使うのは Go のバイナリと JSON 2 つだけです。つまり実質的な用途はgenerated_file_testひとつでした。なぜ「上げる」ではなく「外す」のか
やっていることは「2 つのファイルが同一か」の比較です。そのために外部リポジトリを丸ごと取得するのは釣り合っていません。
加えて、この依存はライブラリ更新の妨げになっています。3.1.0 は 2021 年のリリースで新しい Bazel との組み合わせが保証されず、一方
generated_file_testは rules_nodejs 5.x/6.x には存在しません。上げるにせよどのみち比較の仕組みを書き換える必要があり、書き換えたうえで依存だけが残ります。変更内容
e2e/go/snapshot_test.shを追加し、sh_testから呼びます。diff -uで比較し、差分があれば差分そのものを表示して失敗します（generated_file_testは不一致の事実しか出しませんでした）。スナップショットの更新手順はスクリプト冒頭のコメントにあります。bazel_skylibのdiff_testに置き換える案も検討しましたが、依存を別の依存に入れ替えるだけで減らず、しかも skylib のどのバージョンが Bazel 3.7.0 と組み合わせられるかをこの環境では検証できません（releases.bazel.buildに到達不可）。検証できない選択肢を抱えるより、依存を減らす方を採りました。ADR-0004 に記録しています。npx bazeliskは npm の devDependency 由来で、rules_nodejs とは無関係なので影響ありません。確認
grep -c 'nodejs\|npm' WORKSPACE→ 0）sh_testの$(location)展開や runfiles の解決が期待どおりかは、この PR の e2e ジョブが唯一の検証手段です。赤が出たら追って直します🤖 Generated with Claude Code
https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z
Generated by Claude Code