Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #67: Adds ability to disable all or certain installers #376

Merged
merged 17 commits into from
Apr 5, 2018
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ is not needed to install packages with these frameworks:
| Decibel | `decibel-app`
| DokuWiki | `dokuwiki-plugin`<br>`dokuwiki-template`
| Dolibarr | `dolibarr-module`
| Drupal | <b>`drupal-core`<br>`drupal-module`<br>`drupal-theme`</b><br>`drupal-library`<br>`drupal-profile`<br>`drupal-drush`
| Drupal | <b>`drupal-core`<br>`drupal-module`<br>`drupal-theme`</b><br>`drupal-library`<br>`drupal-profile`<br>`drupal-drush`<br>`drupal-custom-theme`<br>`drupal-custom-module`
| Elgg | `elgg-plugin`
| Eliasis | `eliasis-component`<br>`eliasis-module`<br>`eliasis-plugin`<br>`eliasis-template`
| ExpressionEngine 3 | `ee3-addon`<br>`ee3-theme`
Expand Down Expand Up @@ -207,6 +207,51 @@ will allow this:
Please note the name entered into `installer-name` will be the final and will
not be inflected.

## Disabling installers

There may be time when you want to disable one or more installers from `composer/installers`.
For example, if you are managing a package or project that uses a framework specific installer that
conflicts with `composer/installers` but also have a dependency on a package that depends on `composer/installers`.

Installers can be disabled for your project by specifying the extra
`installer-disable` property. If set to `true`, `"all"`, or `"*"` all installers
will be disabled.

```json
{
"extra": {
"installer-disable": true
}
}
```

Otherwise a single installer or an array of installers may be specified.

```json
{
"extra": {
"installer-disable": [
"cakephp",
"drupal"
]
}
}
```

**Note:** Using a global disable value (`true`, `"all"`, or `"*"`) will take precedence over individual
installer names if used in an array. The example below will disable all installers.

```json
{
"extra": {
"installer-disable": [
"drupal",
"all"
]
}
}
```

## Should we allow dynamic package types or paths? No.

What are they? The ability for a package author to determine where a package
Expand Down
2 changes: 1 addition & 1 deletion src/Composer/Installers/DrupalInstaller.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ class DrupalInstaller extends BaseInstaller
'profile' => 'profiles/{$name}/',
'drush' => 'drush/{$name}/',
'custom-theme' => 'themes/custom/{$name}/',
'custom-module' => 'modules/custom/{$name}',
'custom-module' => 'modules/custom/{$name}/',
);
}
64 changes: 64 additions & 0 deletions src/Composer/Installers/Installer.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,30 @@ class Installer extends LibraryInstaller
'prestashop' => 'PrestashopInstaller'
);

