From a97a0c7cfe06b97f6d1538518db7623da8f24fa0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Thu, 9 Mar 2023 08:36:17 -0500
Subject: [PATCH] Revert "Issue #3346628: ComposerPluginsValidatorTest fails
with pcre.backtrack_limit=1000000 set"
This reverts commit d227242887904ad13e0714c41b31500859ff3972.
---
.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, 50 insertions(+), 78 deletions(-)
create mode 100644 pcre.ini
diff --git a/.cspell.json b/.cspell.json
index 950ad4ab51..368b755a17 100644
--- a/.cspell.json
+++ b/.cspell.json
@@ -4,6 +4,7 @@
{ "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 061781f288..a154b52442 100644
--- a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTestBase.php
+++ b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTestBase.php
@@ -46,16 +46,19 @@ 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 2d5633337e..422d14d781 100644
--- a/drupalci.yml
+++ b/drupalci.yml
@@ -9,6 +9,12 @@ 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 b08421244e..a333efff4d 100644
--- a/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
+++ b/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
@@ -11,14 +11,7 @@ use Symfony\Component\Filesystem\Filesystem as SymfonyFileSystem;
use Drupal\Component\Serialization\Yaml;
/**
- * 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
+ * It manipulates.
*/
class FixtureManipulator {
@@ -570,46 +563,6 @@ 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.
*
@@ -623,39 +576,30 @@ 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);
- // 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'],
+ 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]);
-
+ ], 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));
+ }
+ }
return $repo_path;
}
@@ -683,8 +627,17 @@ class FixtureManipulator {
}
// Update all the repos in the composer.json file to point to the new
// repos at the absolute path.
- $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));
+ $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));
+ }
$this->runComposerCommand(['install']);
}
diff --git a/pcre.ini b/pcre.ini
new file mode 100644
index 0000000000..0c5780afaf
--- /dev/null
+++ b/pcre.ini
@@ -0,0 +1,8 @@
+# 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 36fbc8f872..0bdb88655a 100644
--- a/scripts/src/Converter.php
+++ b/scripts/src/Converter.php
@@ -72,6 +72,7 @@ class Converter {
'drupalci.yml',
'README.md',
'.git',
+ 'pcre.ini',
'composer.json',
'.gitattributes',
'.gitignore',
--
GitLab