Issue #3258045 by phenaproxima: Ensure update command in Stage::require() only...

Issue #3258045 by phenaproxima: Ensure update command in Stage::require() only updates specified packages

package_manager/src/ComposerUtility.php

@@ -128,23 +128,6 @@ class ComposerUtility {
     return array_values($core_packages);
   }
 
-  /**
-   * Returns the names of the core packages in the dev dependencies.
-   *
-   * All packages listed in ../core_packages.json are considered core packages.
-   *
-   * @return string[]
-   *   The names of the core packages in the dev requirements.
-   *
-   * @todo Make this return a keyed array of packages, not just names in
-   *   https://www.drupal.org/i/3258059.
-   */
-  public function getCoreDevPackageNames(): array {
-    $dev_packages = $this->composer->getPackage()->getDevRequires();
-    $dev_packages = array_keys($dev_packages);
-    return array_intersect(static::getCorePackageList(), $dev_packages);
-  }
-
   /**
    * Returns all Drupal extension packages in the lock file.
    *
@@ -174,7 +157,7 @@ class ComposerUtility {
    *   All packages in the lock file, keyed by name.
    */
   protected function getLockedPackages(): array {
-    $locked_packages = $this->composer->getLocker()
+    $locked_packages = $this->getComposer()->getLocker()
       ->getLockedRepository(TRUE)
       ->getPackages();
 

package_manager/src/Stage.php

@@ -231,28 +231,38 @@ class Stage {
   }
 
   /**
-   * Requires packages in the staging area.
-   *
-   * @param string[] $constraints
-   *   The packages to require, in the form 'vendor/name:version'.
-   * @param bool $dev
-   *   (optional) Whether the packages should be required as dev dependencies.
-   *   Defaults to FALSE.
+   * Adds or updates packages in the staging area.
+   *
+   * @param string[] $runtime
+   *   The packages to add as regular top-level dependencies, in the form
+   *   'vendor/name:version'.
+   * @param string[] $dev
+   *   (optional) The packages to add as dev dependencies, in the form
+   *   'vendor/name:version'. Defaults to an empty array.
    */
-  public function require(array $constraints, bool $dev = FALSE): void {
+  public function require(array $runtime, array $dev = []): void {
     $this->checkOwnership();
 
-    $command = array_merge(['require', '--no-update'], $constraints);
+    $this->dispatch(new PreRequireEvent($this));
+    $dir = $this->getStageDirectory();
+
+    // Change the runtime and dev requirements as needed, but don't update
+    // the installed packages yet.
+    if ($runtime) {
+      $command = array_merge(['require', '--no-update'], $runtime);
+      $this->stager->stage($command, $dir);
+    }
     if ($dev) {
-      $command[] = '--dev';
+      $command = array_merge(['require', '--dev', '--no-update'], $dev);
+      $this->stager->stage($command, $dir);
+    }
+
+    // If constraints were changed, update those packages.
+    if ($runtime || $dev) {
+      $command = array_merge(['update', '--with-all-dependencies'], $runtime, $dev);
+      $this->stager->stage($command, $dir);
     }
 
-    $this->dispatch(new PreRequireEvent($this));
-    $dir = $this->getStageDirectory();
-    $this->stager->stage($command, $dir);
-    // @todo Limit the update command to only packages in $constraints in
-    //   https://www.drupal.org/i/3257849.
-    $this->stager->stage(['update', '--with-all-dependencies'], $dir);
     $this->dispatch(new PostRequireEvent($this));
   }
 

src/Updater.php

@@ -35,11 +35,12 @@ class Updater extends Stage {
       'dev' => [],
     ];
 
+    $require_dev = $composer->getComposer()
+      ->getPackage()
+      ->getDevRequires();
     foreach ($composer->getCorePackageNames() as $package) {
-      $package_versions['production'][$package] = $project_versions['drupal'];
-    }
-    foreach ($composer->getCoreDevPackageNames() as $package) {
-      $package_versions['dev'][$package] = $project_versions['drupal'];
+      $group = array_key_exists($package, $require_dev) ? 'dev' : 'production';
+      $package_versions[$group][$package] = $project_versions['drupal'];
     }
 
     // Ensure that package versions are available to pre-create event
@@ -79,12 +80,7 @@ class Updater extends Stage {
       return $requirements;
     };
     $versions = array_map($map, $this->getPackageVersions());
-
-    $this->require($versions['production']);
-
-    if ($versions['dev']) {
-      $this->require($versions['dev'], TRUE);
-    }
+    $this->require($versions['production'], $versions['dev']);
   }
 
   /**

tests/fixtures/fake-site/composer.lock

@@ -19,5 +19,10 @@
             "version": "9.8.0"
         }
     ],
-    "packages-dev": []
+    "packages-dev": [
+        {
+            "name": "drupal/core-dev",
+            "version": "9.8.0"
+        }
+    ]
 }

tests/src/Kernel/UpdaterTest.php

@@ -57,7 +57,7 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
     $id = $this->container->get('automatic_updates.updater')->begin([
       'drupal' => '9.8.1',
     ]);
-    // Rebuild the container to ensure the project versions are kept in state.
+    // Rebuild the container to ensure the package versions are persisted.
     /** @var \Drupal\Core\DrupalKernel $kernel */
     $kernel = $this->container->get('kernel');
     $kernel->rebuildContainer();
@@ -66,37 +66,55 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
     $this->container->set('package_manager.path_locator', $locator->reveal());
     $this->setCurrentUser($user);
 
+    /** @var \Drupal\automatic_updates\Updater $updater */
+    $updater = $this->container->get('automatic_updates.updater');
+
+    // Ensure that the target package versions are what we expect.
+    $expected_versions = [
+      'production' => [
+        'drupal/core-recommended' => '9.8.1',
+      ],
+      'dev' => [
+        'drupal/core-dev' => '9.8.1',
+      ],
+    ];
+    $this->assertSame($expected_versions, $updater->claim($id)->getPackageVersions());
+
     // When we call Updater::stage(), the stored project versions should be
     // read from state and passed to Composer Stager's Stager service, in the
     // form of a Composer command. This is done using package_manager_bypass's
     // invocation recorder, rather than a regular mock, in order to test that
     // the invocation recorder itself works.
-    // The production dependencies should be updated first...
-    $expected_require_arguments = [
-      'require',
-      '--no-update',
-      'drupal/core-recommended:9.8.1',
-    ];
-    // ...followed by the dev dependencies.
-    $expected_require_dev_arguments = [
-      'require',
-      '--no-update',
-      'drupal/core-dev:9.8.1',
-      '--dev',
+    // The production requirements are changed first, followed by the dev
+    // requirements. Then the installed packages are updated.
+    $expected_arguments = [
+      [
+        'require',
+        '--no-update',
+        'drupal/core-recommended:9.8.1',
+      ],
+      [
+        'require',
+        '--dev',
+        '--no-update',
+        'drupal/core-dev:9.8.1',
+      ],
+      [
+        'update',
+        '--with-all-dependencies',
+        'drupal/core-recommended:9.8.1',
+        'drupal/core-dev:9.8.1',
+      ],
     ];
-    // In both cases, `composer update` will be called separately.
-    $expected_update_arguments = ['update', '--with-all-dependencies'];
-
-    $this->container->get('automatic_updates.updater')->claim($id)->stage();
-
-    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $stager */
-    $stager = $this->container->get('package_manager.stager');
-    $invocation_arguments = $stager->getInvocationArguments();
-    $this->assertCount(4, $invocation_arguments);
-    $this->assertSame($expected_require_arguments, $invocation_arguments[0][0]);
-    $this->assertSame($expected_update_arguments, $invocation_arguments[1][0]);
-    $this->assertSame($expected_require_dev_arguments, $invocation_arguments[2][0]);
-    $this->assertSame($expected_update_arguments, $invocation_arguments[3][0]);
+    $updater->stage();
+
+    $actual_arguments = $this->container->get('package_manager.stager')
+      ->getInvocationArguments();
+
+    $this->assertSame(count($expected_arguments), count($actual_arguments));
+    foreach ($actual_arguments as $i => [$arguments]) {
+      $this->assertSame($expected_arguments[$i], $arguments);
+    }
   }
 
   /**