Limit Replace / Provides to packages required by name in root package or any dep #2690

naderman · 2014-02-11T00:00:20Z

Problem

Replace

Original Goal

Allow a fork to state that it is compatible with another package's API so that packages depending on the original are satisfied by the fork as well. Users may intentionally pick the fork, or if the original is poorly maintained the fork may be picked automatically.

Issue

People publish potentially malicious forks that replace commonly used libraries/frameworks. Users construct dependencies which make it impossible to install the original correctly. The solver decides to install the potentially malicious fork instead - users do not expect nor understand this and may not check the results of the update process because they trust composer too much.

Provides

Original Goal

Allow packages depend on a virtual package - a name which is not an actual package but merely represents an interface. There may then be multiple packages which are all treated equally which provide implementations of the same interface. The user may either pick one of the implementations directly with a require, or he may imply which one is to be used through another dependency.

Issue

If no provider is directly specified or specified through dependencies, it is randomly selected. Users may be ok with the initial choice, but they do not expect a change when running composer update to a potentially malicious package which also provides the same virtual package.

Proposed Solution

Only packages which match a dependency _by name_ (ignoring replaces/providers) or a dependency of any potential dependency identified recursively _by name_ will be considered for installation. The error reporting should further query for alternative replacers/providers which the user can require in the root package to satisfy the dependency. This is related to the improved error reporting utility necessary as a consequence of #2661

Consequences

Positive

We should see fewer users confused by the packages composer picks
Less potential for malicious packages to be installed if users run update automatically on CI or production (_HINT: DO NOT RUN COMPOSER UPDATE IN ANY AUTOMATED FASHION_)
We may be able to reduce the composer memory footprint and complexity on packagist as we no longer need a list of all providers/replacers for installation but only for error reporting

Negative

There may be situations in which Composer will now not be able to update anymore without explicit specification of a dependency, but this should be easy to understand for users. And explicity may even be helpful.

Ocramius · 2014-02-11T00:13:07Z

👍

Ping @waltertamboer: this is the issue that was affecting your package.

lstrojny · 2014-02-11T00:37:18Z

We use replace with a Magento project to make sure, no dependency installs another version of Zend Framework 1 (which is already bundled with Magento). What would be the alternative for that behavior?

Ocramius · 2014-02-11T00:52:46Z

@lstrojny it would work the same, but the top level composer.json must include a reference by name to the provided fork in order for replace or provides to work

naderman · 2014-02-11T01:12:32Z

@Ocramius @lstrojny Meaning in his case, since they use replace on the package itself if I understand that correctly, nothing would change for them at all, since the root package is installed anyway.

lstrojny · 2014-02-11T08:55:07Z

@Ocramius, @naderman Magento itself is not handled via Magento. Download the tarball, add a composer.json and add a replace-directive. Still works in the future?

rk3rn3r · 2014-02-11T11:01:30Z

what we are using atm:

8000 we have a patched symfony 2.X[.Y] in use, because on some points we needed things that were not able to be injected.

we maintain patched 2.X.Y tags for every symfony 2.X.Y that we have in use, this could be 2.0.9, 2.0.15, 2.1.13 etc. and we also provide 2.X.* branches.

in our composer.json we have

"name": "trivago/symfony",
...
"replace": {
    "symfony/symfony",
    ... // components listing ommited
}

our applications for example have:

"require": 
    "trivago/symfony": "2.0.15"

this makes sure:

we only install our internal patched symfony 2
we never load symfony 2 updated dependencies, as long we wanted to update them
tools that depend on symfony 2 will use our components instead of official / unpatched ones
- for example a library lib1 from github has "require": "symfony/console" than trivago/symfony will satisfy this dependency

This is what we really need to make sure everything works quite well and I don't know if I get your suggestion right about changing this behaviour. Do you think it will work?

Ocramius · 2014-02-12T20:35:30Z

@rk3rn3r yes, that will work.

Seldaek · 2014-02-17T17:10:49Z

@naderman I agree the plan looks great on paper. I'm not entirely sure what it means in the code though. Or rather not sure if we'll really save much memory if we still have to load all deps versions recursively to check for things.

