Make Automatic Updates Drupal 10-compatible

https://www.drupal.org/pift-ci-job/2504642

The fact that there is a merge request to make this work on Drupal 10 says that the answer to the current title is already known: "no" 🤓

I think the scope of this issue is to make it compatible with Drupal 10? If so, retitling.

In fact, Automatic Updates will never land as experimental in 9.x, so this is de facto a blocker to get it into core at all. I think the scope of this issue is to also update the automated testing config at https://www.drupal.org/node/2997874/qa to default to Drupal 10?

One less failure if all went well 🤞 (My very first code contribution to AU 🤓)

One less failure! I'll continue this on Thursday (I'm off Tue + Wed).

WTF.

phpcs on 9.5:

FILE: ...ontrib/automatic_updates/tests/src/Functional/UpdatePathTest.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 20 | ERROR | [x] Array indentation error, expected 6 spaces but
    |       |     found 8
 21 | ERROR | [x] Array indentation error, expected 6 spaces but
    |       |     found 8

versus on 10.0:

FILE: ...ontrib/automatic_updates/tests/src/Functional/UpdatePathTest.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 20 | ERROR | [x] Array indentation error, expected 8 spaces but
    |       |     found 6
 21 | ERROR | [x] Array indentation error, expected 8 spaces but
    |       |     found 6

🤯🤣💩

On DrupalCI:

Testing Drupal\Tests\automatic_updates\Unit\VersionPolicy\ForbidDowngradeTest
....                                                                4 / 4 (100%)

Time: 00:00.047, Memory: 6.00 MB

OK (4 tests, 4 assertions)

Remaining self deprecation notices (1)

  1x: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated
    1x in ForbidDowngradeTest::testDowngradeForbidden from Drupal\Tests\automatic_updates\Unit\VersionPolicy

Remaining direct deprecation notices (2)

  1x: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated
    1x in ForbidDowngradeTest::testDowngradeForbidden from Drupal\Tests\automatic_updates\Unit\VersionPolicy

  1x: version_compare(): Passing null to parameter #1 ($version1) of type string is deprecated
    1x in ForbidDowngradeTest::testDowngradeForbidden from Drupal\Tests\automatic_updates\Unit\VersionPolicy

… but none of those deprecated calls ever occur from within the automatic_updates codebase! 🤯

Running locally with PHP 7.4 I cannot reproduce that, but with PHP 8.1 I do:

/opt/homebrew/bin/php /private/var/folders/x1/mlh998q15c70vs7_y8s1mgqm0000gp/T/ide-phpunit.php --configuration /Users/wim.leers/core/core/phpunit.xml --filter "/(Drupal\\Tests\\automatic_updates\\Unit\\VersionPolicy\\ForbidDowngradeTest::testDowngradeForbidden)( .*)?$/" --test-suffix ForbidDowngradeTest.php /Users/wim.leers/core/modules/automatic_updates/tests/src/Unit/VersionPolicy
Testing started at 12:59 PM ...
PHPUnit 8.5.29 #StandWithUkraine

Testing /Users/wim.leers/core/modules/automatic_updates/tests/src/Unit/VersionPolicy
....                                                                4 / 4 (100%)

Time: 542 ms, Memory: 8.00 MB

OK (4 tests, 4 assertions)

Remaining self deprecation notices (1)

  1x: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated
    1x in IDE_PHPUnit_Loader::init

Remaining direct deprecation notices (2)

  1x: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated
    1x in IDE_PHPUnit_Loader::init

  1x: version_compare(): Passing null to parameter #1 ($version1) of type string is deprecated
    1x in IDE_PHPUnit_Loader::init

Process finished with exit code 1

but even after running composer run-script drupal-phpunit-upgrade-check to upgrade PHPUnit, I get:

/opt/homebrew/bin/php /private/var/folders/x1/mlh998q15c70vs7_y8s1mgqm0000gp/T/ide-phpunit.php --configuration /Users/wim.leers/core/core/phpunit.xml --filter "/(Drupal\\Tests\\automatic_updates\\Unit\\VersionPolicy\\ForbidDowngradeTest::testDowngradeForbidden)( .*)?$/" --test-suffix ForbidDowngradeTest.php /Users/wim.leers/core/modules/automatic_updates/tests/src/Unit/VersionPolicy
Testing started at 1:05 PM ...
PHPUnit 9.5.26 by Sebastian Bergmann and contributors.