/**
* Installer constructor.
*
* Disables installers specified in main composer extra installer-disable
* list
*
* @param IOInterface $io
* @param \Composer\Composer $composer
* @param string $type
* @param \Composer\Util\Filesystem|null $filesystem
* @param \Composer\Installer\BinaryInstaller|null $binaryInstaller
*/
public function __construct(
\Composer\IO\IOInterface $io,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these are already imported, can you just use their short classname? Same for docblocks please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

\Composer\Composer $composer,
$type = 'library',
\Composer\Util\Filesystem $filesystem = null,
\Composer\Installer\BinaryInstaller $binaryInstaller = null
) {
parent::__construct($io, $composer, $type, $filesystem,
$binaryInstaller);
$this->removeDisabledInstallers();
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -198,4 +222,44 @@ private function getIO()
{
return $this->io;
}

/**
* Look for installers set to be disabled in composer's extra config and
* remove them from the list of supported installers.
* Uses true, "all", and "*" as global values to disable all installers.
*
* @return void
*/
protected function removeDisabledInstallers()
{
$extra = $this->composer->getPackage()->getExtra();

if (!isset($extra['installer-disable'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth to consider options [] and false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that false is the equivalent of not adding this option, I hadn't considered it before. Now that you bring it up I can see where it would be useful in a situation where wikimedia/composer-merge-plugin is being used. I'll add it as an option.

Is your thought that this would be a global, "all installers are enabled"?

Regarding an options array, what did you have in mind? I'm not at all opposed to having available options, but I'm not sure what options to include or what they would do? Or are you suggesting to have it available for future ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niksamokhvalov
Just wondering if you had a chance to review the latest commit updating this code. I've added false as an option, but am unsure what options[] you had in mind. I'm open to the idea, but am just unsure of what or how to implement.

// No installers are disabled
return;
}

// Get installers to disable
$disable = $extra['installer-disable'];

// Ensure $disabled is an array
if (!is_array($disable)) {
$disable = array($disable);
}

// Check which installers should be disabled
$all = array(true, "all", "*");
$intersect = array_intersect($all, $disable);
if (!empty($intersect)) {
// Disable all installers
$this->supportedTypes = array();
} else {
// Disable specified installers
foreach ($disable as $key => $installer) {
if (is_string($installer) && key_exists($installer, $this->supportedTypes)) {
unset($this->supportedTypes[$installer]);
}
}
}
}
}
79 changes: 64 additions & 15 deletions tests/Composer/Installers/Test/InstallerTest.php
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<?php
namespace Composer\Installers\Test;

use Composer\Composer;
use Composer\Config;
use Composer\Installers\Installer;
use Composer\Util\Filesystem;
use Composer\Package\Package;
use Composer\Package\RootPackage;
use Composer\Composer;
use Composer\Config;
use Composer\Util\Filesystem;

class InstallerTest extends TestCase
{
Expand Down Expand Up @@ -52,6 +52,10 @@ public function setUp()

$this->repository = $this->getMock('Composer\Repository\InstalledRepositoryInterface');
$this->io = $this->getMock('Composer\IO\IOInterface');

$consumerPackage = new RootPackage('foo/bar', '1.0.0', '1.0.0');
$this->composer->setPackage($consumerPackage);

}

/**
Expand Down Expand Up @@ -116,7 +120,14 @@ public function dataForTestSupport()
array('decibel-app', true),
array('dokuwiki-plugin', true),
array('dokuwiki-template', true),
array('drupal-core', true),
array('drupal-module', true),
array('drupal-theme', true),
array('drupal-library', true),
array('drupal-profile', true),
array('drupal-drush', true),
array('drupal-custom-theme', true),
array('drupal-custom-module', true),
array('dolibarr-module', true),
array('ee3-theme', true),
array('ee3-addon', true),
Expand Down Expand Up @@ -279,10 +290,14 @@ public function dataForTestInstallPath()
array('dokuwiki-plugin', 'lib/plugins/someplugin/', 'author/someplugin'),
array('dokuwiki-template', 'lib/tpl/sometemplate/', 'author/sometemplate'),
array('dolibarr-module', 'htdocs/custom/my_module/', 'shama/my_module'),
array('drupal-core', 'core/', 'drupal/core'),
array('drupal-module', 'modules/my_module/', 'shama/my_module'),
array('drupal-theme', 'themes/my_module/', 'shama/my_module'),
array('drupal-profile', 'profiles/my_module/', 'shama/my_module'),
array('drupal-drush', 'drush/my_module/', 'shama/my_module'),
array('drupal-theme', 'themes/my_theme/', 'shama/my_theme'),
array('drupal-library', 'libraries/my_library/', 'shama/my_library'),
array('drupal-profile', 'profiles/my_profile/', 'shama/my_profile'),
array('drupal-drush', 'drush/my_command/', 'shama/my_command'),
array('drupal-custom-theme', 'themes/custom/my_theme/', 'shama/my_theme'),
array('drupal-custom-module', 'modules/custom/my_module/', 'shama/my_module'),
array('elgg-plugin', 'mod/sample_plugin/', 'test/sample_plugin'),
array('eliasis-component', 'components/my_component/', 'shama/my_component'),
array('eliasis-module', 'modules/my_module/', 'shama/my_module'),
Expand Down Expand Up @@ -433,9 +448,7 @@ public function testCustomInstallPath()
$installer = new Installer($this->io, $this->composer);
$package = new Package('shama/ftp', '1.0.0', '1.0.0');
$package->setType('cakephp-plugin');
$consumerPackage = new RootPackage('foo/bar', '1.0.0', '1.0.0');
$this->composer->setPackage($consumerPackage);
$consumerPackage->setExtra(array(
$this->composer->getPackage()->setExtra(array(
'installer-paths' => array(
'my/custom/path/{$name}/' => array(
'shama/ftp',
Expand Down Expand Up @@ -470,9 +483,7 @@ public function testCustomTypePath()
$installer = new Installer($this->io, $this->composer);
$package = new Package('slbmeh/my_plugin', '1.0.0', '1.0.0');
$package->setType('wordpress-plugin');
$consumerPackage = new RootPackage('foo/bar', '1.0.0', '1.0.0');
$this->composer->setPackage($consumerPackage);
$consumerPackage->setExtra(array(
$this->composer->getPackage()->setExtra(array(
'installer-paths' => array(
'my/custom/path/{$name}/' => array(
'type:wordpress-plugin'
Expand All @@ -491,9 +502,7 @@ public function testVendorPath()
$installer = new Installer($this->io, $this->composer);
$package = new Package('penyaskito/my_module', '1.0.0', '1.0.0');
$package->setType('drupal-module');
$consumerPackage = new RootPackage('drupal/drupal', '1.0.0', '1.0.0');
$this->composer->setPackage($consumerPackage);
$consumerPackage->setExtra(array(
$this->composer->getPackage()->setExtra(array(
'installer-paths' => array(
'modules/custom/{$name}/' => array(
'vendor:penyaskito'
Expand Down Expand Up @@ -549,4 +558,44 @@ public function testUninstallAndDeletePackageFromLocalRepo()

$installer->uninstall($repo, $package);
}

/**
* testDisabledInstallers
*
* @dataProvider dataForTestDisabledInstallers
*/
public function testDisabledInstallers($disabled, $type, $expected)
{
$this->composer->getPackage()->setExtra(array(
'installer-disable' => $disabled,
));
$this->testSupports($type, $expected);
}

/**
* dataForTestDisabledInstallers
*
* @return array
*/
public function dataForTestDisabledInstallers()
{
return array(
array(false, "drupal-module", true),
array(true, "drupal-module", false),
array("true", "drupal-module", true),
array("all", "drupal-module", false),
array("*", "drupal-module", false),
array("cakephp", "drupal-module", true),
array("drupal", "cakephp-plugin", true),
array("cakephp", "cakephp-plugin", false),
array("drupal", "drupal-module", false),
array(array("drupal", "cakephp"), "cakephp-plugin", false),
array(array("drupal", "cakephp"), "drupal-module", false),
array(array("cakephp", true), "drupal-module", false),
array(array("cakephp", "all"), "drupal-module", false),
array(array("cakephp", "*"), "drupal-module", false),
array(array("cakephp", "true"), "drupal-module", true),
array(array("drupal", "true"), "cakephp-plugin", true),
);
}
}