From d227242887904ad13e0714c41b31500859ff3972 Mon Sep 17 00:00:00 2001
From: omkar podey <58183-omkar.podey@users.noreply.drupalcode.org>
Date: Thu, 9 Mar 2023 13:20:41 +0000
Subject: [PATCH] Issue #3346628: ComposerPluginsValidatorTest fails with
pcre.backtrack_limit=1000000 set
---
.cspell.json | 1 -
.../src/Functional/UpdaterFormTestBase.php | 3 -
drupalci.yml | 6 -
.../src/FixtureManipulator.php | 109 +++++++++++++-----
pcre.ini | 8 --
scripts/src/Converter.php | 1 -
6 files changed, 78 insertions(+), 50 deletions(-)
delete mode 100644 pcre.ini
diff --git a/.cspell.json b/.cspell.json
index 368b755a17..950ad4ab51 100644
--- a/.cspell.json
+++ b/.cspell.json
@@ -4,7 +4,6 @@
{ "name": "automatic_updates", "path": "./dictionary.txt"}
],
"ignorePaths": [
- "pcre.ini",
"README.md"
]
}
diff --git a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTestBase.php b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTestBase.php
index a154b52442..061781f288 100644
--- a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTestBase.php
+++ b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTestBase.php
@@ -46,19 +46,16 @@ abstract class UpdaterFormTestBase extends UpdaterFormFunctionalTestBase {
'name' => 'drupal/semver_test',
'version' => '8.1.0',
'type' => 'drupal-module',
- 'install_path' => '../../web/projects/semver_test',
])
->addPackage([
'name' => 'drupal/aaa_update_test',
'version' => '2.0.0',
'type' => 'drupal-module',
- 'install_path' => '../../web/projects/aaa_update_test',
])
->addPackage([
'name' => 'drupal/automatic_updates_extensions_test_theme',
'version' => '2.0.0',
'type' => 'drupal-theme',
- 'install_path' => '../../web/projects/automatic_updates_extensions_test_theme',
])
->commitChanges();
$this->drupalPlaceBlock('local_tasks_block', ['primary' => TRUE]);
diff --git a/drupalci.yml b/drupalci.yml
index 422d14d781..2d5633337e 100644
--- a/drupalci.yml
+++ b/drupalci.yml
@@ -9,12 +9,6 @@ build:
# Run code quality checks.
container_command.commit-checks:
commands:
- # Disable the PCRE engine's JIT, since it causes Composer to die during the
- # update process, but only on Drupal CI, and for reasons that are essentially
- # impossible to trace into. The PCRE JIT is not necessary for Automatic Updates
- # to work correctly, and disabling it is a known workaround.
- # @see pcre.ini
- - sudo cp modules/contrib/automatic_updates/pcre.ini /usr/local/etc/php/conf.d
# @todo Replace in favor of commit-code-check.sh once https://www.drupal.org/project/drupal/issues/3314100 lands.
- modules/contrib/automatic_updates/scripts/commit-code-check.sh --drupalci
halt-on-fail: true
diff --git a/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php b/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
index a333efff4d..b08421244e 100644
--- a/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
+++ b/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
@@ -11,7 +11,14 @@ use Symfony\Component\Filesystem\Filesystem as SymfonyFileSystem;
use Drupal\Component\Serialization\Yaml;
/**
- * It manipulates.
+ * Manipulates a test fixture using Composer commands.
+ *
+ * The composer.json file CANNOT be safely created or modified using the
+ * json_encode() function, because Composer does not use a real JSON parser — it
+ * updates composer.json using \Composer\Json\JsonManipulator, which is known to
+ * choke in a number of scenarios.
+ *
+ * @see https://www.drupal.org/i/3346628
*/
class FixtureManipulator {
@@ -563,6 +570,46 @@ class FixtureManipulator {
return $plain_output;
}
+ /**
+ * Transform the received $package into options for `composer init`.
+ *
+ * @param array $package
+ * A Composer package definition.
+ *
+ * @return array
+ * The corresponding `composer init` options.
+ */
+ private static function getComposerInitOptionsForPackage(array $package): array {
+ return array_filter(array_map(function ($k, $v) {
+ switch ($k) {
+ case 'name':
+ case 'description':
+ case 'type':
+ return "--$k=$v";
+
+ case 'require':
+ case 'require-dev':
+ $requirements = array_map(
+ fn(string $req_package, string $req_version): string => "$req_package:$req_version",
+ array_keys($v),
+ array_values($v)
+ );
+ return "--$k=" . implode(',', $requirements);
+
+ case 'version':
+ // This gets set in the repository metadata itself.
+ return NULL;
+
+ case 'extra':
+ // Cannot be set using `composer init`, only `composer config` can.
+ return NULL;
+
+ default:
+ throw new \InvalidArgumentException($k);
+ }
+ }, array_keys($package), array_values($package)));
+ }
+
/**
* Adds a path repository.
*
@@ -576,30 +623,39 @@ class FixtureManipulator {
$name = $package['name'];
$path_repo_base = \Drupal::state()->get(self::PATH_REPO_STATE_KEY);
$repo_path = "$path_repo_base/" . str_replace('/', '--', $name);
- $composer_json_path = $repo_path . DIRECTORY_SEPARATOR . 'composer.json';
$fs = new SymfonyFileSystem();
if (!is_dir($repo_path)) {
// Create the repo if it does not exist.
$fs->mkdir($repo_path);
- file_put_contents($composer_json_path, json_encode($package, JSON_THROW_ON_ERROR));
- $repository = json_encode([
- 'type' => 'path',
- 'url' => $repo_path,
- 'options' => [
- 'symlink' => FALSE,
- ],
- ], JSON_UNESCAPED_SLASHES);
- $this->runComposerCommand(['config', "repo.$name", $repository]);
- }
- else {
- $composer_json_data = json_decode(file_get_contents($composer_json_path), TRUE);
- // Update the version if needed.
- // @todo Should we create 1 repo per version.
- if ($composer_json_data['version'] !== $package['version']) {
- $composer_json_data['version'] = $package['version'];
- file_put_contents($composer_json_path, json_encode($composer_json_data, JSON_THROW_ON_ERROR));
+ // 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;
+ }
+
+ // 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,
+ 'options' => [
+ 'symlink' => FALSE,
+ 'versions' => [
+ $name => $package['version'],
+ ],
+ ],
+ ], JSON_UNESCAPED_SLASHES);
+ $this->runComposerCommand(['config', "repo.$name", $repository]);
+
return $repo_path;
}
@@ -627,17 +683,8 @@ class FixtureManipulator {
}
// Update all the repos in the composer.json file to point to the new
// repos at the absolute path.
- $json_data = json_decode(file_get_contents($this->dir . '/composer.json'), TRUE);
- $composer_json_needs_update = FALSE;
- foreach ($json_data['repositories'] as &$existing_repo_data) {
- if (is_array($existing_repo_data) && isset($existing_repo_data['url']) && str_starts_with($existing_repo_data['url'], '../path_repos/')) {
- $composer_json_needs_update = TRUE;
- $existing_repo_data['url'] = str_replace('../path_repos/', "$path_repo_base/", $existing_repo_data['url']);
- }
- }
- if ($composer_json_needs_update) {
- file_put_contents($this->dir . '/composer.json', json_encode($json_data, JSON_THROW_ON_ERROR));
- }
+ $composer_json = file_get_contents($this->dir . '/composer.json');
+ file_put_contents($this->dir . '/composer.json', str_replace('../path_repos/', "$path_repo_base/", $composer_json));
$this->runComposerCommand(['install']);
}
diff --git a/pcre.ini b/pcre.ini
deleted file mode 100644
index 0c5780afaf..0000000000
--- a/pcre.ini
+++ /dev/null
@@ -1,8 +0,0 @@
-# This file is used on Drupal CI because Composer occasionally dies with
-# PCRE_JIT_STACKLIMIT_ERROR during certain test cases, and there is no insight
-# into where the failure is happening, or why. This has only been observed on
-# Drupal CI. Since the PCRE engine's JIT is not strictly necessary to the
-# functioning of the Automatic Updates module, this file disables the PCRE JIT
-# as a workaround.
-# @see drupalci.yml
-pcre.jit = 0
diff --git a/scripts/src/Converter.php b/scripts/src/Converter.php
index 0bdb88655a..36fbc8f872 100644
--- a/scripts/src/Converter.php
+++ b/scripts/src/Converter.php
@@ -72,7 +72,6 @@ class Converter {
'drupalci.yml',
'README.md',
'.git',
- 'pcre.ini',
'composer.json',
'.gitattributes',
'.gitignore',
--
GitLab