Testing /Users/wim.leers/core/modules/automatic_updates/tests/src/Unit/VersionPolicy
....                                                                4 / 4 (100%)

Time: 00:00.014, Memory: 10.00 MB

OK (4 tests, 4 assertions)

Remaining self deprecation notices (1)

  1x: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated
    1x in IDE_PHPUnit_Loader::init

Remaining direct deprecation notices (2)

  1x: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated
    1x in IDE_PHPUnit_Loader::init

  1x: version_compare(): Passing null to parameter #1 ($version1) of type string is deprecated
    1x in IDE_PHPUnit_Loader::init

Process finished with exit code 1

So … something is definitely wrong here. I can reproduce the same problem, but now with the PHPStorm-generated ide-phpunit.php 🤔

But this reproduces it too:

php core/scripts/run-tests.sh --sqlite /tmp/test.sqlite --url http://core.test --module automatic_updates --types PHPUnit-Unit --verbose

or

$ php core/scripts/run-tests.sh --sqlite /tmp/test.sqlite --url http://core.test --verbose --class \\Drupal\\Tests\\automatic_updates\\Unit\\VersionPolicy\\ForbidDowngradeTest

Drupal test run
---------------

Tests to be run:
  - \Drupal\Tests\automatic_updates\Unit\VersionPolicy\ForbidDowngradeTest

Test run started:
  Thursday, November 3, 2022 - 12:11

Test summary
------------

\Drupal\Tests\automatic_updates\Unit\VersionPolicy\ForbidDow   0 passes   1 fails                            

Test run duration: 0 sec

Detailed test results
---------------------


---- \Drupal\Tests\automatic_updates\Unit\VersionPolicy\ForbidDowngradeTest ----


Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Fail      Other      phpunit-56.xml       0 \Drupal\Tests\automatic_updates\Uni
    PHPUnit Test failed to complete; Error: PHPUnit 9.5.26 by Sebastian
    Bergmann and contributors.
    
    Testing
    Drupal\Tests\automatic_updates\Unit\VersionPolicy\ForbidDowngradeTest
    ....                                                                4 / 4
    (100%)
    
    Time: 00:00.014, Memory: 10.00 MB
    
    OK (4 tests, 4 assertions)
    
    Remaining self deprecation notices (1)
    
      1x: htmlspecialchars(): Passing null to parameter #1 ($string) of type
    string is deprecated
        1x in ForbidDowngradeTest::testDowngradeForbidden from
    Drupal\Tests\automatic_updates\Unit\VersionPolicy
    
    Remaining direct deprecation notices (2)
    
      1x: strpos(): Passing null to parameter #1 ($haystack) of type string is
    deprecated
        1x in ForbidDowngradeTest::testDowngradeForbidden from
    Drupal\Tests\automatic_updates\Unit\VersionPolicy
    
      1x: version_compare(): Passing null to parameter #1 ($version1) of type
    string is deprecated
        1x in ForbidDowngradeTest::testDowngradeForbidden from
    Drupal\Tests\automatic_updates\Unit\VersionPolicy

Investigating further … 🕵️‍♀️

Root cause for the strpos() and version_compare(): https://github.com/composer/semver/issues/134 hardened against a PHP 8.1 deprecation in an incomplete/poor way — we need to fix our calls to the underlying API too. Done.

And … the same root cause for the htmlspecialchars() deprecation, which gets triggered by

$actual_errors = array_map('strval', $rule->validate($installed_version, $target_version, $available_releases));

… because @target_version is NULL for a formattable string.

This really just revealed a weakness/bug in \Drupal\automatic_updates\Validator\VersionPolicy\ForbidDowngrade::validate(), caused by \Drupal\automatic_updates\Validator\VersionPolicyValidator::getTargetVersion().

Testing /Users/wim.leers/core/modules/automatic_updates/tests/src/Functional
E                                                                   1 / 1 (100%)

Time: 00:37.094, Memory: 10.00 MB

There was 1 error:

1) Drupal\Tests\automatic_updates\Functional\UpdatePathTest::testUpdatePath
Exception: Warning: include(/Users/wim.leers/core/modules/aggregator/src/FeedStorage.php): Failed to open stream: No such file or directory
include()() (Line: 571)


