Skip to content
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

Improve autotools AC_ARG_ENABLE usage #193

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve autotools AC_ARG_ENABLE usage #193

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 10, 2016

This config.m4 falls into an autotools issue of present/not present vs enabled/disabled described at https://autotools.io/autoconf/arguments.html

Previously, the macros appeared to have the use of "if enabled ; then x ; else y"

This is not how autoconf interprets the AC_ARG_ENABLE macro.
Instead it is "if present; then x; else y"

Now, it uses the result of the AC_ARG_ENABLE to test and act in a true IF.

The apparent desired behavior is preserved.

Brian Evans added 2 commits August 10, 2016 14:13
Previously, the macros appeared to have the use of "if enabled ; then x ; else y"

This is not how autoconf interprets the AC_ARG_ENABLE macro.
Instead it is "if present; then x; else y"

Now, it uses the result of the AC_ARG_ENABLE to test and act in a true IF
@krakjoe
Copy link
Owner

krakjoe commented Sep 29, 2016

@remicollet do you have a build matrix with lots of different configurations by chance ?

@remicollet
Copy link
Collaborator

remicollet commented Sep 30, 2016

I can test on RHEL-6 (the older version for which I build PHP 7) and Fedora 25 (the more recent version)

So autoconf 2.63 - 2.69 => OK.

But perhaps we should prefer the PHP macros (PHP_ARG_ENABLE, PHP_ARG_WITH) instead of AC ones

@krakjoe
Copy link
Owner

krakjoe commented Sep 30, 2016

I feel unsure about why it used AC ones in the first place ... I'm scared of autotools ...

Anyone else got some input ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants