Problem/Motivation

In #2432791: Skip Config::save schema validation of config data for trusted data we decided to not calculate configuration dependencies when we are working with trusted data.

But, it is possible for a module in hook_entity_create or hook_entity_ENTITY_TYPE_create to influence dependencies.

Issue priority

Critical because "Cause loss/corruption of stored data."
"Incorrect dependencies can lead to things being deleted when the shouldn't and not being deleted when they should." (#14)

Proposed resolution

calculate dependencies even for trusted data

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
@@ -341,7 +341,7 @@ public function preSave(EntityStorageInterface $storage) {
-    if (!$this->isSyncing() && !$this->trustedData) {
+    if (!$this->isSyncing()) {

Remaining tasks

User interface changes

No.

API changes

No.

Data model changes

No.

Files: 
CommentFileSizeAuthor
#66 2520526-2-64.patch23.3 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,716 pass(es). View
#66 2520526-62-64-interdiff.txt4.53 KBalexpott
#62 2520526-2-62.patch21.81 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,218 pass(es), 428 fail(s), and 22 exception(s). View
#62 2520526-61-62-interdiff.txt2.07 KBalexpott
#61 2520526-57-61-interdiff.txt5.34 KBalexpott
#61 2520526-2-61.patch21.94 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,512 pass(es). View
#57 interdiff.txt3.57 KBclaudiu.cristea
#57 2520526-57.patch20.99 KBclaudiu.cristea
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,498 pass(es). View
#56 2520526-2-56.patch20.95 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,561 pass(es). View
#56 2520526-52-56-interdiff.txt2.04 KBalexpott
#54 calculate_configuration-2520526-52.patch21.38 KBnlisgo
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,502 pass(es). View
#54 interdiff-2520526-48-52.txt2.11 KBnlisgo
#48 interdiff.txt8.53 KBdawehner
#48 2520526-48.patch21.26 KBdawehner
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,164 pass(es). View
#43 2520526-2-43.patch18.21 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,147 pass(es), 11 fail(s), and 1 exception(s). View
#43 40-43-interdiff.txt7.7 KBalexpott
#40 increment.txt1.29 KBpwolanin
#40 2520526-40.patch14.52 KBpwolanin
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,180 pass(es). View
#37 increment.txt2.79 KBpwolanin
#37 2520526-37.patch13.75 KBpwolanin
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,150 pass(es), 2 fail(s), and 0 exception(s). View
#31 interdiff.txt4.21 KBdawehner
#31 2520526-31.patch13.3 KBdawehner
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 112,847 pass(es), 457 fail(s), and 35 exception(s). View
#20 2520526-20.patch12.24 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,653 pass(es). View

Comments

catch’s picture

Issue tags: +rc target
alexpott’s picture

Status: Active » Needs review
FileSize
2.99 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,249 pass(es), 3 fail(s), and 0 exception(s). View
swentel’s picture

  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
    @@ -341,7 +341,7 @@ public function preSave(EntityStorageInterface $storage) {
    -    if (!$this->isSyncing() && !$this->trustedData) {
    

    So, would the test fail without this change ? Because I can't find calls to trustData when creating an entity anywhere.

  2. +++ b/core/modules/config/src/Tests/ConfigInstallTest.php
    @@ -7,6 +7,7 @@
    +use Drupal\config_test\Entity\ConfigTest;
    

    The use statement doesn't seem to be used, so can go away ?

Eli-T’s picture

FileSize
2.76 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,631 pass(es), 432 fail(s), and 22 exception(s). View
494 bytes

Removed redundant use statement identified in #3, otherwise the original patch looks good.

Also to confirm the test does fail without the change.

Status: Needs review » Needs work

The last submitted patch, 4: calculate_configuration-2520526-4.patch, failed testing.

Eli-T’s picture

Patch in #2 also currently fails testing locally when applied to current HEAD.

The last submitted patch, 4: calculate_configuration-2520526-4.patch, failed testing.

Eli-T queued 2: 2520526.1.patch for re-testing.

The last submitted patch, 2: 2520526.1.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
8.85 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,183 pass(es), 2 fail(s), and 0 exception(s). View
6.08 KB

Fixed the patch... although hmmm.

Status: Needs review » Needs work

The last submitted patch, 10: 2520526-10.patch, failed testing.

The last submitted patch, 10: 2520526-10.patch, failed testing.

alexpott’s picture

Issue tags: -rc target +rc deadline
alexpott’s picture

Priority: Major » Critical
Status: Needs work » Needs review
FileSize
1.63 KB
10.48 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,665 pass(es). View

Considering that configuration is being saved with incorrect dependencies and the affects that that can have I think this is a critical issue. Incorrect dependencies can lead to things being deleted when the shouldn't and not being deleted when they should.

dawehner’s picture

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
@@ -301,8 +301,11 @@ public function hasNewEntity() {
+
+    $target_entity_type = \Drupal::entityManager()->getDefinition($field_definition->getFieldStorageDefinition()->getSetting('target_type'));
+    $dependencies['module'][] = $target_entity_type->getProvider();
+

This patch seems not to add test coverage for this bit?

Wim Leers’s picture

Status: Needs review » Needs work
  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
    @@ -341,7 +341,7 @@ public function preSave(EntityStorageInterface $storage) {
    -    if (!$this->isSyncing() && !$this->trustedData) {
    +    if (!$this->isSyncing()) {
    

    This is the key change. I've verified locally that there is test coverage: reverting this change causes the updated ConfigInstallTest to fail.

  2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
    @@ -301,8 +301,11 @@ public function hasNewEntity() {
    +    $target_entity_type = \Drupal::entityManager()->getDefinition($field_definition->getFieldStorageDefinition()->getSetting('target_type'));
    +    $dependencies['module'][] = $target_entity_type->getProvider();
    

    So this was a missing dependency.

    Isn't that technically out of scope for this issue? And agreed with @dawehner that test coverage for this is missing.

  3. +++ b/core/modules/aggregator/config/install/core.entity_view_display.aggregator_item.aggregator_item.summary.yml
    @@ -5,7 +5,6 @@ dependencies:
    -    - entity_reference
    
    +++ b/core/modules/book/config/install/core.entity_form_display.node.book.default.yml
    @@ -5,7 +5,6 @@ dependencies:
    -    - entity_reference
    

    I don't understand what change in this patch can cause some dependencies to be omitted.

  4. +++ b/core/profiles/standard/config/install/field.field.node.article.field_image.yml
    @@ -5,6 +5,7 @@ dependencies:
    +    - file
    
    +++ b/core/profiles/standard/config/install/field.field.node.article.field_tags.yml
    @@ -6,6 +6,7 @@ dependencies:
    +    - taxonomy
    

    I do understand that additional dependencies can appear.

  5. +++ b/core/profiles/standard/config/install/field.storage.node.field_image.yml
    @@ -2,8 +2,8 @@ langcode: en
    -    - node
    ...
    +    - node
    

    I don't understand how this patch can cause listed dependencies to have a different order.

  6. +++ b/core/profiles/standard/config/install/field.storage.node.field_tags.yml
    @@ -2,8 +2,8 @@ langcode: en
    +    - entity_reference
    ...
    -    - taxonomy
    

    I don't understand how the changes in this patch can cause dependencies to change.

yched’s picture

Regarding entity_reference : this is why we really should proceed with #2429191: Deprecate entity_reference.module and move its functionality to core (which is 99% ready)

Atm, entity_reference.module is not needed for e_r base fields (Core now contains the full-featured e_r field type), but only to create configurable e_r fields. That is an arbitrary split at this point. (I think that explains the hunks mentioned by @Wim)

YesCT’s picture

critical so the rc deadline tag is not needed.

filled out more in the summary. seems like no api or data model changes.

alexpott’s picture

Re #16

  1. Yep there is explicit test coverage added for this bit of the change
  2. The current entity reference fields assume that this dependency is there - it is correct to add it and I think reduces the amount of change here.
  3. Because atm there is nothing that makes this calculable in this specific instance I think the formatter and widget have moved to core making this change correct
  4. Because of the hunk you commented in #2
  5. Because dependencies are ordered but the default config contains dependencies in the wrong order
  6. Because the default config has the wrong dependencies - this is kind of the whole point of the patch
alexpott’s picture

Status: Needs work » Needs review
Issue tags: +missing config dependencies
FileSize
1.76 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,660 pass(es), 1 fail(s), and 0 exception(s). View
12.24 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,653 pass(es). View

Here is a test and a test only patch (which is the interdiff)

The last submitted patch, 20: 2520526-20.test-only.patch, failed testing.

The last submitted patch, 20: 2520526-20.test-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 20: 2520526-20.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
jibran’s picture

Status: Needs review » Reviewed & tested by the community

Seems ready to me.

dawehner’s picture

Thank you alex!

xjm’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs upgrade path, +Needs upgrade path tests, +Needs manual testing

This looks like it needs an upgrade path (or several actually) -- if there are all these config entities on existing sites that are missing dependencies, and furthermore shipped entities with wrong dependencies, then we also need to update those existing sites.

Also, some manual testing would be a really good idea, given the broad impact of this bug and fix.

cosmicdreams’s picture

What is a scenario that you would like to be tested?

heddn’s picture

dawehner’s picture

Looking at the update path.

dawehner’s picture

Status: Needs work » Needs review
Issue tags: +D8 Accelerate
FileSize
13.3 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 112,847 pass(es), 457 fail(s), and 35 exception(s). View
4.21 KB

There we go.

Status: Needs review » Needs work

The last submitted patch, 31: 2520526-31.patch, failed testing.

The last submitted patch, 31: 2520526-31.patch, failed testing.

Wim Leers’s picture

  1. +++ b/core/modules/system/src/Tests/Update/RecalculatedDependencyTest.php
    @@ -0,0 +1,45 @@
    +  public function testUpdate() {
    +    $this->runUpdates();
    +
    +    $data = \Drupal::config('field.field.node.article.field_tags')->get();
    +    $this->assertEqual(['taxonomy'], $data['dependencies']['module']);
    +
    +    $data = \Drupal::config('field.field.user.user.user_picture')->get();
    +    $this->assertEqual(['file', 'image', 'user'], $data['dependencies']['module']);
    +
    +    $data = \Drupal::config('field.storage.node.field_image')->get();
    +    $this->assertEqual(['image', 'node'], $data['dependencies']['module']);
    +
    +    $data = \Drupal::config('field.storage.node.field_tags')->get();
    +    $this->assertEqual(['node'], $data['dependencies']['module']);
    

    I think it'd be clearer (and safer) to also test the expectations *before* running the updates, so that it's clear what exactly you expect to change.

  2. +++ b/core/modules/system/system.post_update.php
    @@ -31,5 +34,30 @@ function system_post_update_fix_enforced_dependencies() {
    +  // Loads scan all module install folders and resave the entities in there.
    +  $modules = array_keys(\Drupal::moduleHandler()->getModuleList());
    +  foreach ($modules as $module) {
    +    $default_install_path = drupal_get_path('module', $module) . '/' . InstallStorage::CONFIG_INSTALL_DIRECTORY;
    +    $storage = new FileStorage($default_install_path, StorageInterface::DEFAULT_COLLECTION);
    

    Does this then also include optional config?

    Either way: would be good to have explicit test coverage for optional config too.

  3. +++ b/core/profiles/standard/config/install/field.field.node.article.field_tags.yml
    @@ -5,7 +5,6 @@ dependencies:
    -    - entity_reference
    
    +++ b/core/profiles/standard/config/install/field.storage.node.field_tags.yml
    @@ -2,7 +2,6 @@ langcode: en
    -    - entity_reference
    

    Why these changes?

catch’s picture

#34-3 #2429191: Deprecate entity_reference.module and move its functionality to core but also prior to that patch entity_reference was only required for field_ui not for programmatic creation.

pwolanin’s picture

trying to fix some of the test fails

pwolanin’s picture

Status: Needs work » Needs review
FileSize
13.75 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,150 pass(es), 2 fail(s), and 0 exception(s). View
2.79 KB

This fixes some of them

Status: Needs review » Needs work

The last submitted patch, 37: 2520526-37.patch, failed testing.

The last submitted patch, 37: 2520526-37.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
14.52 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,180 pass(es). View
1.29 KB

Might fix the other 2.

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
@@ -416,6 +416,8 @@ public static function calculateDependencies(FieldDefinitionInterface $field_def
     $target_entity_type = $manager->getDefinition($field_definition->getFieldStorageDefinition()->getSetting('target_type'));
 
+    $dependencies['module'][] = $target_entity_type->getProvider();
+

+++ b/core/profiles/standard/config/install/field.storage.node.field_tags.yml
@@ -3,7 +3,6 @@ status: true
 dependencies:
   module:
     - node
-    - taxonomy

So one other problem is that I think that we're adding these dependencies on the wrong level - we're adding on the FieldConfig level and not the FieldStorageConfig level. Going to try and address this.

jibran’s picture

Added entity tags to alert team entity. Removed upgrade path and upgrade path test tags as we have them now.

alexpott’s picture

Status: Needs work » Needs review
FileSize
7.7 KB
18.21 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,147 pass(es), 11 fail(s), and 1 exception(s). View

Entity reference field storages now depend on the module that provides there target entity type. What makes lots of sense. The fields (instances) inherit this dependency because they are all dependent on their field storages (obviously).

Status: Needs review » Needs work

The last submitted patch, 43: 2520526-2-43.patch, failed testing.

The last submitted patch, 43: 2520526-2-43.patch, failed testing.

yched’s picture

Not familiar with the exact stakes of this patch, but the changes in the last interdiff in FieldItemInterface & EntityReferenceItem make total sense. We have FieldItemInterface::calculateDependencies() for the field config, there should definitely be an equivalent for the storage config.

dawehner’s picture

Oh yeah I totally think that this is how you expect it to be.

+++ b/core/modules/field/src/Entity/FieldStorageConfig.php
@@ -343,6 +343,16 @@ public function calculateDependencies() {
+    // Plugins can declare additional dependencies in their definition.
+    if (isset($definition['config_dependencies'])) {
+      $this->addDependencies($definition['config_dependencies']);
+    }

Is this something existing or did you invented this here ?

Looking at the failures for a while.

dawehner’s picture

Status: Needs work » Needs review
FileSize
21.26 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,164 pass(es). View
8.53 KB

I'm actually quite happy that Drupal\system\Tests\Installer\StandardInstallerTest failed ...

Here is a fix for the rest of the failures. Now the dependencies actually make more sense to be honest, like the tags field storage depending on taxonomy module.

Gábor Hojtsy’s picture

+++ b/core/lib/Drupal/Core/Field/FieldItemInterface.php
@@ -419,6 +419,39 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state);
+   * return an array of dependencies listing module that provides the entity

"dependencies listing module" does not make sense. I think it wanted to be "dependencies listing THE module"?

Gábor Hojtsy’s picture

Wrote change notice draft at https://www.drupal.org/node/2581375 for the new API addition. Found two more nits while writing it:

  1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
    @@ -446,6 +446,16 @@ public static function calculateDependencies(FieldDefinitionInterface $field_def
    +   * @inheritDoc
    

    {@inheritdoc}

  2. +++ b/core/modules/field/tests/src/Unit/FieldStorageConfigEntityUnitTest.php
    @@ -94,8 +107,37 @@ public function testCalculateDependencies() {
    +class TestFieldType extends FieldItemBase {
    

    Let's add one line phpdoc on what does this supposed to test IMHO.

nlisgo’s picture

  1. +++ b/core/modules/system/src/Tests/Entity/EntityReferenceFieldTest.php
    @@ -406,4 +407,35 @@ public function testTargetEntityNoLoad() {
    +          'handler' => 'default'
    

    comma needed at the end of the line.

dawehner’s picture

Thanks for the reviews, well someone please make a patch, I'm not necessarily online.

nlisgo’s picture

Assigned: Unassigned » nlisgo

I'll do it.

nlisgo’s picture

FileSize
2.11 KB
21.38 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,502 pass(es). View
nlisgo’s picture

Assigned: nlisgo » Unassigned

This addresses all but #50.1.

Because the class is not a test class but is only needed for FieldStorageConfigEntityUnitTest it is not appropriate to have @coversDefaultClass so could some who working on that provide an example doc please.

alexpott’s picture

FileSize
2.04 KB
20.95 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,561 pass(es). View

Added the missing documentation requested by #50 and removed the ability of the field type plugins 'config_dependencies' info to end up in the field storage configuration as it already ends up in the field configuration.

claudiu.cristea’s picture

FileSize
20.99 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,498 pass(es). View
3.57 KB

More nits.

catch’s picture

  1. +++ b/core/lib/Drupal/Core/Field/FieldItemInterface.php
    @@ -419,6 +419,39 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state);
    +   * Dependencies added here affect the storage and therefore are always
    

    field storage or other storage?

  2. +++ b/core/modules/book/config/install/core.entity_view_mode.node.print.yml
    @@ -1,11 +1,11 @@
     dependencies:
    -  module:
    -    - node
       enforced:
         module:
           - book
    +  module:
    +    - node
    

    Why the ordering change?

  3. +++ b/core/modules/system/system.post_update.php
    @@ -31,5 +34,30 @@ function system_post_update_fix_enforced_dependencies() {
    +  // Loads scan all module install folders and resave the entities in there.
    

    Loads scan all?

    Also what about optional config that doesn't actually depend on the module, then that module got uninstalled but the config stuck around?

Two nits one actual question.

alexpott’s picture

Assigned: Unassigned » alexpott
Status: Needs review » Needs work

Good questions and looking at the update function we're not dealing with optional config. Fixing that...

Wim Leers’s picture

#59: RE: optional config, I asked about that in #34.2. Which is why I think #34.1 is worth having/doing too: it makes the expectations of pre-update and post-update much clearer, hence preventing accidental regressions.

alexpott’s picture

Status: Needs work » Needs review
FileSize
21.94 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,512 pass(es). View
5.34 KB

re #58

  1. Fixed
  2. Because dependency keys are sorted
  3. Fixed.

I also fixed the fact we're weren't checking optional and uninstalled modules configuration. Thinking about it what happens if the module code no longer exists because the user removed it. I think the only thing we can do it save all configuration entities again. Which then makes me think we should batch this :(

alexpott’s picture

FileSize
2.07 KB
21.81 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,218 pass(es), 428 fail(s), and 22 exception(s). View

Using a batch and re-saving everything.

dawehner’s picture

  1. +++ b/core/modules/system/src/Tests/Update/RecalculatedDependencyTest.php
    @@ -27,8 +27,28 @@ protected function setDatabaseDumpFiles() {
    +
    +    // Explicitly break an optional configuration dependencies to ensure it is
    +    // recalculated.
    +    \Drupal::configFactory()->getEditable('search.page.node_search')->clear('dependencies')->save();
    +
    

    Urgs, it feels just bad to execute code before the update. Do we really need that and can't just manipulate the objects in the DB directly?

  2. +++ b/core/modules/system/system.post_update.php
    @@ -36,24 +36,26 @@ function system_post_update_fix_enforced_dependencies() {
    +  $install_storage = new InstallStorage();
    +  $config_names = $install_storage->listAll();
    +  $install_storage = new InstallStorage(InstallStorage::CONFIG_OPTIONAL_DIRECTORY);
    +  $config_names = array_merge($config_names, $install_storage->listAll());
    

    Maybe some docs about why we want to scan optional config as well would be nice

YesCT’s picture

  1. +++ b/core/modules/system/system.post_update.php
    @@ -31,5 +31,38 @@ function system_post_update_fix_enforced_dependencies() {
    + * Drupal did not calculate dependencies on install, so recalculate them by
    + * saving all matching configuration entities.
    

    I didnt think we wrote comments talking about what used to happen. I know this is in an update though...

  2. +++ b/core/modules/system/system.post_update.php
    @@ -31,5 +31,38 @@ function system_post_update_fix_enforced_dependencies() {
    + * @see https://www.drupal.org/node/2520526
    

    and I dont think we reference the issue that is changing code. that is what blame is for eh?

Status: Needs review » Needs work

The last submitted patch, 62: 2520526-2-62.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
4.53 KB
23.3 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 116,716 pass(es). View

We can remove system_post_update_fix_enforced_dependencies() because all config entities are going to be re-saved anyway.

Batches and the post update thing don't exactly work as expected atm - it records the same post update multiple times.

Fixed the issues raised by @YesCT

The last submitted patch, 62: 2520526-2-62.patch, failed testing.

alexpott’s picture

Just realised we could also remove the entire core/modules/field/field.post_update.php... as these are updates that just resave config entities...

alexpott’s picture

cosmicdreams’s picture

Well for what it's worth the patch looks good to me. The only quibble I could find was that EntityReferenceFieldTest and UpdatePostUpdateTest didn't have any @covers directives. But then I noticed they weren't unit tests so I guess that's alright.

RTBC++ from me.

cosmicdreams’s picture

Status: Needs review » Reviewed & tested by the community

I will be bold.

effulgentsia’s picture

This looks great to me as well. I'm about to commit it. Ticking credit boxes for everyone who participated.

effulgentsia’s picture

Status: Reviewed & tested by the community » Fixed

And pushed!

  • effulgentsia committed e1984e6 on 8.0.x
    Issue #2520526 by alexpott, dawehner, pwolanin, nlisgo, Eli-T, claudiu....
effulgentsia’s picture

FYI: One of the ways I manually tested this was installing 2 Standard Profile sites: one from beta-16 and then updating to HEAD post commit of this patch, and another by installing straight to HEAD post commit of this patch. Then I exported the full configuration of each site and diffed them, and all dependencies were the same.

dawehner’s picture

@effulgentsia
You would not have had to do that. The \Drupal\system\Tests\Installer\StandardInstallerTest is doing exactly that, and did, see some of the failures earlier.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.