/Users/wim.leers/core/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:49
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/Promise.php:204
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/Promise.php:153
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/TaskQueue.php:48
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/Promise.php:248
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/Promise.php:224
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/Promise.php:269
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/Promise.php:226
/Users/wim.leers/core/vendor/guzzlehttp/promises/src/Promise.php:62
/Users/wim.leers/core/vendor/guzzlehttp/guzzle/src/Client.php:182
/Users/wim.leers/core/core/tests/Drupal/Tests/DrupalTestBrowser.php:137
/Users/wim.leers/core/vendor/symfony/browser-kit/Client.php:404
/Users/wim.leers/core/vendor/friends-of-behat/mink-browserkit-driver/src/BrowserKitDriver.php:145
/Users/wim.leers/core/vendor/behat/mink/src/Session.php:148
/Users/wim.leers/core/core/tests/Drupal/Tests/UiHelperTrait.php:334
/Users/wim.leers/core/core/tests/Drupal/Tests/UpdatePathTestTrait.php:46
/Users/wim.leers/core/core/tests/Drupal/FunctionalTests/Update/UpdatePathTestBase.php:270
/Users/wim.leers/core/modules/automatic_updates/tests/src/Functional/UpdatePathTest.php:51
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

This is … nonsensical.

What isn't: \Drupal\Core\Update\UpdateRegistry::getPendingUpdateFunctions() returns [] on D10, but a list of 14 pending update functions on 9.5. 0d672b0 fixed that.

On UpdatePathTest, I'm seeing an integrity violation constraint being triggered by automatic_updates_update_9001()'s first rename() call, which is why the second key does not get renamed. How is this possible?!

Well … if you put a breakpoint in automatic_updates_cron() you can see that it gets invoked while running updates, heck, even just on visiting update.php for the very first time. 🙈

I did not dig deeper to figure out why this only happens on Drupal 10 (it could be as simple as "PHP 8.1"), but it's a fact that this is an upstream bug. Created #3318964: automated_cron should not run cron when visiting update.php.

9.5

array(2) {
  ["readiness_check_timestamp"]=>
  string(13) "i:1666285089;"
  ["readiness_validation_last_run"]=>
  string(938) "a:2:{i:0;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";N;s:11:"*messages";a:1:{i:0;s:93:"The active directory at "/Users/phen/Sites/drupal" contains symlinks, which is not supported.";}s:11:"*severity";i:2;}i:1;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:55:"Updating from Drupal @installed_version is not allowed.";s:12:"*arguments";a:1:{s:18:"@installed_version";s:9:"9.5.0-dev";}s:10:"*options";a:0:{}}s:11:"*messages";a:1:{i:0;O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:171:"Drupal cannot be automatically updated from the installed version, @installed_version, because automatic updates from a dev version to any other version are not supported.";s:12:"*arguments";a:1:{s:18:"@installed_version";s:9:"9.5.0-dev";}s:10:"*options";a:0:{}}}s:11:"*severity";i:2;}}"
}
array(2) {
  ["status_check_last_run"]=>
  string(2299) "a:4:{i:0;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:55:"Updating from Drupal @installed_version is not allowed.";s:12:"*arguments";a:1:{s:18:"@installed_version";s:9:"9.5.0-dev";}s:10:"*options";a:0:{}}s:11:"*messages";a:1:{i:0;O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:171:"Drupal cannot be automatically updated from the installed version, @installed_version, because automatic updates from a dev version to any other version are not supported.";s:12:"*arguments";a:1:{s:18:"@installed_version";s:9:"9.5.0-dev";}s:10:"*options";a:0:{}}}s:11:"*severity";i:2;}i:1;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";N;s:11:"*messages";a:1:{i:0;O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:130:"Some modules have database schema updates to install. You should run the database update script immediately.";s:12:"*arguments";a:1:{s:7:":update";s:35:"/subdirectory/update.php/update.php";}s:10:"*options";a:0:{}}}s:11:"*severity";i:2;}i:2;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:32:"The file system is not writable.";s:12:"*arguments";a:0:{}s:10:"*options";a:0:{}}s:11:"*messages";a:1:{i:0;O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:44:"The Drupal directory "@dir" is not writable.";s:12:"*arguments";a:1:{s:4:"@dir";s:13:"/var/www/html";}s:10:"*options";a:0:{}}}s:11:"*severity";i:2;}i:3;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";N;s:11:"*messages";a:1:{i:0;O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:109:"@message See the help page for information on how to resolve the problem.";s:12:"*arguments";a:2:{s:8:"@message";s:121:"The active directory at "/var/www/html" contains symlinks, which is not supported. The first one is "/var/www/html/html".";s:21:":package-manager-help";s:86:"/subdirectory/update.php/admin/help/package_manager#package-manager-faq-symlinks-found";}s:10:"*options";a:0:{}}}s:11:"*severity";i:2;}}"
  ["status_check_timestamp"]=>
  string(13) "i:1667497021;"
}

