Add hook_requirements() for PECL YAML parser

Makes sense

Something like this

Screenshots:

Without

With

1. +++ b/core/modules/system/system.install
   @@ -529,6 +529,24 @@ function system_requirements($phase) {
   +        '@url' => 'http://pecl.php.net/package/yaml'
   

   Missing comma

2. +++ b/core/modules/system/system.install
   @@ -529,6 +529,24 @@ function system_requirements($phase) {
   +        '@url' => 'http://pecl.php.net/package/yaml'
   

   And again.

Just a few style issues otherwise looks good.

2: yaml-extension-2157447.1.patch queued for re-testing.

Added the commas, fixed a typo and a minor rewording.

I tested this locally both without and with the extension enabled, and it works as expected.

Surely this should only be done once #1920902: Add a Drupal Yaml wrapper so we can default to PECL Yaml component if it is available is done?

Sorry for the confusion.

+++ b/core/modules/system/system.install
@@ -529,6 +529,24 @@ function system_requirements($phase) {
+      $requirements['pecl_yaml']['value'] = t('Enabled');

I'd better displayed version installed somehow

reroll patch against 8.3.x. tried to find out the version but not yet success.

I think http://php.net/manual/en/reflectionextension.getversion.php is what you looking for

Thank you @andypost for sharing the link. Here is the updated patch along with the screen short after change.

Looks great for me

+++ b/core/modules/system/system.install
@@ -257,6 +257,25 @@ function system_requirements($phase) {
+  if ($phase == 'runtime') {

Any reason we're not also doing this for $phase == 'install' ?

For checking the opcache extension, for example, we are also taking both install and runtime into account.

Probably it makes sense to notice at install as well otoh this is PECL and not so easy as opcache shiped
Looking at #2792877: Symfony YAML parser fails on some strings when running PHP 7 also I worry about this sort of issues

Reroll & fix #19

+++ b/core/modules/system/system.install
@@ -283,6 +283,25 @@ function system_requirements($phase) {
+      'severity' => REQUIREMENT_WARNING,

This shouldn't be a warning. It's not wrong to not use it. Personally I feel like this is not necessary to put requirements screen. Also I can't see how this is related to #2844452: Export configuration YAML strings as multiline at all.

It is related by different behavior when pecl enabled.
As it gives 10% speed-up sometimes this looks like warning similar to opcache

10% only on very cold caches. It is completely unlike opcache which affects every request. This affects 0 requests once caches are in place. Also the only reason you are seeing differences is because you are writing with PECL yaml which we do not support at all. All writes should take place with Symfony to ensure that config is transportable regardless of what you have installed.

I wonders why you so cares about formating when yaml supposed to be formating agnostic, and it was the main reason to select it over more common xml because it more readable for humans

Finally core operates at config objects which are going to be typed (mostly) then excachange should just have tests that pecl->SF and back works at config level

@andypost so... we care about portability and people ending up with the same results regardless of what PHP extensions installed. And seeing unexpected differences in YAML files which end up having no differences in the data is to be avoided at all costs. That's why Symfony is always used to write. Plus there's the fact that Symfony respects the 1.2 spec whereas PECL is 1.1.

As stated before I think we shouldn't recommend PECL Yaml on the status report and certainly not with a warning.

I agree that the WARNING is a bit alarming.

For comparison, upload progress doesn't set a severity level: https://api.drupal.org/api/drupal/core%21modules%21file%21file.install/f...

Let's downgrade the warning.

As related showing it still good to have #3205480-18: Drop PECL YAML library support in favor of only Symfony YAML

Created MR with the latest patch

Fixed #33

I don't know if we have to recommend the PECL yaml parser. I think we could indicate what's be used for YAML parsing on the status report and link to a page on drupal.org that discusses the possibilities..

#47++

Recommending it is too strong, because one needs control over their hosting environment to be able to do this.

Informing is great 👍

The MR already provides informing

Your server is not using the PECL YAML extension for parsing YAML. Installing this extension will improve the performance of parsing YAML files on your server.

maybe it needs section at https://www.drupal.org/docs/getting-started/system-requirements/php-requ... with the link from report/status

"will improve" sounds like a recommendation to me? 😅

Left a suggestion for changing it to "merely informing", i.e. purely observing.

Agree that

Installing this extension will improve the performance of parsing YAML files on your server.

sounds like a recommendation, or. that not doing it means I'm doing something wrong.

But maybe that sentence could be added to a section in https://www.drupal.org/docs/getting-started/system-requirements/php-requ... that @andypost recommended.

So maybe

Your server is not using the PECL YAML extension for parsing YAML. See @link for more information.

It becoming a follow-up for #3205480: Drop PECL YAML library support in favor of only Symfony YAML

proper status