In order to add an enforced dependency we duplicate information in the dependencies key.

core/modules/book/config/install/node.type.book.yml

 dependencies:
  module:
    - book
   enforced:
     module:
       - book

Removing this duplication make config dependencies cleaner and easier to understand. Also separating the concerns of calculating and get the list of dependencies makes the API simpler to change and maintain.

This is made worse by the fact that we don't recalculate dependencies during install - see #2520526: Calculate configuration entity dependencies on install

Files: 
CommentFileSizeAuthor
#18 2520540-18.patch52.13 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2520540-18.patch. Unable to apply patch. See the log in the details link for more information. View
#17 2520540-16.patch93.08 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Setup environment: Test cancelled by admin prior to completion. View
#17 11-16-interdiff.txt713 bytesalexpott
#11 2520540-11.patch51.43 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2520540-11.patch. Unable to apply patch. See the log in the details link for more information. View
#11 2520540-11.test-only.patch2.15 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2520540-11.test-only.patch. Unable to apply patch. See the log in the details link for more information. View
#9 2520540-9.patch49.28 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,655 pass(es). View
#9 2-9-interdiff.txt1.04 KBalexpott
#2 2520540-2.patch50.1 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,137 pass(es). View
#1 2520540.1.patch49.39 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 97,233 pass(es). View

Comments

alexpott’s picture

Status: Active » Needs review
FileSize
49.39 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 97,233 pass(es). View
alexpott’s picture

Issue summary: View changes
FileSize
50.1 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,137 pass(es). View

Rerolled.

Status: Needs review » Needs work

The last submitted patch, 2: 2520540-2.patch, failed testing.

The last submitted patch, 2: 2520540-2.patch, failed testing.

Berdir’s picture

As discussed, it would be great if we could check the return value in the one call to calculateDependencies() that really counts and throw a helpful exception about what changed and what you need to update. Then fixing it will be easy, not too worried about that.

I counted around 5 modules/project that this breaks that I'm using.

Wim Leers’s picture

Issue tags: +rc target
alexpott’s picture

Issue tags: -rc target +rc deadline
Wim Leers’s picture

As discussed, it would be great if we could check the return value in the one call to calculateDependencies() that really counts and throw a helpful exception about what changed and what you need to update.

AFAICT once this is done, this is RTBC.

alexpott’s picture

Status: Needs work » Needs review
FileSize
1.04 KB
49.28 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,655 pass(es). View

Actually it is possible to do this in a way that does not even break BC at the point that really matters :)

alexpott’s picture

Status: Needs review » Needs work

Needs an upgrade path.

alexpott’s picture

Status: Needs work » Needs review
FileSize
2.15 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2520540-11.test-only.patch. Unable to apply patch. See the log in the details link for more information. View
51.43 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2520540-11.patch. Unable to apply patch. See the log in the details link for more information. View

Here's an upgrade path and a test for it.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/system/src/Tests/Update/UpdatePathTestBaseFilledTest.php
@@ -415,6 +416,12 @@ public function testUpdatedSite() {
+    // Ensure that the Book module's node type does not have duplicated enforced
+    // dependencies.
+    // @see system_post_update_fix_enforced_dependencies()
+    $book_node_type = NodeType::load('book');
+    $this->assertEqual(['enforced' => ['module' => ['book']]], $book_node_type->get('dependencies'));

Book module being tested in a System module test… smells fishy. But apparently this is pre-existing.

(This really belongs in Standard AFAICT.)

alexpott’s picture

@Wim Leers that is testing the update function provided the system module so I think it is acceptable.

The last submitted patch, 11: 2520540-11.test-only.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2520540-11.patch, failed testing.

The last submitted patch, 9: 2520540-9.patch, failed testing.

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
713 bytes
93.08 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Setup environment: Test cancelled by admin prior to completion. View

Wow that test sucks.

alexpott’s picture

FileSize
52.13 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2520540-18.patch. Unable to apply patch. See the log in the details link for more information. View

Interdiff is correct just rerolled against the wrong thing.

The last submitted patch, 17: 2520540-16.patch, failed testing.

Wim Leers’s picture

Wow that test sucks.

+1, been bit by that too :P

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 6e0225e on 8.0.x
    Issue #2520540 by alexpott: Enforced configuration dependencies shouldn'...
yched’s picture

That test is super painful indeed :-/
(and #2578249: Some e_r fields get the wrong Selection handler, that has to do the same, will need a reroll :-p)

The last submitted patch, 11: 2520540-11.test-only.patch, failed testing.

The last submitted patch, 11: 2520540-11.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 18: 2520540-18.patch, failed testing.

alexpott’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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