Skip to content

draft support for Pickle in Composer #3897

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.

Sign up for GitHub

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

Closed
wants to merge 18 commits into from
Closed

draft support for Pickle in Composer #3897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
daeaebf
register ExtensionInstaller
pierrejoye Apr 1, 2015
1a8ee01
register ExtensionInstaller
pierrejoye Apr 1, 2015
8e850ee
ws
pierrejoye Apr 1, 2015
b353c3f
WS
pierrejoye Apr 1, 2015
14b47e0
default pickle being present in PATH, allow override using COMPOSER_P…
pierrejoye Apr 1, 2015
f85d7be
add pickle version and sanity check, add common process to this insta…
pierrejoye Apr 1, 2015
8929c3a
add pickle version and sanity check, add common process to this insta…
pierrejoye Apr 1, 2015
5bd5e0a
only 'extension' type
pierrejoye Apr 1, 2015
567b07d
drop unnessary arguements or properties
pierrejoye Apr 1, 2015
0bac263
look for pickle only when used
pierrejoye Apr 2, 2015
4aacd4e
start install-extensions
pierrejoye Apr 2, 2015
b708265
some cleanup
pierrejoye Apr 2, 2015
7f85097
start install-extensions
pierrejoye Apr 2, 2015
6206a2c
skip extensiosn earlier, sounds better and no API change required
pierrejoye Apr 3, 2015
df287ee
use cache-file-dir as tmp build dir
pierrejoye Apr 3, 2015
a278fc0
fix bug, continue missing, remove change in the rulegenerator, not re…
pierrejoye Apr 3, 2015
1066f27
WS
pierrejoye Apr 3, 2015
291ea96
restore header
pierrejoye Apr 4, 2015
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
43 changes: 22 additions & 21 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions src/Composer/Command/InstallCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ protected function configure()
new InputOption('verbose', 'v|vv|vvv', InputOption::VALUE_NONE, 'Shows more details including new commits pulled in when updating packages.'),
new InputOption('optimize-autoloader', 'o', InputOption::VALUE_NONE, 'Optimize autoloader during autoloader dump'),
new InputOption('ignore-platform-reqs', null, InputOption::VALUE_NONE, 'Ignore platform requirements (php & ext- packages).'),
new InputOption('install-extensions', null, InputOption::VALUE_NONE, 'install PHP extensions'),
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong indentation (in all lines changed in this file btw)

new InputArgument('packages', InputArgument::IS_ARRAY | InputArgument::OPTIONAL, 'Should not be provided, use composer require instead to add a given package to composer.json.'),
))
->setHelp(<<<EOT
Expand Down Expand Up @@ -122,9 +123,10 @@ protected function execute(InputInterface $input, OutputInterface $output)
->setRunScripts(!$input->getOption('no-scripts'))
->setOptimizeAutoloader($optimize)
->setIgnorePlatformRequirements($input->getOption('ignore-platform-reqs'))
->setInstallExtensions($input->getOption('install-extensions'));
;

if ($input->getOption('no-plugins')) {
if ($input->getOption('no-plugins')) {
$install->disablePlugins();
}

Expand Down
4 changes: 2 additions & 2 deletions src/Composer/DependencyResolver/RuleSetGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ protected function addRulesForJobs($ignorePlatformReqs)
}
}