10

array(2) {
  ["readiness_check_timestamp"]=>
  string(13) "i:1666285089;"
  ["readiness_validation_last_run"]=>
  string(938) "a:2:{i:0;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";N;s:11:"*messages";a:1:{i:0;s:93:"The active directory at "/Users/phen/Sites/drupal" contains symlinks, which is not supported.";}s:11:"*severity";i:2;}i:1;O:39:"Drupal\package_manager\ValidationResult":3:{s:10:"*summary";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:55:"Updating from Drupal @installed_version is not allowed.";s:12:"*arguments";a:1:{s:18:"@installed_version";s:9:"9.5.0-dev";}s:10:"*options";a:0:{}}s:11:"*messages";a:1:{i:0;O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:171:"Drupal cannot be automatically updated from the installed version, @installed_version, because automatic updates from a dev version to any other version are not supported.";s:12:"*arguments";a:1:{s:18:"@installed_version";s:9:"9.5.0-dev";}s:10:"*options";a:0:{}}}s:11:"*severity";i:2;}}"
}
array(1) {
  ["status_check_timestamp"]=>
  string(13) "i:1666285089;"
}

… even though I can see that … automatic_updates_update_9001() did in fact rename both keys, the status_check_last_run key-value pair is obviously missing on D10 🤯

Figured it out.

This is caused by automatic_updates_post_update_create_status_check_mail_config() saving config, which triggers \Drupal\automatic_updates\EventSubscriber\ConfigSubscriber::onConfigSave(), which in turn triggers \Drupal\automatic_updates\Validation\StatusChecker::clearStoredResults().

I bet this is passing in 9.5 then only through sheer execution order coincidence: this post-update hook erases status_check_last_run, but then the cron execution recreates it, which allows the assertions in \Drupal\Tests\automatic_updates\Functional\UpdatePathTest::testUpdatePath() to pass. On D10, the execution order is different, which causes it to fail.

This is happening since #3316611: If unattended updates are enabled, send an email when status checks start failing. That issue is technically not testing the update path correctly, because it requires all update functions to be tested by \Drupal\Tests\automatic_updates\Functional\UpdatePathTest::testUpdatePath(). This is wrong, we need one update path test per update. EDIT: no, only Drupal core requires that.

Full sequence of events on 10.1:

1. upon visiting update.php, automatic_updates_cron() gets called, which calls \Drupal\automatic_updates\Validation\StatusChecker::run(), which sets status_check_last_run and status_check_timestamp — ⚠️ WRONG! → this de facto means there is no point to run this update path at all… we could instead just have it be wiped and it'd auto-heal after cron…
2. then automatic_updates_update_9001() gets executed. Renaming works fine, but now everything is in there TWICE already: the original and the renamed. Which means that the rename() calls both trigger an integrity constraint violation which gets eaten by \Drupal\Core\KeyValueStore\KeyValueStoreInterface::rename() — ⚠️ now the same data is in there twice, and our update path silently didn't do anything at all, leaving the system in an inconsistent state
3. automatic_updates_post_update_create_status_check_mail_config() gets executed, triggering ConfigSubscriber::onConfigSave(), which in turn triggers StatusChecker::clearStoredResults(), meaning status_check_last_run is removed, hence causing the assertions to fail — ⚠️ this means that it's logically impossible for BOTH of the update path tests' results to be true! 🤯😱🤔

4.     foreach ($map as $new_key) {
         $this->assertNotEmpty($key_value->get($new_key));
       }
   

   fails

Full sequence of events on 9.5:

1. Same.
2. Same.
3. Same.
4. system_post_update_enable_provider_database_driver() gets invoked, which does \Drupal::service('module_installer')->install(array_keys($modules_to_install));, which triggers automatic_updates_modules_installed(), which in turn triggers StatusChecker::run(), which overwrites status_check_last_run and status_check_timestamp again
5. … which is why
   

       foreach ($map as $new_key) {
         $this->assertNotEmpty($key_value->get($new_key));
       }
   

   passes!

