@tadean

Thanks @joegraduate!

It looked like MR !4 might potentially have had an issue with configuration items that were also falsy. This seems resolved in MR !5 with the strict check. It seems reasonable that the PHPStan issues can be addressed separately from the test failure.

I tested out MR !5 and the snapshot storage still seems to work as expected. !5 looks good to me.

Added potential patch.

tadean → created an issue.

Added potential patch approach for the issue - thanks to @trackleft2 for suggestion.

tadean → created an issue.

Thank you for your work on this! Tested out MR !10 locally with Drush 12.1.3. Verified the original issue:

$ drush config-sync-list-updates
In LegacyServiceInstantiator.php line 282:                       
  You have requested a non-existent parameter "config.import.commands". 

Verified that with the changes in MR !10 the error does not occur, and config_sync reports expected changes correctly:

$ drush config-sync-list-updates
 ----------------- ---------------- ----------------- 
  Extension         Operation type   Label            
 ----------------- ---------------- ----------------- 
  my_test_content   update           My Content Type  
 ----------------- ---------------- -----------------

config_sync tests pass:

PHPUnit 9.6.10 by Sebastian Bergmann and contributors.

Testing /usr/local/config_sync
....                                                                4 / 4 (100%)

There is one minor phpcs complaint about the create() function:
74 | ERROR | [x] Missing function doc comment (Drupal.Commenting.FunctionComment.Missing)

And one minor complaint about the $state. in the docblock of the constructor:
58 | ERROR | [x] Doc comment parameter name "$state." must not end with a dot (Drupal.Commenting.FunctionComment.ParamNameDot)
This error was unrelated and not introduced by this MR though.

Thanks for all the promising work on this issue! I tested the refactored config-distro-update command against Drush 12.1.2, used in conjunction with the related patch from the config_sync issue 🐛 Config Sync breaks with Drush 12, need to refactor not to use ConfigImportCommands Fixed to avoid the similar error and was able to import configuration changes from a custom module without error:

$ drush config-distro-update
+------------+---------------------------+-----------+
| Collection | Config                    | Operation |
+------------+---------------------------+-----------+
|            | node.type.my_content_type | Update    |
+------------+---------------------------+-----------+

 Import the listed configuration changes? (yes/no) [yes]:
 > yes

 [notice] Synchronized configuration: update node.type.my_content_type.
 [notice] Finalizing configuration synchronization.
 [success] The configuration was imported successfully.

I verified manually that the configuration seemed to be imported correctly.

joegraduate → credited tadean → .

joegraduate → credited tadean → .

Thank you for these great patches @a.dmitriiev !

I tested both of these patches out with the config_distro_ignore functionality. In my testing it appears that the patch in #3144145-6: Consider replacing fnmatch usage → has an issue where any ignore rule will match any config name (e.g. foo will match node.type.my_content_type). The original patch in #3144145-2: Consider replacing fnmatch usage → seemed to work in almost all cases but has a minor issue where a partial match now matches a complete config name (e.g. content matches node.type.my_content_type because the generated regex pattern was missing the ^$

I have updated the patch in #3144145-2: Consider replacing fnmatch usage → to fix the substring issue, use preg_quote, and for coding standards. I have removed the case insensitivity flag because fnmatch() wasn't originally being used with FNM_CASEFOLD. I think in some cases (windows platforms, maybe?) the original implementation using fnmatch() might have been case insensitive in some places and not others.

Most of the fnmatch() replacements in Drupal have not been a complete reimplementation of the shell-style patterns as regex, but this patch retains the wildcard functionality on * and ?, which I think is useful for the config_distro_ignore use case.

This patch refactors isActiveStorageContext() to make use of Drupal\Component\DependencyInjection\ReverseContainer for reverse lookup of the storage service. Since this branch is currently cites compatibility with D9 in general, I have attempted to make the change backwards compatible, with the following approaches:

• In 9.5.1+, ReverseContainer will be used
• In 9.5.0, as ReverseContainer does not exist, getServiceIdMapping() will be used
• in 9.4 and below, it will fall back on the previous approach of the _serviceId property

At the point when only 9.5.1+ compatibility is needed, I think these deprecated fallbacks should probably be removed.

The patch includes the new test from @joegraduate thanks!

In my testing with 9.4.15, 9.5.0, and 9.5.1+ this seems to fix the original broken config_sync → test ( 📌 Address 3.0.x-dev test failures since Drupal 9.5.x Fixed ) that revealed this issue.

testUpdateModeFullReset() in config_sync is failing correctly; I think I have traced the reason for the failure to an apparent issue in config_normalizer related to a change in 9.5.x related to the _serviceId property:

🐛 isActiveStorageContext() fails in 9.5.x Fixed

tadean → created an issue.

I think this looks pretty promising, and seems to address the test failures. Setting this to RTBC.