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

Feature: require one package OR another #6525

Closed
garrettw opened this issue Jul 3, 2017 · 8 comments
Labels

Comments

@garrettw
Copy link

@garrettw garrettw commented Jul 3, 2017

Just had an idea related to thephpleague/climate#110 - what if composer allowed them to say that a user would need EITHER ext-mbstring or symfony/polyfill-mbstring?

I don't know if this functionality would be useful outside of extensions vs polyfills, but at least in this case, I'm thinking composer could look for the extension and if it's not found, install the polyfill.

If this were written in such a way as to be applicable to any pair of packages, then it would need to have a way to mark one or the other as preferred for the case where the user has neither installed.

@curry684

This comment has been minimized.

Copy link
Contributor

@curry684 curry684 commented Jul 3, 2017

This behavior is implemented using virtual packages, see for example symfony/cache providing psr/cache-implementation. Other packages can now depend on the virtual package, and Composer will happily break during resolution stating that an implementation is required but not included.

This obviously requires the active participation of the providing package's authors, which won't work in your specific example. However that won't work anyway as the whole point of a polyfill is to achieve platform independence at runtime. The mbstring polyfill ensures mb_ functions always work, even if you uninstalll the mbstring extension after installing your project. And Composer, as an infrastructural tool, has no beef with runtime considerations like that.

@garrettw

This comment has been minimized.

Copy link
Author

@garrettw garrettw commented Jul 3, 2017

The whole "provides"/virtual package thing is a bit opaque to me. I understand, for example, the difference between classes and interfaces, so I can understand how a virtual package is similar to an interface that can be implemented by other packages – but if some package has a dependency on a virtual package, then it sounds to me like it will always break Composer unless (1) you've already downloaded an implementation, maybe because another library has brought one in as a concrete dependency, or (2) you go into that composer.json and manually change the reference from the virtual package to a concrete one.

Is that how it works? If so, I guess that's a key difference with my idea – I would find it useful if a concrete package could be installed if no other installed package "provides" the virtual one.

@curry684

This comment has been minimized.

Copy link
Contributor

@curry684 curry684 commented Jul 4, 2017

Yes, it is intended to break Composer, and to have you fix that as you describe under 1 (obviously 2 is always an incredibly Bad Idea™️).

The whole point is that it's not your job as a library developer to decide how I run my server. In your specific example: who are you to decide I want to use the polyfills? Perhaps I just forgot to install the native mbstring extension. Now because I simply forgot that my application is suffering from reduced performance because some library I'm including decided for me I must be happy with the reduced polyfill performance. Erm no, I want it to break so I know what and how to fix.

While the mbstring case is a tricky one as platform packages do not expose virtual packages underneath the alternative is certainly no better.

@alcohol alcohol added the Support label Jul 4, 2017
@jasverix

This comment has been minimized.

Copy link

@jasverix jasverix commented Jul 4, 2017

Composer have a 'replace' statement. Is it possible to add extensions to that 'replace' statement?

{ "replace": { "ext-mbstring": "*" } }

If so, then we can open an issue on the polyfill to "replace" the extension, and the main package may require the extension. People who does not want the extension can then use the polyfill, and it will still work - but it will not work without any of them.

@jasverix

This comment has been minimized.

Copy link

@jasverix jasverix commented Jul 4, 2017

Look at these issues:

This will solve this problem, won't it @garrettw ?

@fpoirotte

This comment has been minimized.

Copy link

@fpoirotte fpoirotte commented Jul 17, 2017

While virtual packages are indeed intended to provide such a feature, they're hard to use in practice, mainly because of the poor feedback given ATM during any packag's install.

I think they would be pretty useful if instead of the following error message:

$ composer.phar require psr/log-implementation

                                                                                                                                                          
  [InvalidArgumentException]                                                                                                                              
  Could not find package psr/log-implementation at any version for your minimum-stability (stable). Check the package spelling or your minimum-stability  
                                                                                                                                                          

require [--dev] [--prefer-source] [--prefer-dist] [--no-progress] [--no-suggest] [--no-update] [--no-scripts] [--update-no-dev] [--update-with-dependencies] [--ignore-platform-reqs] [--prefer-stable] [--prefer-lowest] [--sort-packages] [-o|--optimize-autoloader] [-a|--classmap-authoritative] [--apcu-autoloader] [--] [<packages>]...

we could have something like this:

$ composer.phar require psr/log-implementation
Your requirements could not be resolved to an installable set of packages.
  Problem 1
    - The requested package psr/log-implementation refers to a virtual package. A concrete implementation must be installed instead. Possible candidates: monolog/monolog, zendframework/zend-log, etc.                                                                                                                                                          

Installation failed, reverting ./composer.json to its original content.

Maybe with possible candidates formatted as a vertical list for readability instead of the compact line of text shown above:

Possible candidates:
* monolog/monolog
* zendframework/zend-log
* ...

Not sure how the version requirements of the virtual package would get propagated to concrete implementations though.

Just my 2 cents,
François

@curry684

This comment has been minimized.

Copy link
Contributor

@curry684 curry684 commented Jul 17, 2017

That most certainly seems like a good idea. PR's welcomed ;)

@Seldaek

This comment has been minimized.

Copy link
Member

@Seldaek Seldaek commented Aug 7, 2017

Duplicate of #751 and a few others.. Unlikely to happen really. Improving feedback for providers is something else though and that might happen I reckon but please report a new issue for that if you're interested in working on it then we can discuss further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.