8.2.28: Add sendmail and switch build to wasixcc#14
8.2.28: Add sendmail and switch build to wasixcc#14zebreus wants to merge 10 commits into8.2.28-wasixfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request migrates PHP 8.2.28 from a custom clang-20 based build toolchain to the wasixcc toolchain wrapper, and replaces the custom Rust-based sendmail implementation with the standard sendmail binary approach. This change aligns with PR #11 but is specifically for PHP 8.2.28.
Changes:
- Switched from manual clang-20 toolchain configuration to wasixcc/wasixcc++ wrapper tools
- Replaced custom wasix_sendmail Rust library with standard sendmail binary via package dependency
- Updated Wasmer volume mounting from
--mapdirto--volumesyntax - Consolidated and streamlined CI/CD workflow using matrix strategy
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wasmer-64.toml | Added wasmer/bash and sendmail/sendmail package dependencies |
| wasmer-32.toml | Added wasmer/bash and sendmail/sendmail package dependencies |
| wasix-test-server.sh | Migrated from --mapdir to --volume for directory mounting |
| wasix-execute-eh.sh | Migrated from --mapdir to --volume and removed SMTP configuration parameters |
| wasix-configure-eh.sh | Switched from clang-20 toolchain to wasixcc, removed SYSROOT dependency, added PROG_SENDMAIL configuration |
| wasix-configure.sh | Deleted - consolidated into wasixcc-based configuration |
| wasix-execute.sh | Deleted - no longer needed |
| wasix-build.sh | Deleted - build handled by wasix-build-eh.sh |
| main/main.c | Removed sendmail_username and sendmail_password INI entries |
| ext/standard/mail.c | Removed custom wasix_sendmail Rust implementation and SMTP credential handling |
| ext/standard/config.m4 | Removed WASIX_SENDMAIL_LIBS linker configuration |
| build/php.m4 | Changed C++ standard library from libstdc++ to libc++ for WASIX compatibility |
| .github/workflows/wasix-pr.yaml | Switched to wasixcc action, removed secrets, added matrix strategy, simplified workflow |
| .github/workflows/release.yaml | Removed sendmail credential secrets |
| .github/workflows/release-dev.yaml | Removed sendmail credential secrets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SENDMAIL_DEFAULT_USERNAME: ${{ secrets.SENDMAIL_DEFAULT_USERNAME }} | ||
| SENDMAIL_DEFAULT_PASSWORD: ${{ secrets.SENDMAIL_DEFAULT_PASSWORD }} | ||
| run: | | ||
| export SYSROOT=$(pwd)/sysroot/wasix-sysroot-eh/sysroot |
There was a problem hiding this comment.
The SYSROOT environment variable is being set to a path that no longer exists. Since the wasixcc toolchain is now being used (installed via the wasix-org/wasixcc action at line 44), the sysroot is managed internally by wasixcc and this manual SYSROOT export is no longer needed. The wasix-configure-eh.sh script has been updated to use wasixcc which doesn't reference the SYSROOT environment variable. This line should be removed to avoid confusion.
| export SYSROOT=$(pwd)/sysroot/wasix-sysroot-eh/sysroot |
| --volume ../php-wasix-deps/icu:/icu \ | ||
| --env SSL_CERT_DIR=/etc/ssl/certs \ | ||
| --env OPENSSL_CONF=/etc/ssl/openssl.cnf \ | ||
| --use amin/bash \ |
There was a problem hiding this comment.
The script uses --use amin/bash but the wasmer.toml files (wasmer-32.toml and wasmer-64.toml) declare a dependency on "wasmer/bash" = "=1.0.25". This inconsistency could cause runtime issues if the bash package locations don't match. Consider updating this to use --use wasmer/bash to match the dependency declaration, or verify that amin/bash is the correct package to use.
| --use amin/bash \ | |
| --use wasmer/bash \ |
wasix-configure-eh.sh
Outdated
| --with-pgsql=$PHP_WASIX_DEPS/pgsql-eh --with-pdo-pgsql=$PHP_WASIX_DEPS/pgsql-eh --enable-intl \ | ||
| --with-pdo-sqlite --enable-ftp --enable-igbinary --with-imagick \ | ||
| --with-pgsql="$PHP_WASIX_DEPS"/pgsql-eh --with-pdo-pgsql="$PHP_WASIX_DEPS"/pgsql-eh --enable-intl \ | ||
| --with-pdo-sqlite --enable-ftp --enable-igbinary --with-imagick --with-iconv="$PHP_WASIX_DEPS"/iconv-eh \ |
There was a problem hiding this comment.
There are two spaces between --with-imagick and --with-iconv which appears to be a formatting inconsistency. Consider using a single space for consistency with the rest of the configuration line.
| --with-pdo-sqlite --enable-ftp --enable-igbinary --with-imagick --with-iconv="$PHP_WASIX_DEPS"/iconv-eh \ | |
| --with-pdo-sqlite --enable-ftp --enable-igbinary --with-imagick --with-iconv="$PHP_WASIX_DEPS"/iconv-eh \ |
947f2fe to
c028b66
Compare
The same as #11 and #16 but for php 8.2.28