From 73730fb909fdf2cc033397d2a4d199ef88cff878 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Wed, 19 Jan 2022 14:26:37 +0000
Subject: [PATCH] Issue #3258045 by phenaproxima: Ensure update command in
Stage::require() only updates specified packages
---
package_manager/src/ComposerUtility.php | 19 +------
package_manager/src/Stage.php | 42 +++++++++------
src/Updater.php | 16 +++---
tests/fixtures/fake-site/composer.lock | 7 ++-
tests/src/Kernel/UpdaterTest.php | 70 ++++++++++++++++---------
5 files changed, 83 insertions(+), 71 deletions(-)
diff --git a/package_manager/src/ComposerUtility.php b/package_manager/src/ComposerUtility.php
index cfab9e4087..4cf3d48ddd 100644
--- a/package_manager/src/ComposerUtility.php
+++ b/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();
diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php
index c109ea85ed..b1b9483f6d 100644
--- a/package_manager/src/Stage.php
+++ b/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));
}
diff --git a/src/Updater.php b/src/Updater.php
index cbc1b3a02c..93aa600370 100644
--- a/src/Updater.php
+++ b/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']);
}
/**
diff --git a/tests/fixtures/fake-site/composer.lock b/tests/fixtures/fake-site/composer.lock
index dd29a0514d..05715cda77 100644
--- a/tests/fixtures/fake-site/composer.lock
+++ b/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"
+ }
+ ]
}
diff --git a/tests/src/Kernel/UpdaterTest.php b/tests/src/Kernel/UpdaterTest.php
index 7cabf9e29d..a1c96278ce 100644
--- a/tests/src/Kernel/UpdaterTest.php
+++ b/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);
+ }
}
/**
--
GitLab