Also one issue right now is that when loading symfony/symfony in the solver we also load all the symfony components because they're replaced and the exclusive-or (whatever they're called) rules must be added to the solver for each package replaced. I'm not sure if this model improves on that or not, but it'd be great if so.

naderman · 2014-02-17T17:21:16Z

I do think this will require quite a few large changes to how the RuleSetGenerator works. I would probably implement it as two passes. One in which we only recursively determine which packages are even explicitly named in any dep as a require - these would currently be loaded too. But then we no longer need to load any potential replacers in the second pass unless they were explicitly mentioned in the first pass. So in a lot of cases that should be avoidable.

lmammino · 2014-02-21T09:55:49Z

Does the replace feature exists in other package manager such as gem or npm? If yes how do they handle similar situations?

naderman · 2014-02-21T09:58:02Z

It exists in pretty much every linux distribution's package manager. The dependency management in Composer stems from Suse's RPM installer.

lmammino · 2014-02-21T10:13:47Z

How they managed to avoid such situation (if they did)? I don't think we should reinvent the wheel if a good solution exists...

naderman · 2014-02-21T10:17:43Z

They don't have open repositories like packagist.

…ing rules Addresses composer#2690 doesn't do any performance optimisations yet which we could do now

naderman · 2014-02-21T12:47:12Z

This should be fixed now, let me know if any issues arise.

lsmith77 · 2014-02-21T12:51:15Z

@naderman I assume you will write a follow up blog post .. or maybe better yet update the original blog post?

Seldaek · 2014-02-28T10:20:16Z

As I understand it you have drupal core and drupal modules running as two different composer projects, or rather core shipping with dependencies inlined and then a proper composer project including modules and their dependencies. Then you have two autoloaders: 1) drupal core & core deps, 2) drupal modules & deps.

So if a dep requires guzzle, guzzle will be installed in the module deps but a part of it is already present in drupal core, so it conflicts.

One way I see that you can maybe solve this (assuming the module composer stuff is ran with composer as library i.e. you have programmatic control over it) is by adding a repository with higher priority than packagist, and that repo would contain a guzzle/guzzle package that would be a metapackage, replaces guzzle/whatever-drupal-core-uses, and require all the other bits of guzzle.

That said, this sounds extremely error prone to me. You'll need to maintain this list to match what you use in drupal core, you'll have to maintain the list of requires with what guzzle/guzzle (& symfony etc) replace, and possibly other problems I don't think of right now.

To sum it up, I think the right way would be having one project with everything (drupal core + modules + deps of everyone). It would simplify all that greatly and make drupal behave like all other projects. You could still ship a zip archive to people which includes all core dependencies so they can get started easy, but then the update process would run a composer update then run migration scripts, and the module installer would modify the composer.json and run an update.

donquixote · 2014-02-28T10:31:38Z

