Configuration names do not always begin with the extension that actually owns the configuration

Gave this a spin, I actually like the directory structure, it feels more sane to the active store directory (cf screenshot further down).

Two things I've noticed:

• if you copy system.site.yml to your staging directory, the configuration screen will detect it but will not import it. I guess this can qualify as a bug.
• if you move that file to a subdirectory 'system' (as it probably should), importing works fine. The file is gone, but the system directory remains. Not sure whether we want to remove this or not.

Hello directories :)

Some code comments.

+++ b/core/includes/config.incundefined
@@ -93,10 +91,11 @@ function config_get_storage_names_with_prefix($prefix = '') {
+ *   This either be a string or an instance of \Drupal\Core\Config\ConfigName.

This 'can' either ..

+++ b/core/includes/config.incundefined
@@ -349,7 +366,7 @@ function config_import_invoke_owner(array $config_changes, StorageInterface $sou
+      if ($entity_type = config_get_entity_type_by_name((string) $name)) {

Just wondering why you cast to string here, I'm seeing this in a lot of places, can $name really be something else ?

+++ b/core/lib/Drupal/Core/Config/ConfigDirectoryFilterIterator.phpundefined
@@ -0,0 +1,47 @@
+ * Definition of Drupal\Core\Config\ConfigDirectoryFilterIterator.

We use 'Contains' nowadays right ?

+++ b/core/lib/Drupal/Core/Config/ConfigName.phpundefined
@@ -0,0 +1,86 @@
+ * not define. Also it allows the active file storage to be organised into

It 'also' allows ?

+++ b/core/lib/Drupal/Core/Config/EnableStorage.phpundefined
@@ -0,0 +1,110 @@
+ * Storage controller used by the Drupal when enabling extensions.

redundant 'the'

+++ b/core/lib/Drupal/Core/Config/StorageInterface.phpundefined
@@ -141,4 +142,14 @@ public function listAll($prefix = '');
+   * Gets the a directory if implemented by the storage engine.

redundant 'a' or 'the'.

+++ b/core/lib/Drupal/Core/Config/TypedConfigManager.phpundefined
@@ -39,15 +39,19 @@ public function __construct(StorageInterface $configStorage, StorageInterface $s
+   *   Configuration object name. Either a string on an instance of

'or' instead of 'on'

+++ b/core/lib/Drupal/Core/Config/TypedConfigManager.phpundefined
@@ -39,15 +39,19 @@ public function __construct(StorageInterface $configStorage, StorageInterface $s
+    if (!is_object($name)) {
+      $name = new ConfigName($name);

I'm generally not a fan being able to pass on mixed params. However, I haven't looked in depth whether it's solvable or not.

It seems to me that this issue is just a duplicate of the very very long and involved discussion that has already happened at #1776830: [META-1] Installation and uninstallation of configuration provided by a module that belongs to another module's API? A lot of conclusions have been reached there and I hope everyone has read and ingested all that.

Also, I'm against making any changes for the purposes of disabled modules until #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed gets resolved.

Finally, by moving config into subdirectories we remove the ability to sort all config by date changed, which is a very easy way to tell what config has changed recently or not. I'm not a big fan of that.

IMHO all subdirectories should be (or must be) reserved for config context override files.

I originally asked that the Locale module overrides should be moved into and contained in respective ./locale.whatever-foo/ or ./language-foo subdirectories. Last time I looked, Locale still used config filename modifiers instead of subdirectories. We definitely need to move the overrides into subdirectories — otherwise, the whole thing becomes unmaintainable.

This seems like it would simplify a number of other problems. It's both robust and simple. If we restrict it to a single subdirectory per module, I think it also is an acceptable increase in complexity usability-wise.

See also: #1776830: [META-1] Installation and uninstallation of configuration provided by a module that belongs to another module's API

Also, I'm against making any changes for the purposes of disabled modules until #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed gets resolved.

I'm not sure we can afford to wait on that.

#22: 1932600.configuration-owner.22.patch queued for re-testing.

Hi guys, this is a re-roll of the patch. Tests still fail, but I thought it would be better to post it now than later (specially since I'm probably done for today). I can install drupal, config files are in their own directory and in general seems like things are going well, except on the tests =P.

This is an example test fail from running "sudo -u www-data php ./core/scripts/run-tests.sh Configuration":

exception 'Exception' with message 'The configuration directory type <em class="placeholder">staging</em> does not exist.' in /home/jackbravo/work/drupal-sites/git-drupal2/core/includes/bootstrap.inc:443
Stack trace:
#0 /home/jackbravo/work/drupal-sites/git-drupal2/core/modules/system/system.install(351): config_get_config_directory('staging')
#1 /home/jackbravo/work/drupal-sites/git-drupal2/core/includes/install.inc(1000): system_requirements('install')
#2 /home/jackbravo/work/drupal-sites/git-drupal2/core/includes/install.core.inc(2143): drupal_check_profile('testing', Array)
#3 /home/jackbravo/work/drupal-sites/git-drupal2/core/includes/install.core.inc(941): install_check_requirements(Array)
#4 /home/jackbravo/work/drupal-sites/git-drupal2/core/includes/install.core.inc(657): install_verify_requirements(Array)
#5 /home/jackbravo/work/drupal-sites/git-drupal2/core/includes/install.core.inc(517): install_run_task(Array, Array)
#6 /home/jackbravo/work/drupal-sites/git-drupal2/core/includes/install.core.inc(93): install_run_tasks(Array)
#7 /home/jackbravo/work/drupal-sites/git-drupal2/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php(782): install_drupal(Array)
#8 /home/jackbravo/work/drupal-sites/git-drupal2/core/modules/simpletest/lib/Drupal/simpletest/TestBase.php(741): Drupal\simpletest\WebTestBase->setUp()
#9 /home/jackbravo/work/drupal-sites/git-drupal2/core/scripts/run-tests.sh(484): Drupal\simpletest\TestBase->run()
#10 /home/jackbravo/work/drupal-sites/git-drupal2/core/scripts/run-tests.sh(22): simpletest_script_run_one_test('30', 'Drupal\config\T...')
#11 {main}FATAL Drupal\config\Tests\ConfigEntityListTest: test runner returned a non-zero error code (1).
- Found database prefix 'simpletest349281' for test ID 30.

Sorry for not providing an interdiff, but I could not apply the last patch to go from there.

I modified also core/lib/Drupal.php which like config.inc has a function config($name). So just a minor edit.

Ok. I had a permissions problem, and that's where most exception came from. The patch did have some errors, most related with some tests not being up to date with the code. I updated this tests and now I'm down to two fatal errors when running sudo -u www-data php ./core/scripts/run-tests.sh Configuration. And I don't think this errors are related to this patch :P.

So here is the new patch.

Thanks @jackbravo!

Unfortunately this needs another reroll.

I think the diffstat might be a bit too different for the reroll in #29 as well. It might make sense to do another reroll from #22 - this may be a difficult reroll since it's a relatively large patch touching many files.

Patch in #22:
46 files changed, 791 insertions, 297 deletions.

Patch in #29:
47 files changed, 683 insertions, 214 deletions.

Rerolling.

I can't quite figure this one out either. :(

See also: #1776830: [META-1] Installation and uninstallation of configuration provided by a module that belongs to another module's API

This might not be necessary if we resolve the above issue by using subdirectories or name parsing in the default config directories above, but I'm not 100% convinced of either of those solutions.