Skip to content
Snippets Groups Projects
Commit ef7959d8 authored by Wim Leers's avatar Wim Leers Committed by Adam G-H
Browse files

Issue #3345633 by Wim Leers, phenaproxima: Remove FixtureManipulator::modifyPackage() last usage

parent a7a3e200
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 166 additions and 224 deletions
package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
+ 130
− 169
View file @ ef7959d8
......@@ -2,7 +2,6 @@
namespace Drupal\fixture_manipulator;
use Composer\Semver\VersionParser;
use Drupal\Component\FileSystem\FileSystem;
use Drupal\Component\Utility\NestedArray;
use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
......@@ -81,8 +80,7 @@ class FixtureManipulator {
* Adds a package.
*
* @param array $package
* The package info that should be added to installed.json and
* installed.php. Must include the `name` and `type` keys.
* A Composer package definition. Must include the `name` and `type` keys.
* @param bool $is_dev_requirement
* Whether or not the package is a development requirement.
* @param bool $allow_plugins
......@@ -143,7 +141,28 @@ class FixtureManipulator {
file_put_contents("$repo_path/$file_name", $file_contents);
}
}
$command_options = ['require', "{$package['name']}:{$package['version']}"];
return $this->requirePackage($package['name'], $package['version'], $is_dev_requirement, $allow_plugins);
}
/**
* Requires a package.
*
* @param string $package
* A package name.
* @param string $version
* A version constraint.
* @param bool $is_dev_requirement
* Whether or not the package is a development requirement.
* @param bool $allow_plugins
* Whether or not to use the '--no-plugins' option.
*/
public function requirePackage(string $package, string $version, bool $is_dev_requirement = FALSE, bool $allow_plugins = FALSE): self {
if (!$this->committingChanges) {
$this->queueManipulation('requirePackage', func_get_args());
return $this;
}
$command_options = ['require', "$package:$version"];
if ($is_dev_requirement) {
$command_options[] = '--dev';
}
......@@ -156,28 +175,31 @@ class FixtureManipulator {
}
/**
* Modifies a package's installed info.
* Modifies a package's composer.json properties.
*
* @todo Since ::setVersion() is not longer calling this method the only test
* the is using this that is not just testing this method itself is
* \Drupal\Tests\automatic_updates\Kernel\StatusCheck\ScaffoldFilePermissionsValidatorTest::testScaffoldFilesChanged
* That test is passing, so we could leave it, then we have to leave
* ::setPackage() which is very complicated. Will leave notes on
* testScaffoldFilesChanged() how we might solve that with composer commands
* instead of this method.
*
* @param string $name
* @param string $package_name
* The name of the package to modify.
* @param array $package
* The package info that should be updated in installed.json and
* installed.php.
* @param string $version
* The version to use for the modified package. Can be the same as the
* original version, or a different version.
* @param array $config
* The config to be added to the package's composer.json.
* @param bool $is_dev_requirement
* Whether or not the package is a development requirement.
*
* @see \Composer\Command\ConfigCommand
*/
public function modifyPackage(string $name, array $package): self {
public function modifyPackageConfig(string $package_name, string $version, array $config, bool $is_dev_requirement = FALSE): self {
if (!$this->committingChanges) {
$this->queueManipulation('modifyPackage', func_get_args());
$this->queueManipulation('modifyPackageConfig', func_get_args());
return $this;
}
$this->setPackage($name, $package, TRUE);
$package = [
'name' => $package_name,
'version' => $version,
] + $config;
$this->addRepository($package);
$this->runComposerCommand(array_filter(['require', "$package_name:$version", $is_dev_requirement ? '--dev' : NULL]));
return $this;
}
......@@ -198,13 +220,7 @@ class FixtureManipulator {
$this->queueManipulation('setVersion', func_get_args());
return $this;
}
$package = [
'name' => $package_name,
'version' => $version,
];
$this->addRepository($package);
$this->runComposerCommand(array_filter(['require', "$package_name:$version", $is_dev_requirement ? '--dev' : NULL]));
return $this;
return $this->modifyPackageConfig($package_name, $version, [], $is_dev_requirement);
}
/**
......@@ -236,125 +252,6 @@ class FixtureManipulator {
return $this;
}
/**
* Changes a package's installation information in a particular directory.
*
* This function is internal and should not be called directly. Use
* ::addPackage(), ::modifyPackage(), and ::removePackage() instead.
*
* @todo Remove this method once ::modifyPackage() doesn't call it.
*
* @param string $pretty_name
* The name of the package to add, update, or remove.
* @param array|null $package
* The package information to be set in installed.json and installed.php, or
* NULL to remove it. Will be merged into the existing information if the
* package is already installed.
* @param bool $should_exist
* Whether or not the package is expected to already be installed.
* @param bool|null $is_dev_requirement
* Whether or not the package is a developer requirement.
*/
private function setPackage(string $pretty_name, ?array $package, bool $should_exist, ?bool $is_dev_requirement = NULL): void {
// @see \Composer\Package\BasePackage::__construct()
$name = strtolower($pretty_name);
if ($should_exist && isset($is_dev_requirement)) {
throw new \LogicException('Changing an existing project to a dev requirement is not supported');
}
$composer_folder = $this->dir . '/vendor/composer';
$file = $composer_folder . '/installed.json';
self::ensureFilePathIsWritable($file);
$data = file_get_contents($file);
$data = json_decode($data, TRUE, 512, JSON_THROW_ON_ERROR);
// If the package is already installed, find its numerical index.
$position = NULL;
for ($i = 0; $i < count($data['packages']); $i++) {
if ($data['packages'][$i]['name'] === $name) {
$position = $i;
break;
}
}
// Ensure that we actually expect to find the package already installed (or
// not).
$expected_package_message = $should_exist
? "Expected package '$pretty_name' to be installed, but it wasn't."
: "Expected package '$pretty_name' to not be installed, but it was.";
if ($should_exist !== isset($position)) {
throw new \LogicException($expected_package_message);
}
if ($package) {
$package = ['name' => $pretty_name] + $package;
$install_json_package = $package;
// Composer will use 'version_normalized', if present, to determine the
// version number.
if (isset($install_json_package['version']) && !isset($install_json_package['version_normalized'])) {
$parser = new VersionParser();
$install_json_package['version_normalized'] = $parser->normalize($install_json_package['version']);
}
}
if (isset($position)) {
// If we're going to be updating the package data, merge the incoming data
// into what we already have.
if ($package) {
$install_json_package = $install_json_package + $data['packages'][$position];
}
// If `$package === NULL`, the existing package should be removed.
if ($package === NULL) {
array_splice($data['packages'], $position, 1);
$is_existing_dev_package = in_array($name, $data['dev-package-names'], TRUE);
$data['dev-package-names'] = array_diff($data['dev-package-names'], [$name]);
$data['dev-package-names'] = array_values($data['dev-package-names']);
}
}
// Add the package back to the list, if we have data for it.
if (isset($install_json_package)) {
// If it previously existed, put it back in the previous position.
if ($position) {
$data['packages'][$i] = $install_json_package;
}
// Otherwise, it must be new: append it to the list.
else {
$data['packages'][] = $install_json_package;
}
if ($is_dev_requirement || !empty($is_existing_dev_package)) {
$data['dev-package-names'][] = $name;
}
}
file_put_contents($file, json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
self::ensureFilePathIsWritable($file);
$file = $composer_folder . '/installed.php';
self::ensureFilePathIsWritable($file);
$data = require $file;
// Ensure that we actually expect to find the package already installed (or
// not).
if ($should_exist !== isset($data['versions'][$name])) {
throw new \LogicException($expected_package_message);
}
if ($package) {
$install_php_package = $should_exist ?
NestedArray::mergeDeep($data['versions'][$name], $package) :
$package;
$data['versions'][$name] = $install_php_package;
}
else {
unset($data['versions'][$name]);
}
$data = var_export($data, TRUE);
file_put_contents($file, "<?php\nreturn $data;");
}
/**
* Adds a project at a path.
*
......@@ -402,7 +299,9 @@ class FixtureManipulator {
}
/**
* Modifies a package's installed info.
* Modifies the project root's composer.json properties.
*
* @see \Composer\Command\ConfigCommand
*
* @param array $additional_config
* The configuration to add.
......@@ -574,7 +473,7 @@ class FixtureManipulator {
* Transform the received $package into options for `composer init`.
*
* @param array $package
* A Composer package definition.
* A Composer package definition. Must include the `name` and `type` keys.
*
* @return array
* The corresponding `composer init` options.
......@@ -589,6 +488,9 @@ class FixtureManipulator {
case 'require':
case 'require-dev':
if (empty($v)) {
return NULL;
}
$requirements = array_map(
fn(string $req_package, string $req_version): string => "$req_package:$req_version",
array_keys($v),
......@@ -610,11 +512,50 @@ class FixtureManipulator {
}, array_keys($package), array_values($package)));
}
/**
* Creates a path repo.
*
* @param array $package
* A Composer package definition. Must include the `name` and `type` keys.
* @param string $repo_path
* The path at which to create a path repo for this package.
* @param string|null $original_repo_path
* If NULL: this is the first version of this package. Otherwise: a string
* containing the path repo to the first version of this package. This will
* be used to automatically inherit the same files (typically *.info.yml).
*/
private function createPathRepo(array $package, string $repo_path, ?string $original_repo_path): void {
$fs = new SymfonyFileSystem();
if (is_dir($repo_path)) {
throw new \LogicException("A path repo already exists at $repo_path.");
}
// Create the repo if it does not exist.
$fs->mkdir($repo_path);
// Forks also get the original's additional files (e.g. *.info.yml files).
if ($original_repo_path) {
$fs->mirror($original_repo_path, $repo_path);
// composer.json will be freshly generated by `composer init` below.
$fs->remove($repo_path . '/composer.json');
}
// Switch the working directory from project root to repo path.
$project_root_dir = $this->dir;
$this->dir = $repo_path;
// Create a composer.json file using `composer init`.
$this->runComposerCommand(['init', ...static::getComposerInitOptionsForPackage($package)]);
// Set the `extra` property in the generated composer.json file using
// `composer config`, because `composer init` does not support it.
foreach ($package['extra'] ?? [] as $extra_property => $extra_value) {
$this->runComposerCommand(['config', "extra.$extra_property", '--json', json_encode($extra_value, JSON_UNESCAPED_SLASHES)]);
}
// Restore the project root as the working directory.
$this->dir = $project_root_dir;
}
/**
* Adds a path repository.
*
* @param array $package
* The package.
* A Composer package definition. Must include the `name` and `type` keys.
*
* @return string
* The repository path.
......@@ -623,27 +564,45 @@ class FixtureManipulator {
$name = $package['name'];
$path_repo_base = \Drupal::state()->get(self::PATH_REPO_STATE_KEY);
$repo_path = "$path_repo_base/" . str_replace('/', '--', $name);
$fs = new SymfonyFileSystem();
if (!is_dir($repo_path)) {
// Create the repo if it does not exist.
$fs->mkdir($repo_path);
// Switch the working directory from project root to repo path.
$project_root_dir = $this->dir;
$this->dir = $repo_path;
// Create a composer.json file using `composer init`.
$this->runComposerCommand(['init', ...static::getComposerInitOptionsForPackage($package)]);
// Set the `extra` property in the generated composer.json file using
// `composer config`, because `composer init` does not support it.
foreach ($package['extra'] ?? [] as $extra_property => $extra_value) {
$this->runComposerCommand(['config', "extra.$extra_property", '--json', json_encode($extra_value, JSON_UNESCAPED_SLASHES)]);
// Determine if the given $package is a new package or a fork of an existing
// one (that means it's either the same version but with other metadata, or
// a new version with other metadata). Existing path repos are never
// modified, not even if the same version of a package is assigned other
// metadata. This allows always comparing with the original metadata.
$is_new_or_fork = !is_dir($repo_path) ? 'new' : 'fork';
if ($is_new_or_fork === 'fork') {
$original_composer_json_path = $repo_path . DIRECTORY_SEPARATOR . 'composer.json';
$original_repo_path = $repo_path;
$original_composer_json_data = json_decode(file_get_contents($original_composer_json_path), TRUE);
$forked_composer_json_data = NestedArray::mergeDeep($original_composer_json_data, $package);
if ($original_composer_json_data === $forked_composer_json_data) {
throw new \LogicException(sprintf('Nothing is actually different in this fork of the package %s.', $package['name']));
}
$package = $forked_composer_json_data;
$repo_path .= "--{$package['version']}";
// Cannot create multiple forks with the same version. This is likely
// due to a test simulating a failed Stage::apply().
if (!is_dir($repo_path)) {
$this->createPathRepo($package, $repo_path, $original_repo_path);
}
}
else {
$this->createPathRepo($package, $repo_path, NULL);
}
// Register the repository, keyed by package name and version. This ensures
// each package is registered only once and its version can be updated (but
// must have unique versions).
$repo_key = "repo.$name";
if ($is_new_or_fork === 'fork') {
$repositories = json_decode(file_get_contents($this->dir . '/composer.json'), TRUE)['repositories'];
// @todo consistently use 'version' or 'options.versions.PACKAGE_NAME', by fixing ComposerFixtureCreator in https://drupal.org/i/3347055
$original_version = isset($repositories[$name]['version']) ? $repositories[$name]['version'] : $repositories[$name]['options']['versions'][$name];
if ($package['version'] !== $original_version) {
$repo_key .= "--" . $package['version'];
}
// Restore the project root as the working directory.
$this->dir = $project_root_dir;
}
// Register the repository, keyed by package name. This ensures each package
// is registered only once and its version can be updated.
// @todo Should we create 1 repo per version.
$repository = json_encode([
'type' => 'path',
'url' => $repo_path,
......@@ -652,9 +611,11 @@ class FixtureManipulator {
'versions' => [
$name => $package['version'],
],
// @see https://getcomposer.org/repoprio
'canonical' => FALSE,
],
], JSON_UNESCAPED_SLASHES);
$this->runComposerCommand(['config', "repo.$name", $repository]);
$this->runComposerCommand(['config', $repo_key, $repository]);
return $repo_path;
}
......
package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php
+ 1
− 3
View file @ ef7959d8
......@@ -214,9 +214,7 @@ class ComposerPatchesValidatorTest extends PackageManagerKernelTestBase {
]);
}
if ($in_stage & static::REQUIRE_PACKAGE_FROM_ROOT && !($in_active & static::REQUIRE_PACKAGE_FROM_ROOT)) {
$package_data = json_decode(file_get_contents(__DIR__ . '/../../fixtures/path_repos/cweagans--composer-patches/composer.json'), TRUE);
$package_data['version'] = '24.12.1999';
$stage_manipulator->addPackage($package_data);
$stage_manipulator->requirePackage('cweagans/composer-patches', '24.12.1999');
}
if (!($in_stage & static::REQUIRE_PACKAGE_FROM_ROOT) && $in_active & static::REQUIRE_PACKAGE_FROM_ROOT) {
$stage_manipulator
......
package_manager/tests/src/Kernel/FixtureManipulatorTest.php
+ 14
− 35
View file @ ef7959d8
......@@ -6,7 +6,6 @@ namespace Drupal\Tests\package_manager\Kernel;
use Drupal\fixture_manipulator\ActiveFixtureManipulator;
use Drupal\fixture_manipulator\FixtureManipulator;
use Symfony\Component\Filesystem\Filesystem;
/**
* @coversDefaultClass \Drupal\fixture_manipulator\FixtureManipulator
......@@ -64,6 +63,7 @@ class FixtureManipulatorTest extends PackageManagerKernelTestBase {
->addPackage([
'name' => 'my/package',
'type' => 'library',
'version' => '1.2.3',
])
->addPackage(
[
......@@ -177,8 +177,9 @@ class FixtureManipulatorTest extends PackageManagerKernelTestBase {
/**
* @covers ::modifyPackage
*/
public function testModifyPackage(): void {
$fs = (new Filesystem());
public function testModifyPackageConfig(): void {
$inspector = $this->container->get('package_manager.composer_inspector');
// Assert ::modifyPackage() works with a package in an existing fixture not
// created by ::addPackage().
$decode_installed_json = function () {
......@@ -188,41 +189,19 @@ class FixtureManipulatorTest extends PackageManagerKernelTestBase {
$this->assertIsArray($original_installed_json);
(new ActiveFixtureManipulator())
// @see ::setUp()
->modifyPackage('my/dev-package', ['version' => '2.1.0'])
->modifyPackageConfig('my/dev-package', '2.1.0', ['description' => 'something else'], TRUE)
->commitChanges();
$this->assertSame($original_installed_json, $decode_installed_json());
// Assert that ::modifyPackage() throws an error if a package exists in the
// 'installed.json' file but not the 'installed.php' file. We cannot test
// this with the trait functions because they cannot produce this starting
// point.
$existing_incorrect_fixture = __DIR__ . '/../../fixtures/FixtureUtilityTraitTest/missing_installed_php';
$temp_fixture = $this->siteDirectory . $this->randomMachineName('42');
$fs->mirror($existing_incorrect_fixture, $temp_fixture);
try {
(new FixtureManipulator())
->modifyPackage('the-org/the-package', ['irrelevant' => TRUE])
->commitChanges($temp_fixture);
$this->fail('Modifying a non-existent package should raise an error.');
}
catch (\LogicException $e) {
$this->assertSame("Expected package 'the-org/the-package' to be installed, but it wasn't.", $e->getMessage());
}
// We should not be able to modify a non-existent package.
try {
(new ActiveFixtureManipulator())
->modifyPackage('junk/drawer', ['type' => 'library'])
->commitChanges();
$this->fail('Modifying a non-existent package should raise an error.');
}
catch (\LogicException $e) {
$this->assertStringContainsString("Expected package 'junk/drawer' to be installed, but it wasn't.", $e->getMessage());
}
// Verify that the package is indeed properly installed.
$this->assertSame('2.1.0', $inspector->getInstalledPackagesList($this->dir)['my/dev-package']->version);
// Verify that the original exists, but has no description.
$this->assertSame('my/dev-package', $original_installed_json['packages'][3]['name']);
$this->assertArrayNotHasKey('description', $original_installed_json['packages']);
// Verify that the description was updated.
$this->assertSame('something else', $decode_installed_json()['packages'][3]['description']);
(new ActiveFixtureManipulator())
// Add a key to an existing package.
->modifyPackage('my/package', ['type' => 'metapackage'])
->modifyPackageConfig('my/package', '1.2.3', ['extra' => ['foo' => 'bar']])
// Change a key in an existing package.
->setVersion('my/dev-package', '3.2.1', TRUE)
// Move an existing package to dev requirements.
......@@ -249,7 +228,7 @@ class FixtureManipulatorTest extends PackageManagerKernelTestBase {
'name' => 'my/package',
'version' => '1.2.3',
'version_normalized' => '1.2.3.0',
'type' => 'metapackage',
'type' => 'library',
],
];
$installed_php_expected_packages = $install_json_expected_packages;
......
tests/src/Kernel/StatusCheck/ScaffoldFilePermissionsValidatorTest.php
+ 21
− 14
View file @ ef7959d8
......@@ -293,27 +293,34 @@ class ScaffoldFilePermissionsValidatorTest extends AutomaticUpdatesKernelTestBas
* @dataProvider providerScaffoldFilesChanged
*/
public function testScaffoldFilesChanged(array $write_protected_paths, array $active_scaffold_files, array $staged_scaffold_files, array $expected_results): void {
// Rewrite the active and staged installed.json files, inserting the given
// Rewrite the active and staged composer.json files, inserting the given
// lists of scaffold files.
// @todo Remove the use of modifyPackage() in https://drupal.org/i/3345633.
(new ActiveFixtureManipulator())
->modifyPackage('drupal/core', [
'extra' => [
'drupal-scaffold' => [
'file-mapping' => $active_scaffold_files,
if ($active_scaffold_files) {
(new ActiveFixtureManipulator())
->modifyPackageConfig('drupal/core', '9.8.0', [
'extra' => [
'drupal-scaffold' => [
'file-mapping' => $active_scaffold_files,
],
],
],
])
->commitChanges();
$this->getStageFixtureManipulator()
->setCorePackageVersion('9.8.1')
->modifyPackage('drupal/core', [
])
->commitChanges();
}
$stage_manipulator = $this->getStageFixtureManipulator();
$stage_manipulator->setVersion('drupal/core-recommended', '9.8.1');
$stage_manipulator->setVersion('drupal/core-dev', '9.8.1');
if ($staged_scaffold_files) {
$stage_manipulator->modifyPackageConfig('drupal/core', '9.8.1', [
'extra' => [
'drupal-scaffold' => [
'file-mapping' => $staged_scaffold_files,
],
],
]);
}
else {
$stage_manipulator->setVersion('drupal/core', '9.8.1');
}
// Create fake scaffold files so we can test scenarios in which a scaffold
// file that exists in the active directory is deleted in the stage
......@@ -331,7 +338,7 @@ class ScaffoldFilePermissionsValidatorTest extends AutomaticUpdatesKernelTestBas
$updater->apply();
// If no exception was thrown, ensure that we weren't expecting an error.
$this->assertEmpty($expected_results);
$this->assertSame([], $expected_results);
}
// If we try to overwrite any write-protected paths, even if they're not
// scaffold files, we'll get an ApplyFailedException.
......
tests/src/Kernel/StatusCheck/StagedProjectsValidatorTest.php
+ 0
− 3
View file @ ef7959d8
......@@ -157,7 +157,6 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
*/
public function testProjectsRemoved(): void {
(new ActiveFixtureManipulator())
->setCorePackageVersion('9.8.0')
->addPackage([
'name' => 'drupal/test_theme',
'version' => '1.3.0',
......@@ -230,7 +229,6 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
*/
public function testVersionsChanged(): void {
(new ActiveFixtureManipulator())
->setCorePackageVersion('9.8.0')
->addPackage([
'name' => 'drupal/test-module',
'version' => '1.3.0',
......@@ -291,7 +289,6 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
*/
public function testNoErrors(): void {
(new ActiveFixtureManipulator())
->setCorePackageVersion('9.8.0')
->addPackage([
'name' => 'drupal/test-module',
'version' => '1.3.0',
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment