Problem/Motivation

The function has few usages in core and contrib also it extra cost from API maintenance POV, extra function call

Steps to reproduce

 git grep drupal_theme_rebuild
core/includes/theme.inc:122:function drupal_theme_rebuild() {
core/lib/Drupal/Core/Extension/ModuleInstaller.php:340:        drupal_theme_rebuild();
core/lib/Drupal/Core/Extension/ModuleInstaller.php:520:      drupal_theme_rebuild();
core/lib/Drupal/Core/Extension/ThemeInstaller.php:314:   * Wraps drupal_theme_rebuild().
core/lib/Drupal/Core/Extension/ThemeInstaller.php:317:    drupal_theme_rebuild();
core/modules/image/src/Entity/ImageStyle.php:302:    drupal_theme_rebuild();
core/modules/system/tests/modules/theme_test/src/EventSubscriber/ThemeTestSubscriber.php:62:      drupal_theme_rebuild();
core/modules/system/tests/src/Functional/Theme/ThemeTest.php:45:    drupal_theme_rebuild();
core/modules/system/tests/src/Functional/Theme/ThemeTest.php:181:    drupal_theme_rebuild();

Proposed resolution

deprecate and inject theme.registry service where possible

Remaining tasks

- patch/review/commit

User interface changes

no

API changes

drupal_theme_rebuild() deprecated

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3348851-3.patch6.57 KBandypost
#5 interdiff.txt4.59 KBandypost
#2 3348851-2.patch1.98 KBandypost

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 3348851-2.patch1.98 KB

Deprecation and test

CR https://www.drupal.org/node/3348853

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#3348848: Deprecate theme_get_registry()
andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: #2957440: Refactor ThemeRegistry and Theme\Registry » #3097045: [META] Provide modern replacements for and deprecate the legacy include files
Related issues: +#2957440: Refactor ThemeRegistry and Theme\Registry
andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt4.59 KB
new 3348851-3.patch6.57 KB

replace usage, not sure about removal of protected method as BC but now it needs to try inject registry

The last submitted patch, 2: 3348851-2.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes
andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +@deprecated
andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Kill includes
daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

All code changes look good to me.
Deprecation message testing has been added.
The testbot return green.
The IS and the CR are in order.
For me it is RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3348851-3.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

re-queued as can't reproduce locally

Drupal\Tests\user\Kernel\Migrate\MigrateUserStubTest

fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-1178.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\user\Kernel\Migrate\MigrateUserStubTest
F                                                                   1 / 1 (100%)

Time: 00:01.528, Memory: 4.00 MB

There was 1 failure:

1) Drupal\Tests\user\Kernel\Migrate\MigrateUserStubTest::testStub
Failed asserting that an object is empty.

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/var/www/html/core/modules/migrate_drupal/src/Tests/StubTestTrait.php:22
/var/www/html/core/modules/user/tests/src/Kernel/Migrate/MigrateUserStubTest.php:35
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

FAILURES!
Tests: 1, Assertions: 8, Failures: 1.

like sequences table is not installed

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community
purencool’s picture

purencool commented

Applying the 3348851-3 patch produced the following results locally.

$ git branch
* 10.1.x

Before patch was applied

$  git grep drupal_theme_rebuild                                                                    
core/includes/theme.inc:function drupal_theme_rebuild() {
core/lib/Drupal/Core/Extension/ModuleInstaller.php:        drupal_theme_rebuild();
core/lib/Drupal/Core/Extension/ModuleInstaller.php:      drupal_theme_rebuild();
core/lib/Drupal/Core/Extension/ThemeInstaller.php:   * Wraps drupal_theme_rebuild().
core/lib/Drupal/Core/Extension/ThemeInstaller.php:    drupal_theme_rebuild();
core/modules/image/src/Entity/ImageStyle.php:    drupal_theme_rebuild();
core/modules/system/tests/modules/theme_test/src/EventSubscriber/ThemeTestSubscriber.php:      drupal_theme_rebuild();
core/modules/system/tests/src/Functional/Theme/ThemeTest.php:    drupal_theme_rebuild();
core/modules/system/tests/src/Functional/Theme/ThemeTest.php:    drupal_theme_rebuild();

After patch was applied

$  git grep drupal_theme_rebuild 
includes/theme.inc:function drupal_theme_rebuild() {
tests/Drupal/KernelTests/Core/Theme/RegistryTest.php:   * Tests deprecated drupal_theme_rebuild() function.
tests/Drupal/KernelTests/Core/Theme/RegistryTest.php:   * @see drupal_theme_rebuild()
tests/Drupal/KernelTests/Core/Theme/RegistryTest.php:    $this->expectDeprecation('drupal_theme_rebuild() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use theme.registry service reset() method instead. See https://www.drupal.org/node/3348853');
tests/Drupal/KernelTests/Core/Theme/RegistryTest.php:    drupal_theme_rebuild();

PHPUnit test result

$ ../vendor/bin/phpunit --filter testLegacyThemeRegistryRebuild ./tests/Drupal/KernelTests/Core/Theme/RegistryTest.php

PHPUnit 9.6.5 by Sebastian Bergmann and contributors.

Testing Drupal\KernelTests\Core\Theme\RegistryTest
.                                                                   1 / 1 (100%)

Time: 00:01.667, Memory: 8.00 MB

OK (1 test, 3 assertions)
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 8184e39 and pushed to 10.1.x. Thanks!

  • catch committed 8184e399 on 10.1.x 
    Issue #3348851 by andypost: Deprecate drupal_theme_rebuild()
catch’s picture

catch
he/him
Primary language English
commented
andypost’s picture

andypost
he/him
Primary language Russian
commented

Filed follow-up #3350904: Properly inject the theme registry in the ThemeInstaller

Status: Fixed » Closed (fixed)

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