(this is going to be a cross-post with Jordi but I'm posting it anyway)

@stof: indeed, doctrine has no place in this list.
What I could find is these composer.json files with a huge "replace" section but no mention of "metapackage".

So the scenario would be if your root package has a "replace" for e.g. "symfony/yaml", but some indirectly required package of which you don't know yet requires "symfony/symfony".

The solution is to require a meta package which resovles this conflict - like a "symfony-require-all". But I only want to require this if really needed by a vendor package - which I don't know yet. So basically I want to put the 3rd package on the whitelist and allow Composer to use it if needed to resolve the conflicts.

An alternative could be to process the "require" list somewhere in the middle of the process, and conditionally add the metapackage, or simply add all sub-packages explicitly and remove the "symfony/symfony" uber-package.

stof · 2014-02-28T10:38:27Z

@donquixote a metapackage is a package with no code at all in it. symfony/symfony is not a metapackage. It has code in it.

@Seldaek The case of having 2 levels of composer resolution (once for the CMS and once for the project based on it) has been resolved by @simensen for Sculpin by creating https://github.com/dflydev/dflydev-embedded-composer

Seldaek · 2014-02-28T10:44:01Z

@stof yup but I'm not quite sure how this embedded composer model fares with this situation.

Anyway @donquixote another option might be to just add conflict rules for all versions of guzzle/guzzle and symfony/symfony and whatever else you need. That way it should prevent the modules from depending on any of those packages, and they'll have to depend on the subpackages they need. It's not great but it might be the cheapest to implement for you at this point. Unless @simensen says his thing works perfectly fine with this edge case then it may be a better option.

donquixote · 2014-02-28T10:48:24Z

@stof (point 1): Ok. This was refering to KingCrunch who suggested that the approach of guzzle of not using metapackage is flawed. My point was that "others are doing it too", so then symfony/symfony and others are also doing it wrong.

@stof (point 2): I am going to look at dflydev asap, after this post.

@Seldaek:
I would agree, but

It is always hard to estimate for me if a suggested change in Drupal 8 core is realistic or not. This one would be quite a big and consequential change. It might be worth it, but it is not my decision to make, and it is surely going to be a long discussion. Atm I am just exploring the possibilities.
In the current work flow, it is desirable to have some Composer packages available in Drupal core, before we know which multisite directory we are in, and which modules are enabled. This is a nasty chicken-and-egg problem, and having two separate Composer instances is a tempting way out.

You can have conflicting "require" statements in a non-Drupal or non-CMS scenario. The main difference, I think, will be that in a typical Composer scenario you know your packages, so you don't need any "conditional" stuff. You simply add "symfony-require-all" in the "require" section, and don't need any conditional whitelist.

In this case, a "whitelist" could still make sense if you follow a philosophy that is more agnostic of your indirect packages requirements.

donquixote · 2014-02-28T10:50:02Z

That way it should prevent the modules from depending on any of those packages, and they'll have to depend on the subpackages they need.

This can work for modules' direct requirements, but not for modules that require composer packages they have no control about, and this package requires guzzle/guzzle.

kingcrunch · 2014-02-28T10:50:53Z

@donquixote

Ok. This was refering to KingCrunch who suggested that the approach of guzzle
of not using metapackage is flawed. My point was that "others are doing it too", so
then symfony/symfony and others are also doing it wrong.

"Wrong" is such a harsh word. But yeah, thats my opinion. 😄

stof · 2014-02-28T11:04:31Z

@Seldaek EmbeddedComposer registers the packages installed in Sculpin core itself as a platform repository during the installation of the project.
However, Sculpin itself can also be installed as a normal composer dependency for projects fully managed through composer. the embedded composer stuff is used when using the sculpin phar (which includes Sculpin and its deps, and these deps cannot be updated during the installation of plugins because the phar cannot be changed).
I'm not sure it will fit with the case of Drupal where the core deps are copied to the core repo itself.

holtkamp · 2014-03-02T16:46:27Z

Even with this functionality implemented, it seems Composer resolves a combination of dependencies to the official Zend Framework 1 package to an officious 'dummy' package as registered at Packagist:
https://packagist.org/packages/joshribakoff/zf1-empty, which uses version '*': the most general version.

I tried to include a reference to a patched version of this dummy project as a workaround, but that resulted in an unresolvable set of dependencies):
joshribakoff/zf1-empty#1 by @igorw

It seems the only way to prevent the zf1-empty project to be installed is to use patches of all packages that depend on the Zend Framework to let them require a non-specific version:

"zendframework/zendframework1": "*",

For example: https://github.com/holtkamp/zendframework1-doctrine2/blob/patch-composer-prevent-zf1-empty/composer.json#L4
How can I enable the suggested 'strict name matching' for packages in Packagist, is this even possible?

BTW, this started in composer/packagist#362 and I already suggested @joshribakoff to unregister the package from Packagist, joshribakoff/zf1-empty#2

Seldaek · 2014-03-03T22:53:38Z

@holtkamp are you sure you're using the latest composer version (run self-update)? If so, can you post a composer.json that would allow us to reproduce this?

holtkamp · 2014-03-03T23:58:25Z

@Seldaek, yeah, using the latest version fff913d.

When using https://gist.github.com/holtkamp/9337191 and then running

php composer.phar update --no-dev --verbose --profile

results in https://gist.github.com/holtkamp/9337203

