Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add prefer-stable flag to pick stable package over unstable ones when possible #1740

Merged
merged 2 commits into from
@Seldaek
Owner

The thing with dev packages, and especially with some libs requiring dev packages themselves, is that people are often forced to set minimum-stability to dev to avoid going mad tracking deep dependencies. That leads to everything being dev, which is clearly not ideal for most usages. This provides a new flag that tells composer to pick more stable versions when it can, while still allowing dev versions to be installed if that's the only option.

Add "prefer-stable":true in a composer.json that has "minimum-stability" set to something else than stable, and see what happens with composer update --dry-run. It should update packages to stable versions when it can.

If you really want a dev version, instead of just requiring it with @dev like before, you should require explicitly the version you want, e.g. 1.0.x-dev or dev-master.

Happy easter :rabbit:

@stof

The doc needs to be updated too

@Seldaek
Owner

Yeah it's not polished, but I don't want to spend time updating docs until this is proven to be a good approach.

@stof

The idea seems good at least.

@naderman
Owner

PR looks good to me +1

@lsmith77

+1 on the idea.

@schmittjoh

Idea sounds really good.

@trq

:+1: Awesome idea!

@rande

+1

@everzet

Why not making it true by default?

@adrianmacneil

This isn't the best solution. You haven't solved the problem, which you define as:

The thing with dev packages, and especially with some libs requiring dev packages themselves, is that people are often forced to set minimum-stability to dev to avoid going mad tracking deep dependencies

I will still need to set "minimum-stability": "dev" in pretty much every project to avoid "going mad tracking deep dependencies", which makes minimum-stability flag useless, and this new prefer-stable flag just another setting I have to add to every project by default.

A better solution would be if the minimum-stability flag only affected packages actually required in composer.json, and not sub-dependencies. There's no need to ever set a minimum stability for sub-dependencies, by definition I don't care about them, as long as they meet the requirements of packages I depend on.

That way, for most projects you would leave the minimum-stability line out altogether and get stable versions of any packages you specify, and for any sub-dependencies you would get the most stable version available. If you want beta/dev versions of any package, you can specify that requirement in your composer.json file.

@stof

@adrianmacneil IMO, this feature solves the issue: it allows to prefer stable packages when available while still allowing alpha or dev packages when they are needed to match the requirement.

Making minimum-stability affecting only the root requirements is a bad idea IMO:

  • if I want to forbid dev versions (some companies are against using dev packages), it should affect all packages, not only a subset of them
  • your logic would mean adding all packages in the root requirement to force them to be stable. But this means that you require the developer to resolve the dependency graph manually to find the whole list, thus defeating the goal of a dependency manager (which is meant to manage them for you). My current project defines 34 requirements in the root package, but uses 74 packages. I don't want to list them all (and to update the list each time a dev modifies the dependencies of a library)
@hason

Maybe it would be good to have a parameter allow-dev for the case described in #1436 and in @adrianmacneil comment:

"minimum-stability": "stable",
"allow-dev": "true"
@adrianmacneil

It's not just dev packages, beta packages are a problem too. The case where you want to prevent anything but stable is much less common than the case where you just want composer to install what you asked for without complaining. Currently, as proposed this solution means you need to add two lines to pretty much every composer file just to avoid weird package dependency issues. I'm sure we can find a solution which means for the most common requirement no extra configuration is necessary.

Here's an example composer.json which is causing me grief:

{
    "require": { "illuminate/database": "v4.0.0-BETA3" }
}

RIght now, composer can't solve that, which I believe is a problem that needs fixing. Composer has no problem with me specifying the beta package, but because that beta package depends on other beta packages, it complains.

Maybe a better solution would simply be to leave the minimum-stability setting alone, but automatically allow packages to depend on other packages of equal or higher stability. Either way, I think composer should prefer using the most stable version of a package which meets the requirements (essentially making this new prefer-stable flag true by default). It seems very unlikely that you would want the latest dev-master of every package in your requirements, and if you ever did you are still free to specify -dev-master on a per-package basis.

@bamarni

I share @adrianmacneil concern, the main issue is that one would still have to set a global stability policy to dev, only because of maybe a few packages, I guess we all do that (just not get mad as you said :)).

What about his example, I don't exactly know how composer behaves in that case? Maybe it's because the package requirment should be like "4.0.0@beta"? In that situation imo composer should also allow beta packages for deeper requirments of this illuminate/database package (if it's not already the case, sorry I haven't tested it).