IOW: this update path test was not at all testing the update path…

I've got this green locally, but d.o's GitLab instance is currently experiencing severe issues, so I can't push them… 😅

Well, #21 is pretty mind-boggling. Had I tried to debug this, without a doubt it would have melted by brain. I can only humbly tip my hat in your direction, @Wim Leers, and quietly aspire to someday reach even 1% of your skill level.

this de facto means there is no point to run this update path at all

This is technically true; the ephemeral nature of status check caching means the update path was not strictly necessary. However, had we not done that, until status checks were run again people would have seen messages like "You haven't run readiness checks recently", which I figured would be confusing. Not to mention the fact that, since status checks could potentially be expensive operations, we don't want to run them more often than we need to. In retrospect, though, we could have just cleared the stored results and flagged a message saying "Please run readiness checks again", with a link.

I guess the lesson here is that, in most situations, it's not worth trying to seamlessly update what is essentially a cache.

What's also so frustrating about this bug is that it works, albeit by coincidence, in Drupal 9. Which, until this issue lands, is all we test with. So there was no real way for us to see this Lovecraftian betentacled horror show of a bug coming down the road.

When this is pushed, I will review it and likely RTBC.

Yay! That proves it:

9.5

1) Drupal\Tests\automatic_updates\Functional\UpdatePathTest::testUpdatePath
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'status_check_last_run' => Array (
         0 => Drupal\package_manager\ValidationResult Object (
-            'summary' => null
+            'summary' => Drupal\Core\StringTranslation\TranslatableMarkup Object (...)
             'messages' => Array (
-                0 => 'The active directory at "/Use...orted.'
+                0 => Drupal\Core\StringTranslation\TranslatableMarkup Object (...)
             )
             'severity' => 2
         )
         1 => Drupal\package_manager\ValidationResult Object (
-            'summary' => Drupal\Core\StringTranslation\TranslatableMarkup Object (...)
+            'summary' => null
             'messages' => Array (
                 0 => Drupal\Core\StringTranslation\TranslatableMarkup Object (
                     'translatedMarkup' => null
                     'options' => Array ()
                     'stringTranslation' => null
-                    'string' => 'Drupal cannot be automatically updated from the installed version, @installed_version, because automatic updates from a dev version to any other version are not supported.'
+                    'string' => 'Some modules have database schema updates to install. You should run the database update script immediately.'
                     'arguments' => Array (
-                        '@installed_version' => '9.5.0-dev'
+                        ':update' => '/subdirectory/update.php/update.php'
                     )
                 )
             )
             'severity' => 2
         )
+        2 => Drupal\package_manager\ValidationResult Object (...)
+        3 => Drupal\package_manager\ValidationResult Object (...)
     )
-    'status_check_timestamp' => 1666285089
+    'status_check_timestamp' => 1667809617
 )

10

1) Drupal\Tests\automatic_updates\Functional\UpdatePathTest::testUpdatePath
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    'status_check_last_run' => Array (...)
-    'status_check_timestamp' => 1666285089
+    'status_check_timestamp' => 1667809706
 )

Green! On 10.0.0-beta2 … 😬 It was green on 10.0.x HEAD last Friday. I bet that it's the Symfony 6.2 beta update … 😔

Commit history:

* 921ac3acdc9c14724332e31633abac506ca6aa06 Issue #3284422 by catch, phenaproxima, Spokje, andypost, longwave, Gábor Hojtsy, mondrake: [META] Symfony 6.2 compatibility
* 692d7febc29e3fb5ea7a6cdc07945b883dac61f1 Issue #3317879 by longwave, pooja saraah, andypost, nod_: Remove Chromedriver as a JavaScript dependency

It's green on the bottom commit (the one before #3284422: [META] Symfony 6.2 compatibility) and red after.

😬

InvalidArgumentException: Service ID names must be lowercase: PhpTuf\ComposerStager\Domain\Exception

/Users/wim.leers/core/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php:55
/Users/wim.leers/core/vendor/symfony/dependency-injection/Loader/FileLoader.php:258
/Users/wim.leers/core/vendor/symfony/dependency-injection/Loader/FileLoader.php:113
/Users/wim.leers/core/modules/automatic_updates/package_manager/src/PackageManagerServiceProvider.php:43
[…]

