Issue #3244412 by phenaproxima, tedbow: During updates, dev dependencies are...

Issue #3244412 by phenaproxima, tedbow: During updates, dev dependencies are moved into production dependencies

package_manager/src/ComposerUtility.php

@@ -109,6 +109,22 @@ 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.
+   */
+  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.
    *

package_manager/src/Stage.php

@@ -114,10 +114,16 @@ class Stage {
    *
    * @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.
    */
-  public function require(array $constraints): void {
+  public function require(array $constraints, bool $dev = FALSE): void {
     $command = array_merge(['require'], $constraints);
     $command[] = '--update-with-all-dependencies';
+    if ($dev) {
+      $command[] = '--dev';
+    }
 
     $this->dispatch(new PreRequireEvent($this));
     $this->stager->stage($command, $this->pathLocator->getStageDirectory());

src/Updater.php

@@ -15,11 +15,11 @@ use Drupal\package_manager\StageException;
 class Updater extends Stage {
 
   /**
-   * The state key in which to store the status of the update.
+   * State key under which to store the package versions targeted by the update.
    *
    * @var string
    */
-  protected const STATE_KEY = 'AUTOMATIC_UPDATES_CURRENT';
+  protected const PACKAGES_KEY = 'automatic_updates.packages';
 
   /**
    * The state service.
@@ -49,7 +49,7 @@ class Updater extends Stage {
    */
   public function hasActiveUpdate(): bool {
     $staged_dir = $this->pathLocator->getStageDirectory();
-    if (is_dir($staged_dir) || $this->state->get(static::STATE_KEY)) {
+    if (is_dir($staged_dir) || $this->state->get(static::PACKAGES_KEY)) {
       return TRUE;
     }
     return FALSE;
@@ -61,54 +61,62 @@ class Updater extends Stage {
    * @param string[] $project_versions
    *   The versions of the packages to update to, keyed by package name.
    *
-   * @return string
-   *   A key for this stage update process.
-   *
    * @throws \InvalidArgumentException
    *   Thrown if no project version for Drupal core is provided.
    */
-  public function begin(array $project_versions): string {
+  public function begin(array $project_versions): void {
     if (count($project_versions) !== 1 || !array_key_exists('drupal', $project_versions)) {
       throw new \InvalidArgumentException("Currently only updates to Drupal core are supported.");
     }
 
     $composer = ComposerUtility::createForDirectory($this->pathLocator->getActiveDirectory());
-    $packages = [];
+    $package_versions = $this->getPackageVersions();
+
     foreach ($composer->getCorePackageNames() as $package) {
-      $packages[$package] = $project_versions['drupal'];
+      $package_versions['production'][$package] = $project_versions['drupal'];
     }
-    $stage_key = static::STATE_KEY . microtime();
-    $this->state->set(static::STATE_KEY, [
-      'id' => $stage_key,
-      'package_versions' => $packages,
-    ]);
+    foreach ($composer->getCoreDevPackageNames() as $package) {
+      $package_versions['dev'][$package] = $project_versions['drupal'];
+    }
+    $this->state->set(static::PACKAGES_KEY, $package_versions);
     $this->create();
-    return $stage_key;
   }
 
   /**
    * Returns the package versions that will be required during the update.
    *
-   * @return string[]
-   *   The package versions, as a set of Composer constraints where the keys are
-   *   the package names, and the values are the version constraints.
+   * @return string[][]
+   *   An array with two sub-arrays: 'production' and 'dev'. Each is a set of
+   *   package versions, where the keys are package names and the values are
+   *   version constraints understood by Composer.
    */
   public function getPackageVersions(): array {
-    $metadata = $this->state->get(static::STATE_KEY, []);
-    return $metadata['package_versions'];
+    return $this->state->get(static::PACKAGES_KEY, [
+      'production' => [],
+      'dev' => [],
+    ]);
   }
 
   /**
    * Stages the update.
    */
   public function stage(): void {
-    $metadata = $this->state->get(static::STATE_KEY, []);
+    // Convert an associative array of package versions, keyed by name, to
+    // command-line arguments in the form `vendor/name:version`.
+    $map = function (array $versions): array {
+      $requirements = [];
+      foreach ($versions as $package => $version) {
+        $requirements[] = "$package:$version";
+      }
+      return $requirements;
+    };
+    $versions = array_map($map, $this->getPackageVersions());
+
+    $this->require($versions['production']);
 
-    $requirements = [];
-    foreach ($metadata['package_versions'] as $package => $constraint) {
-      $requirements[] = "$package:$constraint";
+    if ($versions['dev']) {
+      $this->require($versions['dev'], TRUE);
     }
-    $this->require($requirements);
   }
 
   /**
@@ -116,7 +124,7 @@ class Updater extends Stage {
    */
   public function clean(): void {
     $this->destroy();
-    $this->state->delete(static::STATE_KEY);
+    $this->state->delete(static::PACKAGES_KEY);
   }
 
   /**

src/Validator/UpdateVersionValidator.php

@@ -69,7 +69,7 @@ class UpdateVersionValidator implements EventSubscriberInterface {
     else {
       // If the stage has begun its life cycle, we expect it knows the desired
       // package versions.
-      $package_versions = $stage->getPackageVersions();
+      $package_versions = $stage->getPackageVersions()['production'];
     }
 
     $from_version_string = $this->getCoreVersion();
@@ -107,7 +107,7 @@ class UpdateVersionValidator implements EventSubscriberInterface {
     elseif ($from_version->getMinorVersion() !== $to_version->getMinorVersion()) {
       $messages[] = $this->t('Drupal cannot be automatically updated from its current version, @from_version, to the recommended version, @to_version, because automatic updates from one minor version to another are not supported.', [
         '@from_version' => $this->getCoreVersion(),
-        '@to_version' => $event->getPackageVersions()[$core_package_name],
+        '@to_version' => $package_versions[$core_package_name],
       ]);
       $error = ValidationResult::createError($messages);
       $event->addValidationResult($error);

tests/fixtures/fake-site/composer.json

@@ -2,10 +2,14 @@
     "require": {
         "drupal/test-distribution": "*"
     },
+    "require-dev": {
+        "drupal/core-dev": "^9"
+    },
     "extra": {
         "_comment": [
             "This is a fake composer.json simulating a site which requires a distribution.",
-            "The required core packages are determined by scanning the lock file."
+            "The required core packages are determined by scanning the lock file.",
+            "The required dev packages are determined by looking at the require-dev section of this file."
         ]
     }
 }

tests/src/Build/CoreUpdateTest.php

@@ -2,6 +2,8 @@
 
 namespace Drupal\Tests\automatic_updates\Build;
 
+use Drupal\Component\Serialization\Json;
+
 /**
  * Tests an end-to-end update of Drupal core.
  *
@@ -112,20 +114,18 @@ class CoreUpdateTest extends UpdateTestBase {
     ]);
     $repositories['drupal/core-recommended'] = $this->createPathRepository($dir);
 
-    // Create a fake version of drupal/core-project-message.
-    $dir = $this->copyPackage("$drupal_root/composer/Plugin/ProjectMessage");
-    $this->alterPackage($dir, ['version' => $version]);
-    $repositories['drupal/core-project-message'] = $this->createPathRepository($dir);
-
-    // Create a fake version of drupal/core-composer-scaffold.
-    $dir = $this->copyPackage("$drupal_root/composer/Plugin/Scaffold");
-    $this->alterPackage($dir, ['version' => $version]);
-    $repositories['drupal/core-composer-scaffold'] = $this->createPathRepository($dir);
-
-    // Create a fake version of drupal/core-vendor-hardening.
-    $dir = $this->copyPackage("$drupal_root/composer/Plugin/VendorHardening");
-    $this->alterPackage($dir, ['version' => $version]);
-    $repositories['drupal/core-vendor-hardening'] = $this->createPathRepository($dir);
+    // Create fake target versions of core plugins and metapackages.
+    $packages = [
+      'drupal/core-dev' => "$drupal_root/composer/Metapackage/DevDependencies",
+      'drupal/core-project-message' => "$drupal_root/composer/Plugin/ProjectMessage",
+      'drupal/core-composer-scaffold' => "$drupal_root/composer/Plugin/Scaffold",
+      'drupal/core-vendor-hardening' => "$drupal_root/composer/Plugin/VendorHardening",
+    ];
+    foreach ($packages as $name => $dir) {
+      $dir = $this->copyPackage($dir);
+      $this->alterPackage($dir, ['version' => $version]);
+      $repositories[$name] = $this->createPathRepository($dir);
+    }
 
     return [
       'repositories' => $repositories,
@@ -271,6 +271,28 @@ class CoreUpdateTest extends UpdateTestBase {
     // present in the README.
     $placeholder = file_get_contents($this->getWebRoot() . '/core/README.txt');
     $this->assertSame('Placeholder for Drupal core 9.8.1.', $placeholder);
+
+    $composer = file_get_contents($this->getWorkspaceDirectory() . '/composer.json');
+    $composer = Json::decode($composer);
+    // The production dependencies should have been updated.
+    $this->assertSame('9.8.1', $composer['require']['drupal/core-recommended']);
+    $this->assertSame('9.8.1', $composer['require']['drupal/core-composer-scaffold']);
+    $this->assertSame('9.8.1', $composer['require']['drupal/core-project-message']);
+    // The core-vendor-hardening plugin is only used by the legacy project
+    // template.
+    if ($composer['name'] === 'drupal/legacy-project') {
+      $this->assertSame('9.8.1', $composer['require']['drupal/core-vendor-hardening']);
+    }
+    // The production dependencies should not be listed as dev dependencies.
+    $this->assertArrayNotHasKey('drupal/core-recommended', $composer['require-dev']);
+    $this->assertArrayNotHasKey('drupal/core-composer-scaffold', $composer['require-dev']);
+    $this->assertArrayNotHasKey('drupal/core-project-message', $composer['require-dev']);
+    $this->assertArrayNotHasKey('drupal/core-vendor-hardening', $composer['require-dev']);
+
+    // The drupal/core-dev metapackage should not be a production dependency...
+    $this->assertArrayNotHasKey('drupal/core-dev', $composer['require']);
+    // ...but it should have been updated in the dev dependencies.
+    $this->assertSame('9.8.1', $composer['require-dev']['drupal/core-dev']);
   }
 
 }

tests/src/Build/UpdateTestBase.php

@@ -183,15 +183,8 @@ END;
 
     // Update the test site's composer.json.
     $this->writeJson($composer, $data);
-    // Don't install drupal/core-dev, which is defined as a dev dependency in
-    // both project templates.
-    // @todo Handle dev dependencies properly once
-    //   https://www.drupal.org/project/automatic_updates/issues/3244412 is
-    //   is resolved.
-    $this->executeCommand('composer remove --dev --no-update drupal/core-dev');
-    $this->assertCommandSuccessful();
-    // Install production dependencies.
-    $this->executeCommand('composer install --no-dev');
+    // Install dependencies, including dev.
+    $this->executeCommand('composer install');
     $this->assertCommandSuccessful();
   }
 

tests/src/Kernel/UpdaterTest.php

@@ -56,14 +56,23 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
     // form of a Composer command. We set up a mock here to ensure that those
     // calls are made as expected.
     $stager = $this->prophesize(StagerInterface::class);
+    // The production dependencies should be updated first...
     $command = [
       'require',
       'drupal/core-recommended:9.8.1',
       '--update-with-all-dependencies',
     ];
     $stager->stage($command, Argument::cetera())->shouldBeCalled();
-    $this->container->set('package_manager.stager', $stager->reveal());
+    // ...followed by the dev dependencies.
+    $command = [
+      'require',
+      'drupal/core-dev:9.8.1',
+      '--update-with-all-dependencies',
+      '--dev',
+    ];
+    $stager->stage($command, Argument::cetera())->shouldBeCalled();
 
+    $this->container->set('package_manager.stager', $stager->reveal());
     $this->container->get('automatic_updates.updater')->stage();
   }