@Seldaek
Owner

@adrianmacneil the problem you describe with beta packages I believe comes from the fact that the whole illuminate stuff is still an unstable ecosystem. That means for projects using this yes you must have a dev or at least beta minimum-stability. That is not the case for everything though. In symfony projects nowadays I can mostly get away with the default stable setting, and require a couple bundles in dev versions. And even that part would not be necessary if bundle authors would tag more often.

Now regarding having it read the stability requirements from the packages you require, that can't really work for similar reasons as the recursive repositories problem. Another reason is that it would take control away from the root package, which is not acceptable to many people that want tight control over their dependencies.

@hason not sure what that would bring? If you want to allow unstable packages, you lower the minimum stability and you're done. I suppose that would work easier if we make prefer-stable true by default.


Finally on the topic of enabling this by default. I think it might indeed be good, but I'd like to hear from anyone that thinks that's a bad idea.

@bamarni

:+1: for making this as default then, thx for your explanations @Seldaek

@jeromemacias

+1 and making this by default

@Taluu

:+1: for this idea, and making it by default. :)

@Seldaek
Owner

OK it's now on by default and I added docs. If there are no objections will try to merge this soon (it probably could use a blog post to clarify things - also have other stuff I want to write about).

@Seldaek
Owner

OK I realized there is a not so cool side effect of having this on by default. If you have the default minimum-stability (stable), and just want to get one package in dev, you can for now require: "foo/bar": "2.*@dev" for example. But doing so with prefer-stable enabled won't change anything, you'll still get a stable.

So I'd say either we handle those flagged packages specifically (I guess disabling the prefer-stable behavior for them?) or we disable this by default. I think disabled by default might be best because the more "automatic" things we add the more confused people get. Also it would avoid some WTFs for people updating their existing projects.

@everzet

@Seldaek even if I'll have both "prefer-stable":true and "minimum-stability": "dev" set in composer.json manually, I'll still expect "foo/bar": "2.*@dev" to install dev version of the package as that's what I explicitly stated it to do.

If it's not the case currently, I'll assume it's broken. And if feature is broken - it doesn't matter whether if it is enabled by default or not :)

@naderman
Owner

2.@dev was never meant to always install dev versions. The @dev is meant to set minimum stability. Meaning you want a 2. version, preferably stable but are allowing a dev version. If you want the dev version explicitly use 2.x-dev.

@everzet

@naderman ah, ok. My bad then.

@Seldaek
Owner

OK updated the code back to disabled by default, updated the PR description with details on usage, merging.

@Seldaek Seldaek merged commit 79100be into composer:master

1 check passed

