Deprecate system_get_info()

It looks great clean-up! Why it needs to update https://www.drupal.org/node/2709919 instead of creating own one?
It becomes hard to track changes (deprecations) when CRs are updated

+++ b/core/includes/install.inc
@@ -108,7 +108,7 @@ function drupal_install_profile_distribution_name() {
+    $info = \Drupal::service('extension.list.module')->getExtensionInfo($profile);

@@ -133,7 +133,7 @@ function drupal_install_profile_distribution_version() {
+    $info = \Drupal::service('extension.list.module')->getExtensionInfo($profile);

+++ b/core/modules/system/system.install
@@ -45,7 +45,7 @@ function system_requirements($phase) {
+      $info = \Drupal::service('extension.list.module')->getExtensionInfo($profile);

+++ b/core/profiles/demo_umami/demo_umami.install
@@ -15,7 +15,7 @@ function demo_umami_requirements($phase) {
+    $info = \Drupal::service('extension.list.module')->getExtensionInfo($profile);

that's a bit strange that module list used on profile but that's how core works?

that's a bit strange that module list used on profile but that's how core works?

@andypost, Both services are using the same parent method. However, in order to avoid such confusions let's use the profile service. It should work. I guess the 'module version' was inherited from the deprecated function.

The change record has been updated.

Fixed the test.

Just one minor

1. +++ b/core/lib/Drupal/Core/Menu/Form/MenuLinkDefaultForm.php
   @@ -49,13 +49,6 @@ class MenuLinkDefaultForm implements MenuLinkFormInterface, ContainerInjectionIn
   -  protected $moduleData;
   

   ++, unused here and in child classes

2. +++ b/core/modules/system/src/Controller/AdminController.php
   @@ -16,7 +42,7 @@ class AdminController extends ControllerBase {
   +    $module_info = $info = $this->moduleList->getAllInstalledInfo();
   

   looks like typo $info better to remove

Setting to NW per #11.

Fixing stuff.

Looks like some part of the deprecation was already done in #3035274: Deprecate drupal_classloader_register() & system_register()

Updated the patch. Please, review

1. +++ b/core/modules/system/tests/src/Kernel/System/SystemGetInfoTest.php
   @@ -17,8 +17,8 @@
   diff -u b/core/profiles/demo_umami/demo_umami.install b/core/profiles/demo_umami/demo_umami.install
   
   --- b/core/profiles/demo_umami/demo_umami.install
   +++ b/core/profiles/demo_umami/demo_umami.install
   @@ -15,7 +15,7 @@
      $requirements = [];
      if ($phase == 'runtime') {
        $profile = \Drupal::installProfile();
   -    $info = \Drupal::service('extension.list.profile')->getExtensionInfo($profile);
   +    $info = \Drupal::service('extension.list.module')->getExtensionInfo($profile);
        $requirements['experimental_profile_used'] = [
          'title' => t('Experimental installation profile used'),
          'value' => $info['name'],
   

   Not really sure about this change. Should we change service call from extention.list.profile to extention.list.module ? Code in this requirement definition operate with profile info only.

2. Also, all deprecation messages should be updated to fix the CS issues

Updated patch and interdiff.

Looks like Legacy tests need to update their @expectedDeprecation messages.

Re-roll of the patch

Needs reroll against the latest branch version.

Re-roll

1. +++ b/core/modules/help/src/Controller/HelpController.php
   @@ -35,10 +43,13 @@ class HelpController extends ControllerBase {
       *   The help section manager.
   +   * @param \Drupal\Core\Extension\ModuleExtensionList $module_list
   +   *   The module list.
       */
   -  public function __construct(RouteMatchInterface $route_match, HelpSectionManager $help_manager) {
   +  public function __construct(RouteMatchInterface $route_match, HelpSectionManager $help_manager, ModuleExtensionList $module_list) {
        $this->routeMatch = $route_match;
        $this->helpManager = $help_manager;
   +    $this->moduleList = $module_list;
   

   The argument needs to be optional, with a fallback to Drupal::service() and @trigger_error() when it is not set.

2. +++ b/core/modules/system/src/Controller/AdminController.php
   @@ -3,12 +3,38 @@
   +   * AdminController constructor.
   +   *
   +   * @param \Drupal\Core\Extension\ModuleExtensionList $module_list
   +   *   The module extension list service.
   +   */
   +  public function __construct(ModuleExtensionList $module_list) {
   +    $this->moduleList = $module_list;
   +  }
   

   A new construct method is especially tricky, because a potential subclass doesn't call it yet and then even the fallback wouldn't work.

   Not sure if it's worth here, subclasses seem unlikely. But we could ad d a private method to get the service and do the fallback there.

Hi @berdir,

1. Agree.
2. It won't work. if there is controller that extends AdminController and has own constructor there will be fatal error like this:
ArgumentCountError: Too few arguments to function TestAdminController::__construct(), 0 passed in /var/www/web/core/lib/Drupal/Core/Controller/ControllerBase.php on line 118 and exactly 1 expected in Drupal\test_stuff\Controller\TestAdminController->__construct()

In any case, if there are some controllers those extend AdminController their constructor should be fixed.

Set argument as optional

I think setter here is not needed, admin controller is supposed to know enabled module list
The only thing worry me is naming here - @dawehner already mentioned in other issue is "extension_list_module" vs "moduleExtensionList"
Better to check the naming in other places where "module list" injected already

+++ b/core/modules/system/src/Controller/AdminController.php
@@ -3,12 +3,47 @@
+   * @todo Remove null default value in Drupal 9.0.0.
+   */
+  public function __construct(ModuleExtensionList $extension_list_module = NULL) {

better to remove it, for 9.x it will be cleaned by trigger_error

1. +++ b/core/lib/Drupal/Core/Menu/Form/MenuLinkDefaultForm.php
   @@ -66,7 +59,7 @@ class MenuLinkDefaultForm implements MenuLinkFormInterface, ContainerInjectionIn
   -   *   The module handler;
   +   *   The module handler.
   

   This is out-of scope.

2. +++ b/core/modules/system/system.module
   @@ -990,13 +996,13 @@ function system_get_info($type, $name = NULL) {
   - * @deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. This
   + * @deprecated in drupal:8.5.0 and is removed from drupal:9.0.0. This
     *   function is no longer used in Drupal core.
     *
     * @see https://www.drupal.org/node/2709919
     */
    function _system_rebuild_module_data_ensure_required($module, &$modules) {
   -  @trigger_error("_system_rebuild_module_data_ensure_required() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. This function is no longer used in Drupal core. See https://www.drupal.org/node/2709919", E_USER_DEPRECATED);
   +  @trigger_error("_system_rebuild_module_data_ensure_required() is deprecated in drupal:8.5.0 and is removed from drupal:9.0.0. This function is no longer used in Drupal core. See https://www.drupal.org/node/2709919", E_USER_DEPRECATED);
   

   Fixing the deprecation message format for other functions is out of scope here.

I agree with @andypost - controller constructors are best effort BC - they are not covered by our BC promise.

Rerolled and addressed comments

The other function already deprecated, i think it ready

+++ b/core/lib/Drupal/Core/Menu/Form/MenuLinkDefaultForm.php
@@ -49,13 +49,6 @@ class MenuLinkDefaultForm implements MenuLinkFormInterface, ContainerInjectionIn
 
-  /**
-   * The module data from system_get_info().
-   *
-   * @var array
-   */
-  protected $moduleData;
-

Had to double check whether this was OK to remove, but it's dead code already from a previous change.

Committed 5be2df4 and pushed to 8.8.x. Thanks!