What I think is that Composer deduces the most suitable version of the Zend Framework to be '*', which is provided 'only' by that dummy zf1-empty project at https://packagist.org/packages/joshribakoff/zf1-empty

The (annoying way) to circumvent it is by patching some packages to decrease the required version, also to '*'. For example: https://github.com/holtkamp/zendframework1-doctrine2/tree/patch-composer-prevent-zf1-empty

And then using https://gist.github.com/holtkamp/9337260 works. The more packages involved, the more annoying it gets to keep up with all those patched versions.

I hope this clarifies the issue...

joshribakoff · 2014-03-04T00:28:36Z

@holtkamp
You're referencing a fork of a repo you made yourself instead of the official repo, by overriding the repository source. Also the branch/tag you're referencing does not exist. I removed the "repositories" section of your composer.json and changed the version constraint to "*" and it installs the official ZF1 package.

My "dummy" package is for the purposes listed in the 2nd paragraph here, https://getcomposer.org/doc/04-schema.md#replace If composer does the wrong thing, that doesn't make my package officious... it would mean there's a security flaw in composer.

In my opinion composer is working fine here, and my package is not maliciously exploiting anything, and is used in important projects, so it cannot be changed. The error is in you referencing a tag that does not exist.

joshribakoff · 2014-03-04T00:30:21Z

Also there's no tag in your fork matching the version contraint, only a branch. Composer's version contraints do not match against branches, they match tags.

Edit: looks like to match a branch you must prefix the branch name with "dev-". Just like how you have to put "dev-master" to specify the "master" branch.

Seldaek · 2014-03-04T07:34:20Z

@holtkamp so the issue is you require guilhermeblanco/zendframework1-doctrine2 which requires zendframework/zendframework1: ~1.9 but you also require your zf1 fork in version dev-patch-two-level-cache-updates. Since that dev version does not match ~1.19, it gives you an error and refuses to install (since the replace issue has been fixed). You can fix it by requiring this instead:

        "zendframework/zendframework1": "dev-patch-two-level-cache-updates as 1.12.0"

The as 1.12.0 at the end makes an alias to make your dev version pass the ~1.9 requirement and make it look more like an official version. Read more about this at https://getcomposer.org/doc/articles/aliases.md#require-inline-alias

holtkamp · 2014-03-04T08:44:42Z

@Seldaek, ok, thanks, I guess I missed that section about inline aliasing.
@joshribakoff, thanks for your input, not sure I understand it though. I WANT to override the official package with my own changes (this introduced the 'error'...). Composer DOES consider branches, and I DID prefix them with 'dev-', right? With the suggestion of @Seldaek it now works.

The whole approach is also described here I found out: http://mnapoli.fr/overriding-dependencies-with-composer/
Thanks guys!

stof · 2014-03-04T08:46:32Z

@holtkamp sure composer recognize branch. But dev-my_branch does not match the ~1.9 constraint. This is why you would need to alias it to something else

C-Duv · 2014-04-03T17:47:29Z