Details default The Travis build passed
@lsmith77 lsmith77 referenced this pull request from a commit in jackalope/jackalope-jackrabbit
@dbu dbu jackalope core now does transform node types to cnd a49d269
@bartkamphuis bartkamphuis referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@phoenixgao phoenixgao referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
7 doc/04-schema.md
@@ -493,6 +493,13 @@ a given package can be done in `require` or `require-dev` (see
Available options (in order of stability) are `dev`, `alpha`, `beta`, `RC`,
and `stable`.
+### prefer-stable <span>(root-only)</span>
+
+When this is enabled, Composer will prefer more stable packages over unstable
+ones when finding compatible stable packages is possible. If you require a
+dev version or only alphas are available for a package, those will still be
+selected granted that the minimum-stability allows for it.
+
### repositories <span>(root-only)</span>
Custom package repositories to use.
View
4 res/composer-schema.json
@@ -221,6 +221,10 @@
"type": ["string"],
"description": "The minimum stability the packages must have to be install-able. Possible values are: dev, alpha, beta, RC, stable."
},
+ "prefer-stable": {
+ "type": ["boolean"],
+ "description": "If set to true, stable packages will be prefered to dev packages when possible, even if the minimum-stability allows unstable packages."
+ },
"bin": {
"type": ["array"],
"description": "A set of files that should be treated as binaries and symlinked into bin-dir (from config).",
View
13 src/Composer/DependencyResolver/DefaultPolicy.php
@@ -14,15 +14,28 @@
use Composer\Package\PackageInterface;
use Composer\Package\AliasPackage;
+use Composer\Package\BasePackage;
use Composer\Package\LinkConstraint\VersionConstraint;
/**
* @author Nils Adermann <naderman@naderman.de>
+ * @author Jordi Boggiano <j.boggiano@seld.be>
*/
class DefaultPolicy implements PolicyInterface
{
+ private $preferStable;
+
+ public function __construct($preferStable = false)
+ {
+ $this->preferStable = $preferStable;
+ }
+
public function versionCompare(PackageInterface $a, PackageInterface $b, $operator)
{
+ if ($this->preferStable && ($stabA = $a->getStability()) !== ($stabB = $b->getStability())) {
+ return BasePackage::$stabilities[$stabA] < BasePackage::$stabilities[$stabB];
+ }
+
$constraint = new VersionConstraint($operator, $b->getVersion());
$version = new VersionConstraint('==', $a->getVersion());
View
9 src/Composer/Installer.php
@@ -234,7 +234,7 @@ public function run()
// split dev and non-dev requirements by checking what would be removed if we update without the dev requirements
if ($this->devMode && $this->package->getDevRequires()) {
- $policy = new DefaultPolicy();
+ $policy = $this->createPolicy();
$pool = $this->createPool();
$pool->addRepository($installedRepo, $aliases);
@@ -308,7 +308,7 @@ protected function doInstall($localRepo, $installedRepo, $platformRepo, $aliases
$this->io->write('<info>Loading composer repositories with package information</info>');
// creating repository pool
- $policy = new DefaultPolicy();
+ $policy = $this->createPolicy();
$pool = $this->createPool();
$pool->addRepository($installedRepo, $aliases);
if ($installFromLock) {
@@ -513,6 +513,11 @@ private function createPool()
return new Pool($minimumStability, $stabilityFlags);
}
+ private function createPolicy()
+ {
+ return new DefaultPolicy($this->package->getPreferStable());
+ }
+
private function createRequest(Pool $pool, RootPackageInterface $rootPackage, PlatformRepository $platformRepo)
{
$request = new Request($pool);
View
4 src/Composer/Package/Loader/RootPackageLoader.php
@@ -91,6 +91,10 @@ public function load(array $config, $class = 'Composer\Package\RootPackage')
$package->setMinimumStability(VersionParser::normalizeStability($config['minimum-stability']));
}
+ if (isset($config['prefer-stable'])) {
+ $package->setPreferStable((bool) $config['prefer-stable']);
+ }
+
$repos = Factory::createDefaultRepositories(null, $this->config, $this->manager);
foreach ($repos as $repo) {
$this->manager->addRepository($repo);
View
19 src/Composer/Package/RootPackage.php
@@ -20,6 +20,7 @@
class RootPackage extends CompletePackage implements RootPackageInterface
{
protected $minimumStability = 'stable';
+ protected $preferStable = false;
protected $stabilityFlags = array();
protected $references = array();
@@ -60,6 +61,24 @@ public function getStabilityFlags()
}
/**
+ * Set the preferStable
+ *
+ * @param bool $preferStable
+ */
+ public function setPreferStable($preferStable)
+ {
+ $this->preferStable = $preferStable;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ public function getPreferStable()
+ {
+ return $this->preferStable;
+ }
+
+ /**
* Set the references
*
* @param array $references
View
29 tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php
@@ -65,6 +65,35 @@ public function testSelectNewest()
$this->assertEquals($expected, $selected);
}
+ public function testSelectNewestPicksLatest()
+ {
+ $this->repo->addPackage($packageA1 = $this->getPackage('A', '1.0.0'));
+ $this->repo->addPackage($packageA2 = $this->getPackage('A', '1.0.1-alpha'));
+ $this->pool->addRepository($this->repo);
+
+ $literals = array($packageA1->getId(), $packageA2->getId());
+ $expected = array($packageA2->getId());
+
+ $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals);
+
+ $this->assertEquals($expected, $selected);
+ }
+
+ public function testSelectNewestPicksLatestStableWithPreferStable()
+ {
+ $this->repo->addPackage($packageA1 = $this->getPackage('A', '1.0.0'));
+ $this->repo->addPackage($packageA2 = $this->getPackage('A', '1.0.1-alpha'));
+ $this->pool->addRepository($this->repo);
+
+ $literals = array($packageA1->getId(), $packageA2->getId());
+ $expected = array($packageA1->getId());
+
+ $policy = new DefaultPolicy(true);
+ $selected = $policy->selectPreferedPackages($this->pool, array(), $literals);
+
+ $this->assertEquals($expected, $selected);
+ }
+
public function testSelectNewestOverInstalled()
{
$this->repo->addPackage($packageA = $this->getPackage('A', '2.0'));
Something went wrong with that request. Please try again.