-
Notifications
You must be signed in to change notification settings - Fork 266
PHPLIB-572: Add debugging tools #996
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -53,6 +53,20 @@ SAPI. Additionally, :php:`php_ini_loaded_file() <php_ini_loaded_file>` and | |||
:php:`php_ini_scanned_files() <php_ini_scanned_files>` may be used to determine | |||
exactly which INI files have been loaded by PHP. | |||
|
|||
To debug issues with the extension not being loaded, you can use the | |||
``detect-extension`` script provided in the tools directory. You can run this | |||
script from the CLI or include it in a script accessible via your web server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we expect people might run these scripts from a web SAPI, would it make sense to conditionally wrap the output in <pre>
? Or I suppose people can just do that themselves -- there's really no reason to make these more complicated than they already are.
tools/detect-extension.php
Outdated
$version = phpversion($extension); | ||
|
||
if (PHP_SAPI !== 'cli') { | ||
echo '<pre>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I forgot I added this. It is notably omitted from connect.php
, although I don't recall why that is.
Does it make sense to keep this here? Note that we never close this tag later, so we might be better off removing this and suggesting web SAPI users call this like <pre><?php require(...); ?></pre>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also close the <pre>
tag before exiting - the missing closing tag doesn't matter when the script is run on its own, but could cause problems if the script was included in between other "stuff".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with either of these so long as we do the same for all scripts:
- Wrap output in
<pre></pre>
whenPHP_SAPI
is notcli
- Ignore any attempt for format the output and just tell users to use
<pre>
themselves in the docs
Slight preference for the second approach since it makes the scripts less complicated.
If you want to go with the first approach I think the cleanest way to do this would be something like the following so we don't need to worry about multiple exit points:
if (PHP_SAPI !== 'cli') {
echo '<pre>';
register_shutdown_function(function() { echo '</pre>'; });
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the existing echo
and documented the code. Using register_shutdown_function
wouldn't echo the closing pre
tag after returning from the corresponding script, but when the entire execution context finishes, which could leave users with potentially broken output. Note that I've documented this for detect-extension.php
but not for the connection tool as I believe the latter won't be run in the web SAPI as often as the extension tool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using register_shutdown_function wouldn't echo the closing pre tag after returning from the corresponding script, but when the entire execution context finishes
Oh, good point there. This works for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHPLIB-572
This PR adds the two debugging scripts from https://github.com/jmikola/buildfest2021 to a new
tools
directory.The
connect.php
script attempts to connect to a specified MongoDB deployment using socket functions, bypassing the extension. This can be used in case of connection errors when using the library.The
detect-extension.php
script can be used to debug why the extension isn't being loaded (in any SAPI). I've modified the script compared to the version in the original repository:PHP_EXTENSION_DIR
constant withini_get('extension_dir')
. I didn't know this before, but the constant always contains the default extension directory which may be overridden by theextension_dir
ini setting. In that case, the constant directs us to the wrong directory.I've also added language to the FAQ page to point out these tools and show usage examples.