Allow loading extensions by name #1741

flaupretre · 2016-01-28T20:07:37Z

Status : RFC is approved for inclusion in 7.2

This PR provides a portable way to configure the list of PHP extensions to load.

Today, 'extension=' lines in php.ini must contain the extension's file name. Unfortunately, this file name depends on the platform PHP is running on. A 'php_' prefix must even be added on Windows.

While seasoned PHP administrators are used to this mechanism, this is a problem when writing documentation for beginners, or when writing platform-agnostic scripts. A typical example is the coexistence of a Windows development environment and a Linux production. In such cases, it is impossible to write a single configuration file that will work in both environments, forcing developers to manually maintain two separate versions of the file.

This PR allows to specify extensions (PHP and Zend) by their extension name.

Example :

extension=bz2
zend_extension=xdebug

'extension=bz2', for example, will cause PHP to load a file named 'php_bz2.dll' on Windows, and a file named 'bz2.so' on LInux.

Note that filenames are still accepted and handled as before. So, this extension does not cause any BC break.

Example php.ini files are modified because loading extensions by name becomes the recommended way of configuring additional extensions to load.

Cases where the extension name is accepted :

'extension=' INI setting
'zend_extension=' INI setting
argument to the dl() function

Cases where the extension name cannot be used :

The '-z' command line option still requires an absolute file path.
When specifying an absolute path, a filename must be provided. A path like '/path/to/extensions/bz2' is invalid

References:

An article I wrote on the same subject

nikic · 2016-01-28T21:53:16Z

php.ini-development

 ;
 ; ... or under UNIX:
 ;
-;   extension=msql.so


The Windows/UNIX distinction in this comment is no longer necessary.

flaupretre · 2016-01-29T08:20:04Z

@nikic @tpunt Right. I just improved the comments in the php.ini example files.

flaupretre · 2016-05-10T23:08:30Z

RFC is under discussion: https://wiki.php.net/rfc/load-ext-by-name

krakjoe · 2017-01-02T09:24:26Z

@flaupretre fix conflicts please :)

Please push forward with this RFC, alternatively, if you consider this work abandoned, please close this PR.

emirb · 2017-06-01T15:12:51Z

In voting: https://wiki.php.net/rfc/load-ext-by-name

flaupretre · 2017-06-01T15:14:48Z

@emirb Thanks for the notice :)

flaupretre · 2017-06-19T16:42:02Z

The RFC was approved and will be merged in 7.2.

sgolemon

I was about to merge this, fixing the minor nits along the way, but the fwrite(stderr, ...) concerns me in particular.

sgolemon · 2017-06-19T21:28:50Z

NEWS

@@ -15,6 +16,9 @@ PHP                                                                        NEWS
 - Standard
  . Compatibility with libargon2 versions 20161029 and 20160821.
    (charlesportwoodii at erianna dot com)
+  . Allow loading PHP and Zend extensions by name (instead of filename). Also
+    add support for extension name in dl() function.
+    (francois at tekwire dot net)


Move this up to Core

sgolemon · 2017-06-19T21:29:36Z

ext/standard/dl.c

@@ -81,10 +81,13 @@ PHPAPI PHP_FUNCTION(dl)
 PHPAPI int php_load_extension(char *filename, int type, int start_now)
 {
 	void *handle;
-	char *libpath;
+	char *libpath,
+		 *orig_libpath;


Move these to single lines or redeclare the type, please.

sgolemon · 2017-06-19T21:29:55Z

ext/standard/dl.c

+			} else {
+				spprintf(&libpath, 0, "%s%cphp_%s." PHP_SHLIB_SUFFIX, extension_dir, DEFAULT_SLASH, filename); /* SAFE */
+			}
+#else			


Trailing whitespace here and in several other places. Please clean that up.

sgolemon · 2017-06-19T21:30:36Z

main/php_ini.c