Can we also limit replace/provide on Satis side? In my case (composer/satis#129) requiring a package fetches every package that can replace it. Taking time and using too much disk space.

rk3rn3r · 2014-07-09T12:05:21Z

@naderman @Seldaek I already tweeted about an issue related to this change.

we have a package .../amqp that has

            "require": {
                "ext-amqp": "*",
                "ext-json": "*",
                "php": ">=5.4.24",
                ...
            },
            "provide": {
                "ext-amqp": "*"
            }

because we wanted to show php extension amqp is necessary, but you don't need it until you make use of one application part, where it is used and so it is not necessary when you run composer install/update.

recently it worked fine, package amqp got always installed, no matter if ext-amqp is available or not.

now on composer install it is not installed (no message, fails silently) if ext-amqp is not availale, on composer update you get an exception:

[Composer\DependencyResolver\SolverProblemsException]                        

    Problem 1                                                                  
      - The requested package ../amqp could not be found in any version, there may be a typo in the package name.

I know why you made these changes, but I am confused why it fails silently on "composer install".

do you have a solution for this, I suggest using "suggest"?

remove ext-amqp from require, change "provide" to "suggest" the ext-amqp extension?

            "require": {
                "ext-json": "*",
                "php": ">=5.4.24",
                ...
            },
            "suggest": {
                "ext-amqp": "*"
            }

stof · 2014-07-09T12:14:43Z

well, if it is not a strict requirement, you should put it in suggest, not in require with a provide hack (whcih would produce weird thing for packages which really require the extension)

rk3rn3r · 2014-07-09T13:01:42Z

@stof yeah, this was my idea. but the recent idea was, that ext-amqp is required, but the .../amqp package is used in a big application and only 1-2 webpages make use of amqp, but the AmqpBundle is needed, because it's loaded in Smyfony Kernel. Ok, makes sense to put in registerBundles that AmqpBundle and bundles that references AmqpBundle in a extension_loaded('amqp') if clause...
but this is...... ugly.... a bit to me.

we will go the way now with suggest.

the idea of this change was:

you have a dependeny to an interface (for example FirstLevelCacheInterface my-app/my-cache)
and some packages that provide the my-app/my-cache
one requires ext-apc
another ext-xcache and so on
composer would pick the one that matches

I think the issue we have is:
when NO package could be installed for .../amqp (in our case because ext-amqp is not available and there is no other implementation for my-app/amqp on a dev server of a dev that is not working on the part of the application that uses amqp) NO package is installed, but there should be the exception that NO package for requirement ../amqp could be installed because ext-amqp dependency do not met, like we had before this change.
furthermore provide is ignored, what also is an issue, but I understand why it is needed to make the dependency resolving a bit easier

To use this package, either composer.json or any dependency needs to require "ext-mongo", see composer/composer#2690 and alcaeus#67 and heroku/heroku-buildpack-php#146

naderman added this to the Urgent milestone Feb 11, 2014

naderman added Solver labels Feb 11, 2014

naderman self-assigned this Feb 11, 2014

jkavalik mentioned this issue Feb 16, 2014

Installing zf2 on travis via composer got output backplane/zendframework ? zendframework/zendframework#5832

Closed

This was referenced Feb 18, 2014

getting a fork of symfony installed composer/packagist#390

Closed

Composer installed a fork instead if this bundle schmittjoh/JMSTranslationBundle#177

Closed

peterjmit mentioned this issue Feb 20, 2014

Packages using replace are in need of moderating composer/packagist#362

Closed

naderman mentioned this issue Feb 21, 2014

Detect and warn on "replace" at submission time composer/packagist#329

Closed

naderman added a commit to naderman/composer that referenced this issue Feb 21, 2014

Whitelist packages with names matching those specified before generat…

3148ffd

…ing rules Addresses composer#2690 doesn't do any performance optimisations yet which we could do now

naderman mentioned this issue Feb 21, 2014

Whitelist packages with names matching those specified before generating rules #2733

Merged

naderman added a commit to naderman/composer that referenced this issue Feb 21, 2014

Add tests for the changes in composer#2690

bdd7937

naderman added a commit to naderman/composer that referenced this issue Feb 21, 2014

Add tests for the changes in composer#2690

ec12b8a

naderman closed this as completed Feb 21, 2014

claudiumstoica mentioned this issue Mar 4, 2014

Your requirements could not be resolved to an installable set of packages. sendgrid/sendgrid-php#75

Closed

kayue mentioned this issue May 23, 2014

Composer downloading random package #3009

Closed

naderman mentioned this issue Sep 2, 2014

Depend on "PHP and Xdebug" or "HHVM" #2940

Closed

dzuelke mentioned this issue Feb 23, 2016

Back to "replace" for ext-mongo alcaeus/mongo-php-adapter#70

Merged

mbaynton mentioned this issue Nov 24, 2017

Allow replace to work as originally intended when replace is from a trusted repo #6843

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit Replace / Provides to packages required by name in root package or any dep #2690

Limit Replace / Provides to packages required by name in root package or any dep #2690

Limit Replace / Provides to packages required by name in root package or any dep #2690

Limit Replace / Provides to packages required by name in root package or any dep #2690

Comments

Problem

Replace

Original Goal

Issue

Provides

Original Goal

Issue

Proposed Solution

Consequences

Positive

Negative