Skip to content

fix imap c-client headers leaking into configure - #1237

Open
edram wants to merge 1 commit into
crazywhalecc:v3from
edram:fix/imap-c-client-header-layout
Open

fix imap c-client headers leaking into configure#1237
edram wants to merge 1 commit into
crazywhalecc:v3from
edram:fix/imap-c-client-header-layout

Conversation

@edram

@edram edram commented Aug 27, 2026

Copy link
Copy Markdown

What does this PR do?

c-client currently installs its private headers, including unix.h, into the global include directory. PHP's configure check then detects that private header and defines HAVE_UNIX_H, causing installed PHP development headers to include a file that is not shipped with the PHP SDK package. As a result, downstream extensions fail to compile when they include php.h.

This keeps c-client headers under include/c-client, the layout already expected by ext-imap, so they are no longer visible to unrelated PHP configure checks.

Verification

Checklist before merging

  • If you modified *.php or *.yml, run them locally to ensure your changes are valid:
    • composer cs-fix
    • composer analyse
    • composer test
    • bin/spc dev:lint-config

@edram @codex
fix(imap): isolate c-client headers from global include path
8970af6 
c-client's private unix.h leaked into PHP configure checks,
causing installed development headers to require an unshipped file.

Keep c-client headers in the path already supported by ext-imap.

Co-Authored-By: Codex GPT-5 <noreply@openai.com>
henderkes
henderkes reviewed Aug 27, 2026
View reviewed changes

@henderkes henderkes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the extension need to be pointed at that dir then?

@edram

edram commented Aug 28, 2026

Copy link
Copy Markdown
Author

Doesn't the extension need to be pointed at that dir then?

Good question. The extension does need to include that directory, but no
additional configure argument is required because --with-imap=DIR
expects the c-client installation prefix, not the header directory itself.

StaticPHP already passes:

--with-imap=<buildroot>

The upstream pecl-mail-imap config.m4 explicitly probes
$PHP_IMAP/include/c-client via IMAP_INC_CHK(/include/c-client).
Once found, it assigns that path to IMAP_INC_DIR and adds it with
PHP_ADD_INCLUDE($IMAP_INC_DIR):

https://github.com/php/pecl-mail-imap/blob/1.0.3/config.m4#L126-L139

I also verified that the generated PHP Makefile contains:

-I<buildroot>/include/c-client

and the resulting PHP binary builds and loads the imap extension
successfully. Passing <buildroot>/include/c-client directly to
--with-imap would be incorrect because the same prefix is also used to
locate <buildroot>/lib/libc-client.a.

@crazywhalecc crazywhalecc added the need-test This PR has not been tested yet, cannot merge now label Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026 •
edited
Loading

Copy link
Copy Markdown

StaticPHP Test Bot

Detected: Extensions: none | Libraries: imap | Targets: none
Active labels: test/php-83
Available labels: need-test (gate), test/linux test/windows test/macos (platform), test/tier2 (extra arch), test/php-83 test/php-84 (PHP version), sapi/frankenphp (extra FrankenPHP SAPI build)
Config: Linux x86_64 + Windows x86_64 + macOS arm64 | PHP 8.3, PHP 8.5 NTS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henderkes henderkes henderkes left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

need-test This PR has not been tested yet, cannot merge now test/php-83

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@edram @henderkes @crazywhalecc