Repository navigation
Run the ESQL test suites on Windows in CI - #881
Open
yutaro-sakamoto wants to merge 8 commits into
Open
yutaro-sakamoto wants to merge 8 commits into
yutaro-sakamoto wants to merge 8 commits into
Conversation
codegen.c kept the inside_check/inside_stack bookkeeping from the original C code generator, but only the decrements survived the rewrite to Java code generation: nothing ever increments inside_check. On non-GCC builds (the MSVC-built cobj.exe) the counter goes negative as soon as a runtime check is attached to a reference (-debug with a variable subscript, reference modification or SEARCH ALL), and inside_stack[inside_check - 1] then writes out of bounds, crashing the compiler. GCC builds never compile these blocks, which is why Linux was unaffected. Remove the machinery entirely: the generated Java must not depend on the compiler that built cobj. Un-skip the Windows tests that were failing due to this crash. Fixes opensourcecobol#829, opensourcecobol#831, opensourcecobol#832, opensourcecobol#833, opensourcecobol#834 (sub-issues of opensourcecobol#828)
With -java-package the jar arguments were assembled as package_dir + file_path_delimitor + class, mixing the '/' separators from the package path with '\' on Windows (e.g. com/abc\prog.class), which produces malformed entry names in the archive. The jar tool accepts '/' everywhere, so join the paths with '/' unconditionally. The class file cleanup after archiving had the opposite problem: cmd.exe's del does not accept '/' separators. Build the remove paths with the native separator, let cd handle the output directory, and drop a leftover '#aaa' token that del would treat as an argument. Un-skip the Windows jar test. Fixes opensourcecobol#830 (sub-issue of opensourcecobol#828)
Removing the inside_check block exposed a variableScope finding for 'code' in joutput_stmt; declare it in the handler block where it is used.
Every program in a translation unit becomes its own .java/.class file named after the program ID, so two programs whose IDs differ only in letter case overwrite each other's files on a case-insensitive filesystem and the result silently misbehaves. On Windows, detect the collision before code generation and fail with an explicit error. The two affected tests stay skipped on Windows because the behavior they exercise is impossible there by design; a new Windows-only test verifies the diagnostic instead. Fixes opensourcecobol#835 (sub-issue of opensourcecobol#828)
On case-insensitive file systems programs whose PROGRAM-IDs differ only in letter case would generate class files with the same name; cobj now reports a compile error there, so document the restriction in the usage section.
* Use "cd /d" on Windows: cmd.exe's plain "cd" does not switch drives, so with -o pointing at another drive the jar and del commands would run against the wrong directory * Replace the unbounded strcpy into package_dir_native with snprintf and give package_name_to_path an explicit output bound, so an overlong -java-package argument can no longer overflow the buffers
The Windows ESQL job has been present but commented out in push.yml and pull-request.yml since the ESQL support was merged, because the workflow could not bring up PostgreSQL on the runner. initdb was called with --encoding=UTF8 while the runner's default locale is English_United States.1252, and initdb rejects that combination outright: the encoding a locale implies (WIN1252) has to match the one asked for. Pass --no-locale so the cluster is created with the C locale, which is compatible with every encoding. The suites only sort by numeric columns and by values that are prefixes of each other, so the collation that gives is fine, and lc_messages=C keeps the PostgreSQL error texts esql-misc matches in English. That alone is not enough. esql-sqlca.src/connect-disconnect.at connects to a database named "invalid" with no host part, which libcobj turns into a JDBC URL for localhost:5432, and it expects the attempt to be refused (SQLCODE -402 / SQLSTATE 08001). On Linux that holds because PostgreSQL runs in a separate service container; on Windows the server is on the same machine, so the connection reaches it and fails authentication instead, with the role "invalid" not existing (SQLCODE -9999 / SQLSTATE 28000). The cluster therefore listens on 55432, leaving 5432 closed. Rather than leaving any of this inline in the workflow, where nothing outside CI can reach it: - win/start-test-postgresql.ps1 initialises the cluster and starts it, and takes the same steps on a developer's own Windows machine: it reuses a cluster it finds instead of failing, -Stop shuts the server down, and the port, cluster directory, database, role and PostgreSQL installation are all parameters. Its comment header records why --no-locale and the non-default port are required. - .github/workflows/db-settings/embed_db_info_windows.sh joins the scripts already there for the container hosts, and the workflow copies it into place. start-test-postgresql.ps1 reads its own default port, database and role out of that file, so the server and the compiled test programs cannot disagree. Also: - Check the exit status of initdb / pg_ctl / createdb and print the server log when one of them fails, instead of letting the job continue to the tests and fail there with a connection error. - Give each suite its own job, the way windows-test-other.yml does, so a single suite can be re-run, and so that each suite gets a cluster to itself. - Define RUN_MODULE_LOG in tests/atlocal-win.env. Only tests/esql-misc.src/logging.at uses it, and it was missing, so that test ran the compiled program without a java command in front of it. - Drop esql-utf8: it needs a compiler configured with --enable-utf8 and the Windows build is Shift_JIS only, which is also why no other UTF-8 suite runs on Windows.
yutaro-sakamoto
marked this pull request as ready for review
August 8, 2026 12:49
This branch has not been deployed
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.
#862 の上に積んでいます。#862 がマージされるまで、このPRの差分にはそちらのコミットも含まれます。
概要 (日本語)
ESQL のテストスイートは Linux でしか CI で実行されていません。Windows 用のワークフロー
.github/workflows/windows-test-esql.ymlは ESQL サポートのマージ時から存在していますが、push.yml/pull-request.ymlの側でコメントアウトされたままです。動かなかった原因
1.
cobj.exeが ESQL プログラムのコンパイル中に segfault していた。developの上でこのジョブを有効にすると、24 個のテストがcobj.exeのクラッシュで落ちます。コメントに残っていた「cobj.exe segfaults with the new esql-scanner」という推測とは異なり、原因は esql-scanner ではなくcodegen.cのinside_check/inside_stackでした。ESQL のテストは-debug付きで添字付きホスト変数を使うため、ほぼ全テストがこれを踏みます。#862 で修正済みのため、本PRはその上に積んでいます。2.
initdbがクラスタを作れていなかった。--encoding=UTF8を指定する一方でロケールを指定していませんでした。ランナーの既定ロケールEnglish_United States.1252は WIN1252 を要求するため、initdbはこの組み合わせを拒否します。--no-localeを渡し、どのエンコーディングとも両立する C ロケールでクラスタを作るようにしました。3.
esql-sqlcaが 1 グループだけ失敗していた。esql-sqlca.src/connect-disconnect.atはホスト部を持たないinvalidというデータベースへ接続し、libcobj がそれをlocalhost:5432の JDBC URL に変換した結果、接続が拒否されること (SQLCODE -402 / SQLSTATE 08001) を期待しています。Linux では PostgreSQL が別のサービスコンテナで動くためこれが成立しますが、Windows では同一マシン上にサーバがあるため接続がサーバまで届いてしまい、ロールinvalidが存在しない認証エラー (SQLCODE -9999 / SQLSTATE 28000) になります。クラスタを 55432 番ポートで起動し、5432 番を空けたままにしています。PostgreSQL の起動処理とDB接続情報の外出し
ワークフローに直接書くのではなく、いずれもファイルに切り出しました。CI 以外からも使えるようにするためです。
win/start-test-postgresql.ps1— クラスタの初期化と起動を行います。CI 以外の Windows マシンでもそのまま使えるように、既存クラスタがあれば再利用し、-Stopで停止でき、ポート・データディレクトリ・データベース名・ロール名・PostgreSQL の場所をすべてパラメータにしてあります。--no-localeと既定以外のポートが必要な理由は、スクリプト冒頭のコメントに記録しました。.github/workflows/db-settings/embed_db_info_windows.sh— 既存のコンテナ用スクリプトと同じ形式の Windows 版です。ワークフローはこれをtests/embed_db_info.shにコピーします。接続情報の唯一の出どころとなるよう、start-test-postgresql.ps1は既定のポート・データベース名・ロール名をこのファイルから読み取ります。なお
db-settings/のembed_db_info_postgresql_15.sh/_9.6.shは現状どこからも使われていません。Linux の ESQL ジョブはmake distの tar ボールから実行されてactions/checkoutを行わず、tar ボールに.github/が含まれないためです (同じ内容をワークフロー内で直接書き出しています)。Windows のジョブはリポジトリをチェックアウトするので、こちらはdb-settings/を使えます。その他
initdb/pg_ctl/createdbの終了ステータスを確認し、失敗したらサーバログを出力します。windows-test-other.ymlと同じく、スイートごとに 1 ジョブへ分割しました。各ジョブが専用のクラスタを持つため、スイート同士が干渉しません。tests/atlocal-win.envにRUN_MODULE_LOGを追加しました。tests/esql-misc.src/logging.atだけが使う変数ですが未定義だったため、コンパイル済みプログラムがjavaを伴わずに実行されていました。esql-utf8は対象外です。--enable-utf8付きでコンフィグアしたコンパイラが必要ですが、Windows ビルドは Shift_JIS 専用のためです (他の UTF-8 スイートが Windows で実行されていないのと同じ理由です)。Summary (English)
Stacked on #862; its commits appear in this diff until that one merges.
The ESQL suites only run in CI on Linux.
.github/workflows/windows-test-esql.ymlhas existed since ESQL support was merged, but the job is still commented out inpush.ymlandpull-request.yml.Why it did not work
1.
cobj.exesegfaulted while compiling the ESQL programs. Enabling the job ondevelopfails 24 tests with a compiler crash. Contrary to the note left in the workflow ("cobj.exe segfaults with the new esql-scanner"), the cause is not the esql-scanner but theinside_check/inside_stackmachinery incodegen.c: the ESQL tests compile with-debugand use subscripted host variables, so nearly all of them hit it. #862 fixes that, which is why this is stacked on it.2.
initdbnever created a cluster. It asked for--encoding=UTF8without naming a locale, which conflicts with the runner's defaultEnglish_United States.1252— that locale requires WIN1252, andinitdbrejects the combination outright. Passing--no-localecreates the cluster with the C locale, which is compatible with every encoding.3. One
esql-sqlcagroup still failed.esql-sqlca.src/connect-disconnect.atconnects to a database namedinvalidwith no host part. libcobj turns that into a JDBC URL forlocalhost:5432, and the test expects the attempt to be refused (SQLCODE -402 / SQLSTATE 08001). On Linux that holds because PostgreSQL runs in a separate service container; on Windows the server is on the same machine, so the connection reached it and failed authentication instead, with the roleinvalidnot existing (SQLCODE -9999 / SQLSTATE 28000). The cluster now listens on 55432, leaving 5432 closed.Moving the setup out of the workflow
Both the PowerShell that brought PostgreSQL up and the heredoc that wrote
embed_db_info.shwould have been inline in the workflow, where nothing outside CI can reach them. Both are files instead:win/start-test-postgresql.ps1initialises the cluster and starts it, and takes the same steps outside CI: it reuses a cluster it finds instead of failing,-Stopshuts the server down, and the port, cluster directory, database, role and PostgreSQL installation are all parameters. Its comment header records why--no-localeand the non-default port are required..github/workflows/db-settings/embed_db_info_windows.shsits next to the scripts already there for the container hosts, and the workflow copies it into place.start-test-postgresql.ps1reads its own default port, database and role out of that file, so it is the single source for the connection settings.Note that
db-settings/embed_db_info_postgresql_15.shand_9.6.share currently unused: the Linux ESQL jobs run from themake disttarball and never check the repository out, and.github/is not part of the tarball, so they cannot read them and write the same content inline instead. The Windows job does check the repository out, so it can usedb-settings/.Also
initdb/pg_ctl/createdband print the server log when one of them fails.windows-test-other.ymldoes, so each suite gets a cluster to itself.RUN_MODULE_LOGintests/atlocal-win.env. Onlytests/esql-misc.src/logging.atuses it, and it was missing, so that test ran the compiled program without ajavacommand in front of it.esql-utf8is excluded: it needs a compiler configured with--enable-utf8and the Windows build is Shift_JIS only, which is also why no other UTF-8 suite runs on Windows.All jobs pass on this branch.