Related issue: #4103

Looking at the test, I assume you didn't find a way to actually turn on OPcache at runtime. Can you provide a small comment of what you looked into and why it didn't work, for reference?

Thank you for the pull request!

There's only minor code style issues, and I would like the text to be more specific.

Hello,
Yes unfortunately I haven't found a solution for the unit test, the opcache.cli_enable can only be changed on the php.ini file or in the httpd.conf file. So it's not possible to do it.

I have tried to make the Namespace WP_CLI before rewriting the function get_cfg_var but in the end, the Runner.php does not found the WP_CLI class itself.
So I have included the php/class-wp-cli.php file at the beginning of the file but not working either unfortunately.

Using an environment variable is the wp-cli way to make such functions testable - see eg https://github.com/wp-cli/wp-cli/blob/master/php/utils.php#L513 and https://github.com/wp-cli/wp-cli/blob/master/tests/test-utils.php#L147.

(Also some nitpicks - perhaps use the more normal ini_get() rather than get_cfg_var(), and a simple if ( ! ini_get(...) ) as the cast doesn't add anything.)

Actually just tried it with PHP 7.1.4 where opcache.enable_cli is enabled by default (as of 7.1.2 opcache.configuration) so ini_get() must be used.

Though more importantly this makes a strong case for having wp-cli work with opache enabled anyway.

Which makes the reliance on comments for basic functionality such as the hooks mechanism a problem, as it doesn't work with opcache.save_comments disabled (related #1243). For instance, wp-cli/core-command#14 can be reproduced by doing

cd `mktemp -d`
php -dopcache.enable_cli=1 -dopcache.save_comments=0 `which wp` core download --debug

as $docparser->get_tag( 'when' ) returns empty at https://github.com/wp-cli/wp-cli/blob/master/php/WP_CLI/Dispatcher/CompositeCommand.php#L35.

@Demayl could you confirm whether or not opcache.save_comments was disabled in your case?

So I think in the medium term perhaps move away from using comments for basic functionality such as hooks etc, and use some explicit registration system or something instead - this obviously needs thought and discussion!

In the short term wp-cli should be tested with opcache.enable_cli=1 and opcache.save_comments=1 to see if there are any opcache issues apart from comments.

Also re the current issue, the check_opcache() should throw a fatal if save_comments is disabled, eg

if ( ini_get( 'opcache.enable_cli' ) ) {
	if ( ! ini_get( 'opcache.save_comments' ) ) {
		WP_CLI::error( "wp-cli requires the PHP INI directive `opcache.save_comments` to be enabled." );
	}
	if ( ! ini_get( 'opcache.load_comments' ) ) {
		ini_set( 'opcache.load_comments', '1' );
	}
	WP_CLI::warning( "blah" );
}

Also it should be tested using the php -d mechanism (no need for env vars etc) - cheers @davidfavor for the essential idea there! - and using behat instead or as well might be better.

@gitlost opcache.save_comments=0

Ta v much - the case for the prosecution grows!

So I think in the medium term perhaps move away from using comments for basic functionality such as hooks etc, and use some explicit registration system or something instead - this obviously needs thought and discussion!

This would be a substantial undertaking that I'm not sure is warranted. Many parts of WP-CLI (and, subsequently, the WP-CLI ecosystem) are dependent on comments. To refactor would require a huge amount of effort. If the only problem we're faced with is the issue of opcache.save_comments=0, I don't think it justifies a refactor.

Also re the current issue, the check_opcache() should throw a fatal if save_comments is disabled, eg

Correct. If any PHP configuration would cause WP-CLI to behave inconsistently, then we should error for that PHP configuration.

Some prior art #283 (comment)

To refactor would require a huge amount of effort.

I'm guessing there's a good chance it could all be done programmatically - but anyway I'll open an idea about it...

Closing in favor of #4161

Thanks for your work on this, @Rahe. We appreciate it.