https://github.com/symfony/symfony/commit/739789f384cca3b06a3b5f35793e11... introduced

        if (null !== $prefixLen) {
            $attributes = null !== $source ? ['source' => sprintf('in "%s/%s"', basename(\dirname($source)), basename($source))] : [];

            foreach ($excludePaths as $path => $_) {
                $class = $namespace.ltrim(str_replace('/', '\\', substr($path, $prefixLen, str_ends_with($path, '.php') ? -4 : null)), '\\');
                if (!$this->container->has($class)) {
                    $this->container->register($class)
                        ->setAbstract(true)
                        ->addTag('container.excluded', $attributes);
                }
            }
        }

in FileLoader. That register($class) call passes in PhpTuf\ComposerStager\Domain\Exception, which then triggers

  public function register($id, $class = NULL): Definition {
    if (strtolower($id) !== $id) {
      throw new \InvalidArgumentException("Service ID names must be lowercase: $id");
    }

… which understandably triggers this exception.

Turns out Drupal's validation in ContainerBuilder is just … not working and that's why #3021803: Document and add tests for service autowiring didn't hit it …

Asked @longwave and @alexpott at https://drupal.slack.com/archives/C1BMUQ9U6/p1668013246481129, also because of #3049525: Enable service autowiring by adding interface aliases to core service definitions, which is introducing boatloads of non-lowercase service IDs AFAICT!

@longwave in Drupal Slack:

yeah i picked up on this a while ago, i think we have to drop that restriction now we have autowiring #2503567-49: Only allow lowercase service and parameter names [part 2] 😬

Closed #2503567 at #2503567-51: Only allow lowercase service and parameter names [part 2].

Pushed #3049525 forward at #3049525-53: Enable service autowiring by adding interface aliases to core service definitions.

But … we'll still need a work-around until the time that #3049525 lands. On it.

Wow, #3320315: Allow uppercase service IDs/names — necessary for autowiring support landed, so I can revert all the shenanigans of the last 4 commits!

looks like there are already 9 other uses of this in AU

Core brings in symfony/polyfill-php80, so it's safe for us to use the new str_* functions in PHP 8.

Apart from the one nit about using ??=, I think this is ready. I know I wrote a bunch of it, but we gotta get it done. I assume @tedbow will give it a once-over before committing it. Besides, @Wim Leers did the incredibly impressive heavy lifting in debugging this one, particularly the craziness around the update path test. I can't thank you enough for tracing through that thicket of madness.

I discovered while trying to test this that setup_local_dev.sh has a flaw which prevents the set-up from completing on D10: it configures Composer's PHP platform as 7.4.0, but D10 requires PHP 8.1.0. After discussion with @tedbow, I don't think it needs to block commit. Instead, I leave it to @TravisCarden to sort that out in another issue, which should probably be opened before this is committed since it blocks local development on Drupal 10. (IMHO, the best solution would be to run composer config platform.php against the core checkout, to find out what it wants to use as the PHP version, and then propagate that into the local project.) Additionally, Automatic Updates can remove the whole config section from composer.json, because it serves no purpose.

I discovered while trying to test this that setup_local_dev.sh has a flaw which prevents the set-up from completing on D10: it configures Composer's PHP platform as 7.4.0, but D10 requires PHP 8.1.0.

Yep, that's literally the first thing I ran into after git cloneing AU 😅

Additionally, Automatic Updates can remove the whole config section from composer.json, because it serves no purpose.

I have argued in other contexts that setting platform.php in the first place is usually a bad idea, and this problem illustrates why. The only reason setup_local_dev.sh sets it to 7.4 is that drupal/drupal has it at 7.3.

We should probably change the constraint to >=7.4 or remove it altogether. I've created #3320486: Make setup/install scripts remove "platform.php" Composer config override altogether to remove it. It's ready for review.

Core brings in symfony/polyfill-php80, so it's safe for us to use the new str_* functions in PHP 8.

If we're explicitly using an indirect dependency, it's really a direct dependency. And we should composer require symfony/polyfill-php80 it in our own module. Of course, I assume it will go away when we commit the module to core, but apparently its absence causes confusion (exhibit one: this thread), and it's technically "correct".

EDIT: Redacted