Skip to content

Autotools: Move Apache warning to SAPI's config.m4 #14872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

petk
Copy link
Member

@petk petk commented Jul 8, 2024

This uses AC_CONFIG_COMMANDS macro to conditionally output the warning message at the end of configure phase if non-threaded Apache installation has been found and PHP is built without --enable-zts.

Variables set in the 2nd argument (init-cmds) are for the command invocation scope as the first argument doesn't have any knowledge of configure variables as described in the Autoconf docs.

This is moved as it is related only to apache2handler SAPI and also now warning is displayed a bit nicer at the end of the configure phase after files are generated. This also enables outputting warning when using config.status script.

Before:
Generating files
configure: creating build directories
configure: creating Makefile
configure: patching main/php_config.h.in
configure: creating ./config.status
creating main/internal_functions.c
creating main/internal_functions_cli.c
+--------------------------------------------------------------------+
|                        *** WARNING ***                             |
|                                                                    |
| You have built PHP for Apache's current non-threaded MPM.          |
| If you change Apache to use a threaded MPM you must reconfigure    |
| PHP with --enable-zts                                              |
config.status: creating main/build-defs.h
config.status: creating scripts/phpize
config.status: creating scripts/man1/phpize.1
config.status: creating scripts/php-config
config.status: creating scripts/man1/php-config.1
config.status: creating sapi/cli/php.1
config.status: creating sapi/phpdbg/phpdbg.1
config.status: creating sapi/cgi/php-cgi.1
config.status: creating ext/phar/phar.1
config.status: creating ext/phar/phar.phar.1
config.status: creating main/php_config.h
config.status: executing default commands

+--------------------------------------------------------------------+
| License:                                                           |
| This software is subject to the PHP License, available in this     |
| distribution in the file LICENSE. By continuing this installation  |
| process, you are bound by the terms of this license agreement.     |
| If you do not agree with the terms of this license, you must abort |
| the installation process at this point.                            |
+--------------------------------------------------------------------+

Thank you for using PHP.

After:
Generating files
configure: creating build directories
configure: creating Makefile
configure: patching main/php_config.h.in
configure: creating ./config.status
creating main/internal_functions.c
creating main/internal_functions_cli.c
config.status: creating main/build-defs.h
config.status: creating scripts/phpize
config.status: creating scripts/man1/phpize.1
config.status: creating scripts/php-config
config.status: creating scripts/man1/php-config.1
config.status: creating sapi/cli/php.1
config.status: creating sapi/phpdbg/phpdbg.1
config.status: creating sapi/cgi/php-cgi.1
config.status: creating ext/phar/phar.1
config.status: creating ext/phar/phar.phar.1
config.status: creating main/php_config.h
config.status: main/php_config.h is unchanged
config.status: executing apache2handler commands
config.status: WARNING:
+--------------------------------------------------------------------+
|                        *** WARNING ***                             |
|                                                                    |
| You have built PHP for Apache's current non-threaded MPM.          |
| If you change Apache to use a threaded MPM you must reconfigure    |
| PHP with --enable-zts                                              |
+--------------------------------------------------------------------+
  
config.status: executing default commands

+--------------------------------------------------------------------+
| License:                                                           |
| This software is subject to the PHP License, available in this     |
| distribution in the file LICENSE. By continuing this installation  |
| process, you are bound by the terms of this license agreement.     |
| If you do not agree with the terms of this license, you must abort |
| the installation process at this point.                            |
+--------------------------------------------------------------------+

Thank you for using PHP.

This uses AC_CONFIG_COMMANDS macro to conditionally output the warning
message at the end of configure phase if non-threaded Apache
installation has been found and PHP is built without --enable-zts.

Variables set in the 2nd argument (init-cmds) are for the command
invocation scope as the first argument doesn't have any knowledge of
configure variables as described in the Autoconf docs.

This is moved as it is related only to apache2handler SAPI and also
now warning is displayed a bit nicer at the end of the configure phase
after files are generated. This also enables outputting warning when
using config.status script.
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

This makes sense to me

@petk petk merged commit 7fda122 into php:master Jul 8, 2024
11 checks passed
@petk petk deleted the patch-apache2handler-warning branch July 8, 2024 15:17
@nielsdos
Copy link
Member

@petk This breaks my Apache 2 build, configure now fails and I get the following message at the bottom:

./config.status: line 536: syntax error near unexpected token `('
./config.status: line 536: `APACHE_THREADED_MPM=  threaded:     yes (fixed thread count); enable_zts=yes'

@nielsdos
Copy link
Member

Note that Apache 2 SAPI is not tested in CI, so we should be extra careful when applying patches here.

petk added a commit to petk/php-src that referenced this pull request Jul 11, 2024
The init-cmds argument is appended to the config.status script with cat
command and variables $var are replaced during the cat step to their
values, so quoting these values fixes the syntax errors.

Fixes report in phpGH-14872
@petk
Copy link
Member Author

petk commented Jul 11, 2024

@nielsdos, I see. Yes, this init-cmds argument is tricky because its content is inserted to another script file via cat command. I was wondering how to simplify that already but didn't get to that yet. Fixed in GH-14929. Thanks for catching it.

@nielsdos
Copy link
Member

Thanks for fixing it fast!

petk added a commit that referenced this pull request Jul 12, 2024
The init-cmds argument is appended to the config.status script with cat
command and variables $var are replaced during the cat step to their
values, so quoting these values fixes the syntax errors.

* Simplify threaded Apache build detection

Instead of checking for entire "grepped" string, this only checks for
yes|no values instead.

* Redirect the standard output and standard error

The "grep -q" is not portable according to docs so this redirects the
output and checks the exit status.

Fixes report in GH-14872
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants