Config ignore currently depends on the config_filter module, that does the heavy lifting of hooking into the config part of Drupal and adjusting the way it imports and exports configuration.
With the release of Drupal 8.8, a new API is available that makes it much easier for modules to adjust the way config imports and exports work. This API superseeds the API that's available with the config_filter module.
If a 3.x version of config_ignore would be created with the new API, it would lose its dependency on config_filter and it would work by default with tools like Drush.
Proposed resolution
- Create a 3.x version of the config_ignore module that is only compatible with Drupal 8.8+.
- Make use of the new config event api, and thereby remove dependency on config_filter.
Current status
- There is a proof-of-concept patch.
- With the new API, config is ignored both when importing and exporting config. This is a bit different from the behaviour described in this comment from Bircher, as new config won't be imported when it is ignored.
- The patch is not yet formatted according to Drupal standards.
- Tests have not been made / adjusted yet.
- The patch removes the list of config entities that are ignored from the config sync page. I don't know if it is a feature that is used a lot; it's not shown when importing/exporting with Drush.
- The patch also contains fixes for #3067441: Can't ignore keys that contain colon (:) character, #2922596: Undefined index if configuration to ignore is not available , #2857944: Support for export filtering via the UI and superseeds #3098130: Make config_ignore work with Drupal 8.8 (and config_filter 2.x) and #2857247: Support for export filtering via Drush
Comments
Comment #2
bartlangelaanThis is the patch that gives an idea how this could be implemented. As stated in the issue summary, it's just a proof of concept and is not according to standards yet.
Comment #3
jkswoods CreditAttribution: jkswoods commentedHave tested this and it works fine with 8.8. On a related note, I'd have to agree with a 3.x branch for these changes, dropping the dependency on config_filter is a good move and makes the module easier to maintain.
Comment #4
JeroenTComment #5
jkswoods CreditAttribution: jkswoods commentedfnmatch
has been used within the patch which is not available on non-POSIX compliant systems according to php.net https://www.php.net/manual/en/function.fnmatch.phpComment #6
JeroenTAbout
fnmatch
. This is also used in the current version of the module and is already being discussed in #3042661-7: Drupal 9 Deprecated Code Report.Comment #7
jkswoods CreditAttribution: jkswoods commentedOkay cool, based on the discussion in that issue fnmatch is a not a priority for now.
Comment #8
claudiu.cristeaWe shouldn't change existing update hooks. Projects may uninstall
config_filter
if they consider that it's not used by any other functionality.Needs {@inheritdoc} block.
Should have code block delimiters
{...}
.Should start with a verb. E.g. "Acts on config import/export transform."
The services should be injected.
Remove the empty line. Format the comments to fit 80 chars.
Missed doc block.
Comment #9
claudiu.cristeafnmatch()
.Comment #10
Rajab Natshah CreditAttribution: Rajab Natshah at Vardot for Vardot commented+1
Comment #11
jkswoods CreditAttribution: jkswoods commentedRe #9: Have tested locally and can confirm this is working fine.
Comment #12
ayalon CreditAttribution: ayalon commentedI tested it a lot and finally figured out that you need Drush 10. It does not work on Drush 9.7.1 at all because the new service is not used and therefore the hooks are not called. Not very obvious.
Comment #13
jkswoods CreditAttribution: jkswoods commented@ayalon the README.txt has been updated in the patch to mention requiring Drush 10.
Comment #14
JeroenTPatch #9 does not work in combination with configuration translation.
e.g. Create a webform and translate it in multiple languages. As ignored config I used 'webform.webform.*'.
When runnning `drush cim`the translated config is removed.
Comment #15
claudiu.cristea#14 needs regression test.
Comment #16
kim.pepperAny examples of a test that uses config translation?
Comment #17
kim.pepperI thought I'd start working on this with a KernelTest, but ran into issues with a simple copy of the BrowserTest. There must be something I don't get about how to do kernel tests of config import etc.
Comment #19
ayalon CreditAttribution: ayalon commentedThe bug decribed in #14 makes this patch very dangerous for multilingual site because of potential loss of all new translations. So be careful using this patch until the bug is fixed.
Comment #20
ayalon CreditAttribution: ayalon commentedI'm not an expert of the "Configuration Management Interface" of Drupal.
But here is my proposal, how to fix the issue with the translations (collections) of ignored configurations.
The patch is based on #9.
Please give some feedback, if you think this is a possible solution.
Comment #21
kim.pepperComment #22
kim.pepper@ayalon Looks like your patch has some issues. Did you follow the steps outlined in https://www.drupal.org/node/1054616 ?
Comment #23
ayalon CreditAttribution: ayalon commentedI can apply the patch using composer patches. Not sure why you have issues.
Comment #24
claudiu.cristea@ayalon, I cannot apply it either:
Comment #25
mikran CreditAttribution: mikran at Mediamaisteri Oy commentedI managed to apply the patch by adding a newline to the end of the patch file.
Here is the patch with just the newline added.
Comment #26
mikran CreditAttribution: mikran at Mediamaisteri Oy commented#20 did not include the changes made in #17, so here is a new rerolled patch with those changes included, and an interdiff.
Comment #27
kim.pepperThis builds on top of #26 fixing a couple of tests an code style.
Comment #29
claudiu.cristeaHere's a test that checks the export/import but, this time, is using Drush. Unfortunately I couldn't follow the changes since #9, so this patch is built against #9. The test proves that both, import & export are buggy.
Comment #30
claudiu.cristeaHm, the first test should not fail in that line. Look like there's an issue with the file permissions?
Comment #31
claudiu.cristeaRemoving the debug line.
Comment #32
claudiu.cristeaHere's the fix. Note that the main test fails on composer as this needs Drupal >=8.8.x, so check only the right test. Normally, this patch, if it's approved will be against a new 8.x-3.x branch.
Comment #33
claudiu.cristeaFixed coding standards, improved docs, removed code duplication from unit test.
Comment #34
claudiu.cristeaOther small docs improvements.
Comment #35
claudiu.cristeaTry to support also PHP 7.0
Comment #36
claudiu.cristealist() shorthand not supported in 7.0
Comment #38
claudiu.cristeaAs long as this new branch will require Drush 10, which requires PHP >=7.1, we will not support 7.0.
Also the
config_ignore_deactivate
kill-switch should not act when building the listeners list but on each listener. The reason is that the listeners list is rebuild only on container rebuild. Someone might add the kill-switch in settings.php without rebuilding the container (clearing the cache) and, as an effect, the kill-switch will not work.Comment #39
kim.pepperThanks for pushing this forward @claudiu.cristea. Looks like you have addressed the issue in #14.
Comment #40
bircherThis looks very good.
I am going to commit this with the difference that we require
^8.8
.I have some concerns with the following: The expansion of the ignored wildcard keys is done against the active config for both import and export.
Also I inverted the for loops for the collections and ignore names, so that creacteCollection is in only one loop.
Comment #41
bircherComment #42
claudiu.cristeaComment #44
bircherSorry, for some reason I had the wrong patch, fixed now. I guess I should have double checked again after my laptop froze.
Comment #45
ayalon CreditAttribution: ayalon commentedThanks a lot for the fast adoption and all the worktime went into this.
Comment #46
claudiu.cristea@ayalon, it was discovered that the current 3.x branch has some bugs. I've provided a fix in #3117135: TypeError: Argument 1 passed to NestedArray::unsetValue() must be of the type array, null given. Could you test 3.x with that patch applied?