Add Windows support to mariadb_profiler extension#13
Merged
Conversation
- Add config.w32 for Windows PHP extension builds - Add Windows platform compatibility layer to compat header: flock() via LockFileEx/UnlockFileEx, localtime_r via localtime_s, POSIX I/O mapping (_open/_read/_close), mkdir without mode param - Update profiler_log.c, profiler_job.c, mariadb_profiler.c to use portable macros for cross-platform builds - Add release.yml workflow with workflow_dispatch that: - Builds Linux .so for PHP 7.4-8.4 - Builds Windows .dll (NTS/TS, x64) for PHP 7.4-8.4 - Creates GitHub Release with all artifacts https://claude.ai/code/session_01HrzQUREAdAn7ajfqmTL8ij
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughWindows向けのI/O互換レイヤとパス処理を追加し、Linux/Windows向けビルドを行うGitHub Actionsリリースワークフローを導入、各プラットフォーム向けビルド成果物を集約してGitHub Releaseを作成します。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User\n(workflow_dispatch)"
participant GH_Actions as "GitHub Actions"
participant Build_Linux as "build-linux\n(Job)"
participant Build_Windows as "build-windows\n(Job)"
participant Artifacts as "Artifact Storage\n(actions/artifacts)"
participant Release_Job as "release\n(Job)"
participant GH_Release as "GitHub Release"
User->>GH_Actions: トリガー(tag)
GH_Actions->>Build_Linux: 起動（並列）
GH_Actions->>Build_Windows: 起動（並列）
Build_Linux->>Artifacts: .so をアップロード
Build_Windows->>Artifacts: .dll をアップロード
GH_Actions->>Release_Job: ジョブ完了後起動
Release_Job->>Artifacts: 全アーティファクトをダウンロード
Release_Job->>GH_Release: release/ 配下を使って Release を作成・公開
GH_Release-->>User: リリース公開通知
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 分 Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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: 2
🤖 Fix all issues with AI agents
In @.github/workflows/release.yml:
- Around line 95-101: Update the GitHub Actions step that uses the
php/setup-php-sdk action by changing its version pin from
php/setup-php-sdk@v0.10 to php/setup-php-sdk@v0.12; locate the workflow step
where "uses: php/setup-php-sdk@v0.10" is declared and replace the tag to "v0.12"
so the release.yml uses the latest action release.
In `@ext/mariadb_profiler/php_mariadb_profiler_compat.h`:
- Around line 261-272: The build error arises because ssize_t is used to typedef
profiler_ssize_t in the Unix branch (around profiler_ssize_t) without including
the header that defines it; update the file to either include the appropriate
POSIX header (e.g., add an `#include` of <unistd.h> or another platform-correct
header) guarded with proper `#ifdefs` so it only applies on non-Windows targets,
or provide a conditional fallback typedef for profiler_ssize_t when ssize_t is
unavailable; ensure the change touches the Unix/Linux/macOS branch where
profiler_open/read/close and PROFILER_MKDIR are defined so static analysis stops
reporting "unknown type name 'ssize_t'".
🧹 Nitpick comments (3)
ext/mariadb_profiler/php_mariadb_profiler_compat.h (1)
242-258:profiler_flockはLOCK_NB（非ブロッキング）フラグに対応していない現在の実装では
operation == LOCK_EXとoperation == LOCK_UNのみを比較していますが、POSIX のflock()はLOCK_SH | LOCK_NBやLOCK_EX | LOCK_NBのようにビットOR でフラグを組み合わせることが一般的です。現在の使用箇所ではLOCK_NBは使われていませんが、将来的な互換性のためにビットマスク比較にすることを検討してください。提案: ビットマスクベースの実装
static inline int profiler_flock(int fd, int operation) { HANDLE h = (HANDLE)_get_osfhandle(fd); DWORD flags; OVERLAPPED ov = {0}; if (h == INVALID_HANDLE_VALUE) { return -1; } - if (operation == LOCK_UN) { + if (operation & LOCK_UN) { return UnlockFileEx(h, 0, MAXDWORD, MAXDWORD, &ov) ? 0 : -1; } - flags = (operation == LOCK_EX) ? LOCKFILE_EXCLUSIVE_LOCK : 0; + flags = 0; + if (operation & LOCK_EX) flags |= LOCKFILE_EXCLUSIVE_LOCK; + if (operation & LOCK_NB) flags |= LOCKFILE_FAIL_IMMEDIATELY; return LockFileEx(h, flags, 0, MAXDWORD, MAXDWORD, &ov) ? 0 : -1; }追加で
LOCK_NBの定義も必要です：# ifndef LOCK_UN # define LOCK_UN 8 # endif +# ifndef LOCK_NB +# define LOCK_NB 4 +# endif.github/workflows/release.yml (2)
92-145: Windows ビルドにロード検証ステップがないLinux ビルド（Line 44-47）には
php -d "extension=..." -m | grep mariadb_profilerによるロード検証がありますが、Windows ビルドには同等のステップがありません。Windows 特有のリンクエラー（欠落した DLL 依存関係など）をCIで早期に検出するために、ビルド後に DLL のロード検証を追加することを推奨します。提案: nmake 後に検証ステップを追加
- name: nmake working-directory: ext\mariadb_profiler run: nmake + - name: Verify extension is loadable + working-directory: ext\mariadb_profiler + shell: cmd + run: php -d "extension=%cd%\${{ matrix.arch }}\Release${{ matrix.ts == 'ts' && '_TS' || '' }}\php_mariadb_profiler.dll" -m | findstr mariadb_profiler + - name: Package artifact注意: DLL のパスは PHP SDK のビルド出力構造に合わせて調整が必要です。
Get-ChildItemでのDLL検索を検証ステップにも応用できます。
172-180: リリース作成前の Git タグ確認がない
softprops/action-gh-release@v2は指定されたtag_nameが存在しない場合、ワークフローがディスパッチされたコミットにタグを自動作成します。意図的な場合は問題ありませんが、誤ったタグ名の入力ミスによる不正なリリースを防ぐために、リリース前にタグの存在確認またはタグ作成のステップを追加することを検討してください。提案: タグの作成/確認ステップを追加
- name: Collect release assets run: | mkdir -p release find artifacts -type f \( -name "*.so" -o -name "*.dll" \) -exec cp {} release/ \; echo "=== Release assets ===" ls -lh release/ + - name: Create and push tag + run: | + git tag "${{ github.event.inputs.tag }}" || true + git push origin "${{ github.event.inputs.tag }}" || true + - name: Create GitHub Release
- Add #include <unistd.h> in the Unix branch of the compat header so ssize_t is defined before the profiler_ssize_t typedef - Update php/setup-php-sdk action from v0.10 to v0.12 https://claude.ai/code/session_01HrzQUREAdAn7ajfqmTL8ij
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ext/mariadb_profiler/php_mariadb_profiler_compat.h`:
- Around line 242-258: The profiler_flock implementation currently compares
operation with LOCK_UN and LOCK_EX using == which fails when multiple POSIX
flock flags (e.g., LOCK_EX|LOCK_NB) are combined; update profiler_flock to test
flags with bitwise operations (use (operation & LOCK_UN) and (operation &
LOCK_EX)) and preserve non-blocking semantics by accepting LOCK_NB; also add a
compatibility macro to define LOCK_NB when missing (e.g., `#ifndef` LOCK_NB /
`#define` LOCK_NB 4 / `#endif`). Ensure UnlockFileEx is used when (operation &
LOCK_UN) and LockFileEx is called with flags = ((operation & LOCK_EX) ?
LOCKFILE_EXCLUSIVE_LOCK : 0) and respect LOCK_NB where applicable when mapping
to Windows APIs.
🧹 Nitpick comments (3)
.github/workflows/release.yml (2)
109-119: Windows ビルドで拡張機能のロード検証が欠落しています。Linux ビルド（Line 44-47）では
php -d "extension=..." -m | grep mariadb_profilerでロード可能かを検証していますが、Windows ビルドには同等のステップがありません。ビルドは成功しても DLL がロードできないケースを早期に検出するため、nmakeの後に検証ステップの追加を検討してください。提案: Windows でのロード検証ステップ追加
- name: nmake working-directory: ext\mariadb_profiler run: nmake + - name: Verify extension is loadable + working-directory: ext\mariadb_profiler + shell: cmd + run: php -d "extension=%CD%\x64\Release_TS\php_mariadb_profiler.dll" -m | findstr mariadb_profilerただし DLL パスは TS/NTS やビルド構成で変わるため、PowerShell で
Get-ChildItemを使って動的に探す方が堅牢です。
150-180: リリースジョブは概ね良好ですが、タグの事前検証がありません。
github.event.inputs.tagはユーザーの自由入力なので、存在しないタグや不正な形式の文字列でもリリースが作成されます。softprops/action-gh-releaseはタグが存在しない場合に自動作成しますが、意図しないリリースを防ぐために入力値のバリデーション（例:vプレフィックス + semver パターンチェック）を追加すると安全です。ext/mariadb_profiler/php_mariadb_profiler_compat.h (1)
229-229:localtime_rとflockのグローバルマクロ定義に注意。
#define localtime_rおよび#define flockはファイル全体（このヘッダをインクルードする全ファイル）に影響するグローバルマクロです。他のライブラリヘッダで同名のシンボルが使われている場合に衝突する可能性があります。現時点で問題が発生していなければ許容範囲ですが、将来的に問題が生じた場合はprofiler_localtime_r/profiler_flockを直接使用する形に切り替えることを検討してください。Also applies to: 259-259
There was a problem hiding this comment.
Pull request overview
This PR adds Windows support to the mariadb_profiler PHP extension by introducing a Windows compatibility layer for POSIX APIs, updating the extension code to use the abstraction, and adding Windows build + release automation.
Changes:
- Added a Windows I/O compatibility layer (
flock,localtime_r,open/read/close,mkdir,ssize_t). - Updated extension sources to compile on Windows (conditional POSIX includes, portable types/APIs, path-separator handling in log-dir creation).
- Added Windows build configuration (
config.w32) and a release workflow producing Linux.soand Windows.dllartifacts.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ext/mariadb_profiler/profiler_log.c | Guards POSIX-only headers on non-Windows builds. |
| ext/mariadb_profiler/profiler_job.c | Switches to compatibility I/O types/functions and guards POSIX headers. |
| ext/mariadb_profiler/php_mariadb_profiler_compat.h | Introduces Windows platform abstractions for POSIX APIs used by the extension. |
| ext/mariadb_profiler/mariadb_profiler.c | Uses cross-platform mkdir macro and adds Windows path separator normalization for recursive log dir creation. |
| ext/mariadb_profiler/config.w32 | Adds Windows build script for PHP SDK builds. |
| .github/workflows/release.yml | Adds manual release workflow building and packaging Linux/Windows artifacts across PHP versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ping - profiler_flock: use bitwise & to test LOCK_UN/LOCK_EX flags instead of ==, so combined flags like LOCK_EX|LOCK_NB work correctly; add LOCK_NB definition and map it to LOCKFILE_FAIL_IMMEDIATELY - profiler_ensure_log_dir: skip drive-letter prefix (C:\) and UNC prefix (\\server\share\) in the recursive mkdir loop so Windows absolute paths don't attempt to stat/create the drive root or UNC server component https://claude.ai/code/session_01HrzQUREAdAn7ajfqmTL8ij
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ext/mariadb_profiler/mariadb_profiler.c (1)
129-150: Windows のelseブランチで相対パスの先頭文字がスキップされるLine 145-147 の
elseブランチは、ドライブレター (C:\) でも UNC (\\server\share\) でもない場合にp++で 1 文字スキップします。これは\foo\barのような単一バックスラッシュで始まる絶対パスには正しく動作しますが（Unix のp = tmp + 1と同等）、相対パス（例：logs\profiler）の場合はパスコンポーネントの先頭文字をスキップすることになります。ただし、ループ内ではセパレータ文字のみを探して中間ディレクトリを作成するため、実際にはディレクトリ作成の失敗にはつながりません。
log_dirの INI デフォルト値が絶対パスであることを考えると、実用上の問題は低いですが、コメントで意図を明示しておくと保守性が向上します。💡 コメント追加の提案
} else { - p++; + /* Relative or single-backslash absolute path: skip first char + (same as Unix: avoids creating empty path from leading sep) */ + p++; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ext/mariadb_profiler/mariadb_profiler.c` around lines 129 - 150, The Windows `else` branch currently does `p++` which unconditionally skips the first character of relative paths (variables p, tmp, sep); change that branch to avoid dropping the first character for relative paths by setting p = tmp (i.e., do not increment) unless the first character is the path separator (check `if (p[0] == sep) p++;`), and add a short comment by the `else`/fallback block explaining the intent and why we only skip a leading separator to match the non-Windows handling and preserve relative path components.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ext/mariadb_profiler/php_mariadb_profiler_compat.h`:
- Around line 245-266: profiler_flock currently returns -1 on
LockFileEx/UnlockFileEx failure but doesn't set errno; update profiler_flock to
capture the Windows error with DWORD lasterr = GetLastError() when
LockFileEx/UnlockFileEx fails and translate it into errno (use
_dosmaperr(lasterr) if available), and explicitly set errno = EWOULDBLOCK when
operation includes LOCK_NB and lasterr indicates a lock contention (e.g.
ERROR_LOCK_VIOLATION) so callers can rely on errno/strerror; make these changes
in the profiler_flock function around the LockFileEx/UnlockFileEx failure paths
(referencing profiler_flock, LockFileEx, UnlockFileEx, LOCK_NB, LOCK_UN).
---
Duplicate comments:
In `@ext/mariadb_profiler/php_mariadb_profiler_compat.h`:
- Around line 269-281: Clang reports a false-positive "'unistd.h' file not
found" for the `#include` <unistd.h> added to define profiler_ssize_t; to fix,
guard the include with a portability check (e.g. `#ifdef` HAVE_UNISTD_H / `#elif`
defined(_WIN32) ... `#else`) or include a generated config header that defines
HAVE_UNISTD_H, and keep a fallback typedef for profiler_ssize_t when unistd.h is
unavailable; update the non-Windows branch around profiler_ssize_t and
PROFILER_MKDIR so builds without system include paths still typedef
profiler_ssize_t and define PROFILER_MKDIR, ensuring the compatibility code
(profiler_ssize_t, PROFILER_MKDIR, and the `#include` <unistd.h>) is conditionally
compiled based on availability rather than unguarded include.
---
Nitpick comments:
In `@ext/mariadb_profiler/mariadb_profiler.c`:
- Around line 129-150: The Windows `else` branch currently does `p++` which
unconditionally skips the first character of relative paths (variables p, tmp,
sep); change that branch to avoid dropping the first character for relative
paths by setting p = tmp (i.e., do not increment) unless the first character is
the path separator (check `if (p[0] == sep) p++;`), and add a short comment by
the `else`/fallback block explaining the intent and why we only skip a leading
separator to match the non-Windows handling and preserve relative path
components.
Use _dosmaperr(GetLastError()) to translate Windows errors into errno on LockFileEx/UnlockFileEx failure. Set errno = EWOULDBLOCK explicitly when LOCK_NB is requested and ERROR_LOCK_VIOLATION indicates contention. Set errno = EBADF for invalid handle. https://claude.ai/code/session_01HrzQUREAdAn7ajfqmTL8ij
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ext/mariadb_profiler/php_mariadb_profiler_compat.h (1)
245-282:profiler_flockの実装は堅牢です —_dosmaperrの使用について一点だけ注記。
errnoの設定、LOCK_NB+ERROR_LOCK_VIOLATION→EWOULDBLOCKのマッピング、EBADFのハンドリングなど、過去のレビュー指摘が全て反映されています。
_dosmaperr()は MSVC CRT の非公開（undocumented）内部関数です。Git for Windows や CPython など広く使われていますが、将来的に削除される可能性がゼロではありません。現時点では問題ありませんが、もし懸念がある場合はGetLastError()の値を手動でerrnoにマッピングする方法も選択肢です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ext/mariadb_profiler/php_mariadb_profiler_compat.h` around lines 245 - 282, The code uses the undocumented MSVC CRT helper _dosmaperr inside profiler_flock to map Win32 errors to errno; to avoid relying on this internal symbol replace the _dosmaperr(GetLastError()) calls with an explicit mapping: call DWORD lasterr = GetLastError(), then switch/map common Win32 error codes (e.g., ERROR_INVALID_HANDLE -> EBADF, ERROR_SHARING_VIOLATION / ERROR_LOCK_VIOLATION -> EACCES or EWOULDBLOCK when combined with LOCK_NB, ERROR_ACCESS_DENIED -> EACCES, ERROR_FILE_NOT_FOUND -> ENOENT, etc.) and set errno accordingly before returning -1; keep the existing special-case for (operation & LOCK_NB) && lasterr == ERROR_LOCK_VIOLATION to set errno = EWOULDBLOCK, and update both UnlockFileEx and LockFileEx error paths (the two places currently calling _dosmaperr) to use this manual mapping instead of _dosmaperr.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ext/mariadb_profiler/php_mariadb_profiler_compat.h`:
- Around line 285-297: Unix branch already fixed: keep the `#include` <unistd.h>
inside the `#else` block, retain the typedef ssize_t profiler_ssize_t and the
macro definitions profiler_open/profiler_read/profiler_close and
PROFILER_MKDIR(path, mode) as shown; do not remove these changes (they resolve
the previous ssize_t issue), and optionally add a brief comment above the
`#include` noting that "'unistd.h' file not found" warnings from Clang are a
cross-compile/static-analysis false positive so future reviewers don't revert
this fix.
---
Nitpick comments:
In `@ext/mariadb_profiler/php_mariadb_profiler_compat.h`:
- Around line 245-282: The code uses the undocumented MSVC CRT helper _dosmaperr
inside profiler_flock to map Win32 errors to errno; to avoid relying on this
internal symbol replace the _dosmaperr(GetLastError()) calls with an explicit
mapping: call DWORD lasterr = GetLastError(), then switch/map common Win32 error
codes (e.g., ERROR_INVALID_HANDLE -> EBADF, ERROR_SHARING_VIOLATION /
ERROR_LOCK_VIOLATION -> EACCES or EWOULDBLOCK when combined with LOCK_NB,
ERROR_ACCESS_DENIED -> EACCES, ERROR_FILE_NOT_FOUND -> ENOENT, etc.) and set
errno accordingly before returning -1; keep the existing special-case for
(operation & LOCK_NB) && lasterr == ERROR_LOCK_VIOLATION to set errno =
EWOULDBLOCK, and update both UnlockFileEx and LockFileEx error paths (the two
places currently calling _dosmaperr) to use this manual mapping instead of
_dosmaperr.
This was referenced Feb 17, 2026
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
This PR adds comprehensive Windows support to the mariadb_profiler PHP extension by implementing platform abstraction for POSIX APIs and adding Windows build configuration.
Key Changes
Platform I/O Compatibility Layer (
php_mariadb_profiler_compat.h):profiler_flock()using Win32LockFileEx/UnlockFileExAPIsprofiler_localtime_r()using Windowslocaltime_s()profiler_open(),profiler_read(),profiler_close()mapping to Windows_open(),_read(),_close()PROFILER_MKDIR()macro handling Windows_mkdir()signature differencesprofiler_ssize_ttype for cross-platform compatibilitySource Code Updates:
mariadb_profiler.cto usePROFILER_MKDIR()macro and handle Windows path separators (\vs/)profiler_job.cto use platform-abstracted I/O functions and conditional includes for POSIX headersprofiler_log.cto conditionally include POSIX-specific headers only on non-Windows platformsWindows Build Configuration (
config.w32):CI/CD (
.github/workflows/release.yml):.sofiles.dllfilesImplementation Details
The Windows compatibility layer uses conditional compilation (
#ifdef PHP_WIN32) to provide equivalent functionality:LockFileEx/UnlockFileExhttps://claude.ai/code/session_01HrzQUREAdAn7ajfqmTL8ij
Summary by CodeRabbit
新機能
修正