Add Windows CI/CD workflow for PHP extension builds#14
Merged
Conversation
Extract the Windows DLL build matrix from release.yml into a standalone workflow that runs on push/PR when ext/ files change. This catches Windows build breakages early instead of only at release time. https://claude.ai/code/session_01KtCQUuRcfQxhRd8esNdkrM
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughWindows向けの新しいGitHub Actionsワークフローを追加し、リリースワークフローからPHP 7.4を除外。さらに拡張内でWindowsエラー→errnoマッピング用の内部ヘルパーを導入し、既存の_dosmaperr呼び出しを置換しています。 Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Runner as Windows Runner (win-*)
participant PHP as Setup PHP SDK
participant MSVC as Enable-MSVC (vcvars)
participant Build as Build Tools (phpize/configure/nmake)
participant FS as Filesystem (artifacts)
GH->>Repo: push/PR triggers workflow
GH->>Runner: provision Windows runner
Runner->>PHP: setup-php action (matrix 8.0-8.4, NTS/TS)
Runner->>MSVC: Enable-MSVC Developer Command Prompt (vs16/vs17)
Runner->>Build: run phpize -> configure --enable-mariadb_profiler -> nmake
Build->>FS: produce php_mariadb_profiler.dll
Runner->>FS: verify DLL exists and output path/size
GH->>GH: mark job success/failure
推定レビュー作業量🎯 3 (Moderate) | ⏱️ ~20 分 Possibly related PRs
説明（更新済み）新しいGitHub Actionsワークフロー 変更内容（更新済み）
ポエム
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/windows-build.yml (3)
15-18: ジョブ名にmatrix.osを含めることを検討Line 17 のジョブ名には
matrix.osが含まれていませんが、runs-on: windows-${{ matrix.os }}で参照されています。現在はos: ['2022']のみですが、将来OSバージョンを追加した場合に名前が重複します。💡 ジョブ名の改善案
- name: Windows - PHP ${{ matrix.php }} ${{ matrix.ts }} ${{ matrix.arch }} + name: Windows ${{ matrix.os }} - PHP ${{ matrix.php }} ${{ matrix.ts }} ${{ matrix.arch }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/windows-build.yml around lines 15 - 18, The job name "build-windows" uses the name field "Windows - PHP ${{ matrix.php }} ${{ matrix.ts }} ${{ matrix.arch }}" but omits `${{ matrix.os }}`, which can cause duplicate job names if multiple OS versions are added; update the job's name string to include `${{ matrix.os }}` (e.g., append or insert `${{ matrix.os }}` into the existing name template) so it matches `runs-on: windows-${{ matrix.os }}` and remains unique when matrix.os grows.
74-84: ビルド成果物のアップロードが未設定DLLの存在確認は行っていますが、
actions/upload-artifactによるアーティファクトのアップロードステップがありません。CI専用ワークフローとして意図的であれば問題ありませんが、PRレビュー時にビルド成果物を確認したい場合やデバッグ時に有用です。必要に応じて追加を検討してください。💡 アーティファクトアップロードの追加案
- name: Verify DLL exists shell: pwsh run: | $dll = Get-ChildItem -Path ext\mariadb_profiler -Recurse -Filter "php_mariadb_profiler.dll" | Select-Object -First 1 if (-not $dll) { Write-Error "php_mariadb_profiler.dll not found" exit 1 } Write-Host "Built DLL: $($dll.FullName)" Write-Host "Size: $([math]::Round($dll.Length / 1KB, 1)) KB" + echo "DLL_PATH=$($dll.FullName)" >> $env:GITHUB_OUTPUT + id: verify + + - name: Upload DLL artifact + uses: actions/upload-artifact@v4 + with: + name: php_mariadb_profiler-php${{ matrix.php }}-${{ matrix.ts }}-${{ matrix.arch }} + path: ${{ steps.verify.outputs.DLL_PATH }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/windows-build.yml around lines 74 - 84, The workflow verifies the DLL but never uploads it; add an artifacts upload step using actions/upload-artifact immediately after the "Verify DLL exists" step so reviewers can download the built DLL; reference the same artifact name and path used in the check (e.g., upload php_mariadb_profiler.dll using the resolved path from $dll.FullName or a glob like ext/mariadb_profiler/**/php_mariadb_profiler.dll) and set a clear artifact name (e.g., php_mariadb_profiler) so CI stores the build output for debugging and PR inspection.
29-43: 未使用のmatrix.vsフィールドを削除する
includeセクション（行 29-43）で各PHPバージョンにvsフィールドを追加していますが、ワークフロー内のどのステップでも参照されていません。行 60 では${{ steps.setup.outputs.toolset }}を使用して、php/setup-php-sdkアクションが自動的に決定したツールセットを参照しています。
matrix.vsは機能的な目的を果たさない不要な設定です。php/setup-php-sdkは PHPバージョンに基づいて正しいツールセットを自動的に出力するため、このフィールドは削除できます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/windows-build.yml around lines 29 - 43, Remove the unused matrix.vs entries from the include block (the per-version "vs" fields) since no workflow step reads them; rely on the php/setup-php-sdk action which emits steps.setup.outputs.toolset to pick the correct toolset automatically (so edit the include list that currently contains php: '7.4' vs: vs16 ... php: '8.4' vs: vs17 and leave only the php versions), and ensure no other step references matrix.vs anywhere in the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/windows-build.yml:
- Around line 66-68: `%INSTALL_DIR%` が未定義で configure --with-prefix=%INSTALL_DIR%
が失敗するので、php/setup-php-sdk@v0.12 の prefix 出力を取り込んで環境変数に設定してください；具体的には Setup PHP
SDK ステップに id: setup を付け（既にある場合はそのまま）、その直後に "Set INSTALL_DIR" ステップを追加して
steps.setup.outputs.prefix を %GITHUB_ENV% に書き出して INSTALL_DIR をエクスポートし、これにより
ext\mariadb_profiler の configure ステップの --with-prefix=%INSTALL_DIR%
が正しい値を参照するようにします（同じ領域で Enable MSVC Developer Command Prompt に
steps.setup.outputs.toolset を渡すことも維持してください）。
---
Nitpick comments:
In @.github/workflows/windows-build.yml:
- Around line 15-18: The job name "build-windows" uses the name field "Windows -
PHP ${{ matrix.php }} ${{ matrix.ts }} ${{ matrix.arch }}" but omits `${{
matrix.os }}`, which can cause duplicate job names if multiple OS versions are
added; update the job's name string to include `${{ matrix.os }}` (e.g., append
or insert `${{ matrix.os }}` into the existing name template) so it matches
`runs-on: windows-${{ matrix.os }}` and remains unique when matrix.os grows.
- Around line 74-84: The workflow verifies the DLL but never uploads it; add an
artifacts upload step using actions/upload-artifact immediately after the
"Verify DLL exists" step so reviewers can download the built DLL; reference the
same artifact name and path used in the check (e.g., upload
php_mariadb_profiler.dll using the resolved path from $dll.FullName or a glob
like ext/mariadb_profiler/**/php_mariadb_profiler.dll) and set a clear artifact
name (e.g., php_mariadb_profiler) so CI stores the build output for debugging
and PR inspection.
- Around line 29-43: Remove the unused matrix.vs entries from the include block
(the per-version "vs" fields) since no workflow step reads them; rely on the
php/setup-php-sdk action which emits steps.setup.outputs.toolset to pick the
correct toolset automatically (so edit the include list that currently contains
php: '7.4' vs: vs16 ... php: '8.4' vs: vs17 and leave only the php versions),
and ensure no other step references matrix.vs anywhere in the workflow.
Comment on lines
+66
to
+68
| - name: configure | ||
| working-directory: ext\mariadb_profiler | ||
| run: configure --enable-mariadb_profiler --with-prefix=%INSTALL_DIR% |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/windows-build.ymlRepository: 39ff/php-ext-mariadb-salvage
Length of output: 2860
🌐 Web query:
php/setup-php-sdk action outputs environment variables INSTALL_DIR
💡 Result:
php/setup-php-sdk does not export an environment variable named INSTALL_DIR.
What it does provide:
- It writes these step outputs:
toolset,prefix,vs. [1] - The PHP “install dir” is effectively the
prefixoutput, which is set to"$pwd\\php-bin"(thephp-bindirectory under the current working directory / workspace). [2]
So to get an INSTALL_DIR env var, define it yourself from the output:
- id: setup-php-sdk
uses: php/setup-php-sdk@v0.12
with:
version: 8.4
arch: x64
ts: nts
- name: Export INSTALL_DIR
shell: pwsh
run: echo "INSTALL_DIR=${{ steps.setup-php-sdk.outputs.prefix }}" >> $env:GITHUB_ENVSources: [php/setup-php-sdk action.yml outputs] [1], [run.ps1 sets prefix=$pwd\php-bin] [2].
[1] (raw.githubusercontent.com)
[2] (raw.githubusercontent.com)
Citations:
- 1: https://raw.githubusercontent.com/php/setup-php-sdk/main/action.yml
- 2: https://raw.githubusercontent.com/php/setup-php-sdk/main/run.ps1
%INSTALL_DIR% は定義されていません。ビルドが失敗します。
configure コマンドで --with-prefix=%INSTALL_DIR% を使用していますが、この環境変数はワークフロー内で定義されていません。php/setup-php-sdk@v0.12 アクションは INSTALL_DIR をエクスポートせず、環境変数は空文字列に展開されてビルド設定が不正になります。
アクションの prefix 出力から INSTALL_DIR を設定する必要があります：
修正例
- name: Setup PHP SDK
id: setup
uses: php/setup-php-sdk@v0.12
with:
version: ${{ matrix.php }}
arch: ${{ matrix.arch }}
ts: ${{ matrix.ts }}
- name: Set INSTALL_DIR
run: echo INSTALL_DIR=${{ steps.setup.outputs.prefix }} >> %GITHUB_ENV%
- name: Enable MSVC Developer Command Prompt
uses: ilammy/msvc-dev-cmd@v1
with:
arch: ${{ matrix.arch }}
toolset: ${{ steps.setup.outputs.toolset }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/windows-build.yml around lines 66 - 68, `%INSTALL_DIR%`
が未定義で configure --with-prefix=%INSTALL_DIR% が失敗するので、php/setup-php-sdk@v0.12 の
prefix 出力を取り込んで環境変数に設定してください；具体的には Setup PHP SDK ステップに id: setup
を付け（既にある場合はそのまま）、その直後に "Set INSTALL_DIR" ステップを追加して steps.setup.outputs.prefix を
%GITHUB_ENV% に書き出して INSTALL_DIR をエクスポートし、これにより ext\mariadb_profiler の configure
ステップの --with-prefix=%INSTALL_DIR% が正しい値を参照するようにします（同じ領域で Enable MSVC Developer
Command Prompt に steps.setup.outputs.toolset を渡すことも維持してください）。
- Replace _dosmaperr() calls with profiler_dosmaperr(), a self-contained inline function. PHP's _dosmaperr is internal and not exported from the PHP DLL, causing LNK2019 unresolved external symbol errors on all PHP versions (8.0-8.4). - Remove PHP 7.4 from Windows build matrices in both windows-build.yml and release.yml. The php/setup-php-sdk action cannot find a suitable toolset for PHP 7.4 on windows-2022 runners. https://claude.ai/code/session_01KtCQUuRcfQxhRd8esNdkrM
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
112-114:⚠️ Potential issue | 🔴 Critical
%INSTALL_DIR%が未定義です（windows-build.ymlと同じ問題）。
windows-build.ymlと同様に、php/setup-php-sdkのprefix出力からINSTALL_DIRを設定するステップが必要です。修正案
+ - name: Set INSTALL_DIR + run: echo INSTALL_DIR=${{ steps.setup.outputs.prefix }}>> %GITHUB_ENV% + - name: configure working-directory: ext\mariadb_profiler run: configure --enable-mariadb_profiler --with-prefix=%INSTALL_DIR%🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 112 - 114, `configure` step currently references an undefined %INSTALL_DIR%; add a prior step that runs `php/setup-php-sdk` with an `id` (e.g. id: setup) and capture its `prefix` output, then set INSTALL_DIR from that output (use the action output `steps.<id>.outputs.prefix`) so the `configure --with-prefix=%INSTALL_DIR%` invocation receives the correct path; update the workflow to export or set the INSTALL_DIR environment variable for subsequent steps accordingly.
🧹 Nitpick comments (1)
.github/workflows/windows-build.yml (1)
73-83: ワークフローファイルの末尾に改行がありません。YAML ファイルの最終行の後に改行がないと、一部のツールやリンターで警告が出る場合があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/windows-build.yml around lines 73 - 83, The workflow file is missing a trailing newline at EOF; open the job step block named "Verify DLL exists" (the pwsh run block using $dll/Get-ChildItem) and add a single newline character at the end of the file so the YAML ends with a line break; save the file ensuring your editor writes the final newline (no other changes needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 112-114: `configure` step currently references an undefined
%INSTALL_DIR%; add a prior step that runs `php/setup-php-sdk` with an `id` (e.g.
id: setup) and capture its `prefix` output, then set INSTALL_DIR from that
output (use the action output `steps.<id>.outputs.prefix`) so the `configure
--with-prefix=%INSTALL_DIR%` invocation receives the correct path; update the
workflow to export or set the INSTALL_DIR environment variable for subsequent
steps accordingly.
---
Duplicate comments:
In @.github/workflows/windows-build.yml:
- Around line 65-67: The configure step fails because %INSTALL_DIR% is not
exported by php/setup-php-sdk@v0.12; set INSTALL_DIR from the SDK step output
instead. Update the "configure" step to consume the prefix output (the step that
produces the "prefix" output) by either adding env: INSTALL_DIR: ${{
steps.<prefix-step-id>.outputs.prefix }} or by interpolating ${{
steps.<prefix-step-id>.outputs.prefix }} directly into the run command, so the
run: configure --with-prefix uses the actual prefix value; target the step named
"configure" and the run line that currently references %INSTALL_DIR%.
---
Nitpick comments:
In @.github/workflows/windows-build.yml:
- Around line 73-83: The workflow file is missing a trailing newline at EOF;
open the job step block named "Verify DLL exists" (the pwsh run block using
$dll/Get-ChildItem) and add a single newline character at the end of the file so
the YAML ends with a line break; save the file ensuring your editor writes the
final newline (no other changes needed).
PHP 7.4 requires the VC15 toolset (MSVC 14.1x) which is not pre-installed on windows-2022 runners. Add a conditional step to install Microsoft.VisualStudio.Component.VC.v141.x86.x64 via vs_installer before running php/setup-php-sdk. Applies to both windows-build.yml (PR/push) and release.yml. https://claude.ai/code/session_01KtCQUuRcfQxhRd8esNdkrM
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
2 participants
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
This PR adds a GitHub Actions workflow to automatically build the MariaDB Profiler PHP extension on Windows across multiple PHP versions and configurations.
Key Changes
.github/workflows/windows-build.ymlworkflow that:ext/directoryImplementation Details
php/setup-php-sdkaction to configure the PHP build environmentilammy/msvc-dev-cmdto set up the MSVC compiler toolchainphpize,configure, andnmakephp_mariadb_profiler.dllwas created and reports its sizehttps://claude.ai/code/session_01KtCQUuRcfQxhRd8esNdkrM
Summary by CodeRabbit
Chores
Bug Fixes