Unverified Commit 61fa1bf2 authored by alexpott's avatar alexpott
Browse files

Issue #3104922 by greg.1.anderson, Mile23, alexpott: Guard against changes to...

Issue #3104922 by greg.1.anderson, Mile23, alexpott: Guard against changes to Scaffold Plugin that might cause upgrade problems
parent 04722fbe
......@@ -7,7 +7,6 @@
use Composer\Installer\PackageEvent;
use Composer\IO\IOInterface;
use Composer\Package\PackageInterface;
use Composer\Plugin\CommandEvent;
use Composer\Util\Filesystem;
use Drupal\Composer\Plugin\Scaffold\Operations\OperationData;
use Drupal\Composer\Plugin\Scaffold\Operations\OperationFactory;
......@@ -82,14 +81,9 @@ public function __construct(Composer $composer, IOInterface $io) {
}
/**
* Registers post-package events before any 'require' event runs.
*
* This method is called by composer prior to doing a 'require' command.
*
* @param \Composer\Plugin\CommandEvent $event
* The Composer Command event.
* Registers post-package events if the 'require' command was called.
*/
public function beforeRequire(CommandEvent $event) {
public function requireWasCalled() {
// In order to differentiate between post-package events called after
// 'composer require' vs. the same events called at other times, we will
// only install our handler when a 'require' event is detected.
......
......@@ -21,6 +21,20 @@
*/
class Plugin implements PluginInterface, EventSubscriberInterface, Capable {
/**
* The Composer service.
*
* @var \Composer\Composer
*/
protected $composer;
/**
* Composer's I/O service.
*
* @var \Composer\IO\IOInterface
*/
protected $io;
/**
* The Composer Scaffold handler.
*
......@@ -28,15 +42,20 @@ class Plugin implements PluginInterface, EventSubscriberInterface, Capable {
*/
protected $handler;
/**
* Record whether the 'require' command was called.
*
* @param bool
*/
protected $requireWasCalled;
/**
* {@inheritdoc}
*/
public function activate(Composer $composer, IOInterface $io) {
// We use a Handler object to separate the main functionality
// of this plugin from the Composer API. This also avoids some
// debug issues with the plugin being copied on initialisation.
// @see \Composer\Plugin\PluginManager::registerPackage()
$this->handler = new Handler($composer, $io);
$this->composer = $composer;
$this->io = $io;
$this->requireWasCalled = FALSE;
}
/**
......@@ -50,6 +69,7 @@ public function getCapabilities() {
* {@inheritdoc}
*/
public static function getSubscribedEvents() {
// Important note: We only instantiate our handler on "post" events.
return [
ScriptEvents::POST_UPDATE_CMD => 'postCmd',
ScriptEvents::POST_INSTALL_CMD => 'postCmd',
......@@ -65,7 +85,7 @@ public static function getSubscribedEvents() {
* The Composer event.
*/
public function postCmd(Event $event) {
$this->handler->scaffold();
$this->handler()->scaffold();
}
/**
......@@ -75,7 +95,7 @@ public function postCmd(Event $event) {
* Composer package event sent on install/update/remove.
*/
public function postPackage(PackageEvent $event) {
$this->handler->onPostPackageEvent($event);
$this->handler()->onPostPackageEvent($event);
}
/**
......@@ -86,8 +106,28 @@ public function postPackage(PackageEvent $event) {
*/
public function onCommand(CommandEvent $event) {
if ($event->getCommandName() == 'require') {
$this->handler->beforeRequire($event);
if ($this->handler) {
throw new \Error('Core Scaffold Plugin handler instantiated too early. See https://www.drupal.org/project/drupal/issues/3104922');
}
$this->requireWasCalled = TRUE;
}
}
/**
* Lazy-instantiate the handler object. It is dangerous to update a Composer
* plugin if it loads any classes prior to the `composer update` operation,
* and later tries to use them in a post-update hook.
*/
protected function handler() {
if (!$this->handler) {
$this->handler = new Handler($this->composer, $this->io);
// On instantiation of our handler, notify it if the 'require' command
// was executed.
if ($this->requireWasCalled) {
$this->handler->requireWasCalled();
}
}
return $this->handler;
}
}
<?php
namespace Drupal\Tests\Composer\Plugin\Scaffold\Functional;
use Drupal\Tests\Composer\Plugin\Scaffold\AssertUtilsTrait;
use Drupal\Tests\Composer\Plugin\Scaffold\ExecTrait;
use Drupal\Tests\Composer\Plugin\Scaffold\Fixtures;
use PHPUnit\Framework\TestCase;
/**
* Tests Upgrading the Composer Scaffold plugin.
*
* Upgrading a Composer plugin can be a dangerous operation. If the plugin
* instantiates any classes during the activate method, and the plugin code
* is subsequentially modified by a `composer update` operation, then any
* post-update hook (& etc.) may run with inconsistent code, leading to
* runtime errors. This test ensures that it is possible to upgrade from the
* last available stable 8.8.x tag to the current Scaffold plugin code (e.g. in
* the current patch-under-test).
*
* @group Scaffold
*/
class ScaffoldUpgradeTest extends TestCase {
use AssertUtilsTrait;
use ExecTrait;
/**
* The Fixtures object.
*
* @var \Drupal\Tests\Composer\Plugin\Scaffold\Fixtures
*/
protected $fixtures;
/**
* {@inheritdoc}
*/
protected function setUp() {
$this->fixtures = new Fixtures();
$this->fixtures->createIsolatedComposerCacheDir();
}
/**
* Test upgrading the Composer Scaffold plugin.
*/
public function testScaffoldUpgrade() {
$this->fixturesDir = $this->fixtures->tmpDir($this->getName());
$replacements = ['SYMLINK' => 'false', 'PROJECT_ROOT' => $this->fixtures->projectRoot()];
$this->fixtures->cloneFixtureProjects($this->fixturesDir, $replacements);
$topLevelProjectDir = 'drupal-drupal';
$sut = $this->fixturesDir . '/' . $topLevelProjectDir;
// First step: set up the Scaffold plug in. Ensure that scaffold operation
// ran. This is more of a control than a test.
$this->mustExec("composer install --no-ansi", $sut);
$this->assertScaffoldedFile($sut . '/sites/default/default.settings.php', FALSE, 'A settings.php fixture file scaffolded from the scaffold-override-fixture');
// Next, bring back packagist.org and install core-composer-scaffold:8.8.0.
// Packagist is disabled in the fixture; we bring it back by removing the
// line that disables it.
$this->mustExec("composer config --unset repositories.packagist.org", $sut);
$stdout = $this->mustExec("composer require drupal/core-composer-scaffold:8.8.0 --no-plugins 2>&1", $sut);
$this->assertContains(" - Installing drupal/core-composer-scaffold (8.8.0):", $stdout);
// Disable packagist.org and upgrade back to the Scaffold plugin under test.
// This puts the `"packagist.org": false` config line back in composer.json
// so that Packagist will no longer be used.
$this->mustExec("composer remove drupal/core-composer-scaffold --no-plugins", $sut);
$this->mustExec("composer config repositories.packagist.org false", $sut);
$stdout = $this->mustExec("composer require drupal/core-composer-scaffold:* 2>&1", $sut);
$this->assertRegExp("#Installing drupal/core-composer-scaffold.*Symlinking from#", $stdout);
// Remove a scaffold file and run the scaffold command again to prove that
// scaffolding is still working.
unlink("$sut/index.php");
$stdout = $this->mustExec("composer scaffold", $sut);
$this->assertContains("Scaffolding files for", $stdout);
$this->assertFileExists("$sut/index.php");
}
}
Markdown is supported
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