search

php / php-tasks

Tasks that need doing. This is for php-src maintainers. The end-user bug tracker is at https://bugs.php.net/.
27 stars 6 forks source link

Accept --enable and --with configure flags interchangeably #2

Open nikic opened 4 years ago

nikic commented 4 years ago

./configure currently distinguishes between --enable-extname and --with-extname flags, where --enable is used for extensions without dependencies, and --with for those with dependencies.

In practice, the actual usage is very inconsistent, and there's plenty of edge cases (what about extensions that have a dependency, but the dependency is bundled?) Since the pkg-config migration in PHP 7.4, most --with options don't even accept a library directory anymore, so the difference between these flags eroded further.

In PHP 7.4, we tried to normalize the usage of --enable and --with somewhat, but this only went partway, and I think was a pretty bad idea in hindsight, because it just broke existing configure scripts without much benefit.

I think it would be better to give up on this distinction entirely, make all extensions enabled via --enable-extname (in docs), but accept --with-extname as an alias.

I'm not sure how this would be done technically, but the right starting point is probably the macro definitions starting at https://github.com/php/php-src/blob/c260613c6f166dc25d7c3a4ed090df5c08735491/build/php.m4#L684.

cmb69 commented 2 months ago

In my opinion, this makes sense, especially since this distinction never made sense on Windows, where only few options take a path anyway (one notable exception is --enable-object-out-dir ;). I don't know what would need to be changed for non Windows systems, but there the following patch should do to accept either variant.

Patch for Windows ````diff win32/build/confutils.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/win32/build/confutils.js b/win32/build/confutils.js index 5a28719f31..f6e230816b 100644 --- a/win32/build/confutils.js +++ b/win32/build/confutils.js @@ -224,13 +224,19 @@ function condense_path(path) function ConfigureArg(type, optname, helptext, defval) { var opptype = type == "enable" ? "disable" : "without"; + var alttype = type == "enable" ? "with" : "enable"; + var altopptype = type == "enable" ? "without" : "disable"; if (defval == "yes" || defval == "yes,shared") { this.arg = "--" + opptype + "-" + optname; this.imparg = "--" + type + "-" + optname; + this.altarg = "--" + altopptype + "-" + optname; + this.altimparg = "--" + alttype + "-" + optname; } else { this.arg = "--" + type + "-" + optname; this.imparg = "--" + opptype + "-" + optname; + this.altarg = "--" + alttype + "-" + optname; + this.altimparg = "--" + altopptype + "-" + optname; } this.optname = optname; @@ -348,7 +354,7 @@ function conf_process_args() configure_help_mode = true; break; } - if (arg == "--disable-all") { + if (arg == "--disable-all" || arg == "--without-all") { disable_all = true; continue; } @@ -365,7 +371,7 @@ function conf_process_args() // Find the arg found = false; for (j = 0; j < configure_args.length; j++) { - if (argname == configure_args[j].imparg || argname == configure_args[j].arg) { + if (argname == configure_args[j].imparg || argname == configure_args[j].arg || argname == configure_args[j].altimparg || argname == configure_args[j].altarg) { found = true; arg = configure_args[j]; @@ -381,7 +387,7 @@ function conf_process_args() } argval = analyzed[1]; - if (argname == arg.imparg) { + if (argname == arg.imparg || argname == arg.altimparg) { /* we matched the implicit, or default arg */ if (argval == null) { argval = arg.defval; ````
Girgias commented 2 months ago

Probably @petk would know about autotools?