+			}
+#endif
+			if (VCWD_ACCESS(libpath, F_OK)) {
+                fprintf(stderr, "Cannot access Zend extension %s (Tried: %s, %s)\n", filename, orig_libpath, libpath);


Perhaps I missed the discussion, but why are you directly writing to stderr here? Surely we can zend_error() instead.

I just copied the same error mechanism as used in zend_load_extension() when the file cannot be loaded. I don't know why it calls 'fprintf(stderr' instead of zend_error(). I can change both if you think it's safe.

Without having looked deeper into it, but I guess it could be that zend_error() is unsafe to call if this is called in early startup, if not then I don't see a reason not to make it use the API

If we want to keep this code consistent with the way error is raised in zend_load_extension(), we should change both but, as you say, I'm not sure zend_error() can be called in every context where zend_load_extension() is executed, especially in the case of '-z' command line options.

I agree, as zend_load_extension use fprintf, shoudl also be used here.

But, IIUC, this is only used for better report about "tried" file name, so perhaps the second if (VCWD_ACCESS can be ignored.

@remicollet Sure, if the file does not exist, zend_load_extension() will fail and we don't need to check before we call it but I found it quite valuable for the user to display an error message containing the different paths we tried before giving up. That's why we keep the 1st path we tried in orig_libpath and display it in the error message.

@flaupretre indeed, (so I understand correctly). Fine for me.

Allow extension name as INI 'extension=' and dl() argument No BC break, as file name is still accepted. When using the '-z' command line option (CLI/CGI), an absolute file name must still be provided (nothing changed here) Change comments in example INI files

php-pulls · 2017-06-22T18:00:29Z

Comment on behalf of pollita at php.net:

Pushed as fe5c8f2 with a followup minor refactor d09edf7

flaupretre changed the title ~~Allow loading extension by name~~ Allow loading extensions by name Jan 28, 2016

nikic reviewed Jan 28, 2016
View reviewed changes

flaupretre force-pushed the load-extensions-by-name branch from 3b55a4a to 49fe831 Compare March 8, 2016 11:46

flaupretre force-pushed the load-extensions-by-name branch from 49fe831 to 0d937c7 Compare May 9, 2016 16:19

smalyshev added the RFC label Nov 27, 2016

flaupretre force-pushed the load-extensions-by-name branch 2 times, most recently from c4367ee to 483a8df Compare January 23, 2017 11:04

flaupretre force-pushed the load-extensions-by-name branch from 483a8df to c4367ee Compare February 12, 2017 10:56

flaupretre force-pushed the load-extensions-by-name branch from c4367ee to 406c866 Compare June 1, 2017 10:57

flaupretre force-pushed the load-extensions-by-name branch 2 times, most recently from 53a4cee to bd5cf8e Compare June 19, 2017 16:40

sgolemon requested changes Jun 19, 2017

View reviewed changes

flaupretre force-pushed the load-extensions-by-name branch from bd5cf8e to 53e9b78 Compare June 20, 2017 09:44

flaupretre force-pushed the load-extensions-by-name branch from 53e9b78 to 96c95de Compare June 20, 2017 10:30

php-pulls closed this Jun 22, 2017

flaupretre deleted the load-extensions-by-name branch June 23, 2017 10:27

thekid mentioned this pull request Nov 16, 2017

Allow loading extensions by name xp-runners/reference#71

Closed

Allow loading extensions by name #1741

Allow loading extensions by name #1741

Uh oh!

Conversation

flaupretre commented Jan 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flaupretre commented Jan 29, 2016

Uh oh!

flaupretre commented May 10, 2016

Uh oh!

krakjoe commented Jan 2, 2017

Uh oh!

emirb commented Jun 1, 2017

Uh oh!

flaupretre commented Jun 1, 2017

Uh oh!

flaupretre commented Jun 19, 2017

Uh oh!

sgolemon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flaupretre Jun 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

php-pulls commented Jun 22, 2017

Uh oh!

Uh oh!

flaupretre commented Jan 28, 2016 •

edited

Loading

flaupretre Jun 20, 2017 •

edited

Loading