public function getRulesFor($jobs, $installedMap, $ignorePlatformReqs = false)
public function getRulesFor($jobs, $installedMap, $ignorePlatformReqs = false, $installExtensions = false)
{
$this->jobs = $jobs;
$this->rules = new RuleSet;
Expand All @@ -327,7 +327,7 @@ public function getRulesFor($jobs, $installedMap, $ignorePlatformReqs = false)

$this->addedMap = array();
foreach ($this->installedMap as $package) {
$this->addRulesForPackage($package, $ignorePlatformReqs);
$this->addRulesForPackage($package, $ignorePlatformReqs, $installExtensions);
}

$this->addRulesForJobs($ignorePlatformReqs);
Expand Down
1 change: 1 addition & 0 deletions src/Composer/Factory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ protected function createDefaultInstallers(Installer\InstallationManager $im, Co
$im->addInstaller(new Installer\PearInstaller($io, $composer, 'pear-library'));
$im->addInstaller(new Installer\PluginInstaller($io, $composer));
$im->addInstaller(new Installer\MetapackageInstaller($io));
$im->addInstaller(new Installer\ExtensionInstaller($io, $composer));
}

/**
Expand Down
21 changes: 20 additions & 1 deletion src/Composer/Installer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class Installer
protected $dumpAutoloader = true;
protected $runScripts = true;
protected $ignorePlatformReqs = false;
protected $installExtensions = false;
protected $preferStable = false;
protected $preferLowest = false;
/**
Expand Down Expand Up @@ -488,7 +489,7 @@ protected function doInstall($localRepo, $installedRepo, $platformRepo, $aliases
$links = $this->package->getRequires();
}

foreach ($links as $link) {
foreach ($links as $link) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you should remove these trailing whitespaces

$request->install($link->getTarget(), $link->getConstraint());
}
}
Expand All @@ -499,6 +500,7 @@ protected function doInstall($localRepo, $installedRepo, $platformRepo, $aliases
// solve dependencies
$this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::PRE_DEPENDENCIES_SOLVING, $this->devMode, $policy, $pool, $installedRepo, $request);
$solver = new Solver($policy, $pool, $installedRepo);

try {
$operations = $solver->solve($request, $this->ignorePlatformReqs);
$this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::POST_DEPENDENCIES_SOLVING, $this->devMode, $policy, $pool, $installedRepo, $request, $operations);
Expand All @@ -523,6 +525,10 @@ protected function doInstall($localRepo, $installedRepo, $platformRepo, $aliases
foreach ($operations as $operation) {
// collect suggestions
if ('install' === $operation->getJobType()) {
if (!$this->installExtensions && $operation->getPackage()->getType() == 'extension') {
$this->io->writeError('<warning>This package has extensions dependencies, run composer install --install-extensons</warning>');
continue;
}
Copy link
Member

Choose a reason for hiding this comment

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

Should maybe support 'update' operations too

Copy link
Author

Choose a reason for hiding this comment

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

If an extension is loaded it cannot be updated. That's the tricky part. It
is also why we somehow need to be able to run pickle against a different
php than the one running pickle. Or use -n but then... :)
On May 5, 2015 5:11 PM, "Jordi Boggiano" notifications@github.com wrote:

In src/Composer/Installer.php
#3897 (comment):

@@ -523,6 +525,10 @@ protected function doInstall($localRepo, $installedRepo, $platformRepo, $aliases
foreach ($operations as $operation) {
// collect suggestions
if ('install' === $operation->getJobType()) {

  •            if (!$this->installExtensions && $operation->getPackage()->getType() ==  'extension') {

  •                $this->io->writeError('<warning>This package has extensions dependencies, run composer install --install-extensons</warning>');

  •                continue;

  •            }

Should maybe support 'update' operations too

—
Reply to this email directly or view it on GitHub
https://github.com/composer/composer/pull/3897/files#r29656783.

Copy link
Member

@Seldaek Seldaek May 5, 2015 via email

Choose a reason for hiding this comment

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

foreach ($operation->getPackage()->getSuggests() as $target => $reason) {
Copy link
Author

Choose a reason for hiding this comment

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

@nadernan this is actually where it skips extension. I suppose the part in the other file is not required anymore, right? Can test again tomorrow but advice's welcome

Copy link
Author

Choose a reason for hiding this comment

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

So no, it is still required.

$this->suggestedPackages[] = array(
'source' => $operation->getPackage()->getPrettyName(),
Expand Down Expand Up @@ -1253,6 +1259,19 @@ public function setIgnorePlatformRequirements($ignorePlatformReqs = false)
return $this;
}

/**
* set install Extensions flag
*
* @param boolean $installExtensions
* @return Installer
*/
public function setInstallExtensions($installExtensions = false)
{
$this->installExtensions = (boolean) $installExtensions;

return $this;
}

/**
* restrict the update operation to a few packages, all other packages
* that are already installed will be kept at their current version
Expand Down
Loading