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 Request] prefer lowest mode for non-dev deps only #6856

Open
frankdejonge opened this Issue Nov 30, 2017 · 15 comments

Comments

Projects
None yet
8 participants
@frankdejonge
Copy link

frankdejonge commented Nov 30, 2017

For some of my projects I've got quite a wide range of supported PHP versions. Due to PHP version upgrades it's hard (and sometimes even simply impossible) to use phpunit. For example, I'm supporting 5.6 to 7.2 for a library. In order to support 5.6 I need my lower bound to be low enough to find a phpunit version (or dep of) that supports 5.6, but it also needs to be high enough for any breaking changes in 7.2 to be patched. This turned out not to be possible unfortunately.

BUT! If composer would allow for the lowest version only to be preferred for the non-dev dependencies this would work out way easier and makes it easier to support older version of packages. The lowest version supported is great for dependencies you need at runtime but actually provide little value during dev time. Package maintainers want to support and test things to ensure production usage, not development usage most of the time.

What do you think about this? If accepted I'm perfectly willing to put in the time to figure out how to implement this and create a PR for this, but I wanted to get all of your opinions in first so no energy is wasted on this for no reason.

@Seldaek Seldaek added the Feature label Nov 30, 2017

@Seldaek Seldaek added this to the Nice To Have milestone Nov 30, 2017

@Seldaek

This comment has been minimized.

Copy link
Member

Seldaek commented Nov 30, 2017

So adding a --prefer-non-dev-lowest flag? I am not sure how feasible it is tbh as you'd need to track down the whole dependency trees of the require'd packages and whitelist those as having prefer-lowest but nothing else. It's probably doable but might be a nasty patch.

I kinda see the value though, so if the patch doesn't introduce too much mess I think it'd be ok. The DefaultPolicy class is what you want to look at.

@nicolas-grekas

This comment has been minimized.

Copy link
Contributor

nicolas-grekas commented Dec 4, 2017

The solution is just to not put phpunit/phpunit as a composer dependency.
It's just wrong to put it there (yes I know, so many ppl are doing...)
Instead, use Symfony's phpunit bridge: it fixes the issue by building phpunit in a dedicated folder, which is the way to not have phpunit alter you own dependency tree (which it must not do, but will do if you add it to your tree!).

@frankdejonge

This comment has been minimized.

Copy link

frankdejonge commented Dec 4, 2017

@nicolas-grekas while that's certainly an option, if this problem can be solved from within composer itself it would be better I think. Rather have the dependency tool solve a dependency issue than have a workaround in place.

@nicolas-grekas

This comment has been minimized.

Copy link
Contributor

nicolas-grekas commented Dec 4, 2017

A composer-side solution could be to have a third "require-tooling" section, where tools are listed.
The way composer would resolve them would be:

  • resolve everything using only require+require-dev
  • then install the tools with the highest versions that fit the constraints set by the previous run of the solver (ignoring prefer-lowest of course)

That'd be awesome IMHO :)

@nesl247

This comment has been minimized.

Copy link

nesl247 commented Dec 4, 2017

A tooling section makes the most sense to me. Using Symfony's phpunit-bridge is not a solution as the same issue can occur in other tools. Also. adding phpunit to require-dev makes perfect sense as it is a development dependency. In order to develop, you write tests, and to run the tests, you need your test runner.

Having a separate require-tooling section would also remove tool's dependency tree from affecting the dependency tree of the application. With this in mind, I would actually think anything in the require-tooling section would have it's own installation of dependencies. Yes it would duplicate dependencies potentially, but it would also alleviate the issue of everything requiring symfony/console and in order to upgrade to v4 for example, you have to wait until those tools support it.

CLI tools that are necessary for the project to run should be included in the project, but they also should alter the ability for a project to be updated. Basically, tools should be able to be required, but be self-contained.

@nicolas-grekas

This comment has been minimized.

Copy link
Contributor

nicolas-grekas commented Dec 4, 2017

anything in the require-tooling section would have it's own installation of dependencies

unfortunately that's not completly possible: the autoloader of the tools and of the app must be shared (eg phpunit, but any other really needs access to your classes), so that having several versions of the same lib is calling for bad conflicts (like when phpunit used to embed symfony/yaml).
As such, deps must be shared with the app.

But each tool could/should have its own set of dedicated additional-deps for sure!

@nesl247

This comment has been minimized.

Copy link

nesl247 commented Dec 4, 2017

True, I forgot about that.

@Jean85

This comment has been minimized.

Copy link
Contributor

Jean85 commented Dec 5, 2017

Hello! I was basically proposing the same stuff in #6868, I missed this issue...

I agree with the proposal, but I'm missing the reason behind a third require-tools section: what would be the difference between that and require-dev?

@curry684

This comment has been minimized.

Copy link
Contributor

curry684 commented Dec 5, 2017

For a prime example of the potential problems arising from this see my comment in the other issue.

I see some merits in a separate tooling section, as I've run into issues before with php-cs-fixer specifically being in my dev requirements, but that package always declares its incompatibility with upcoming PHP versions, therefore requiring --ignore-platform-reqs to run automated tests on nightlies and RCs. Having a solution for that is worth exploring.

@stof

This comment has been minimized.

Copy link
Contributor

stof commented Dec 5, 2017

@Jean85 the proposal in this discussion is that tooling requirements would always install the latest version, even when using --prefer-lowest

@nicolas-grekas hard thing: what would happen for deps shared by normal requirements and tooling ones ? Using higher or lower version ?

@curry684

This comment has been minimized.

Copy link
Contributor

curry684 commented Dec 5, 2017

I think the only "solution" for that would be a completely separated vendor tree.

@nicolas-grekas

This comment has been minimized.

Copy link
Contributor

nicolas-grekas commented Dec 5, 2017

what would happen for deps shared by normal requirements and tooling ones

What I tried to describe in #6856 (comment)
Would just be whatever the current ones are, no impact.

I think the only "solution" for that would be a completely separated vendor tree.

See #6856 (comment)

@curry684

This comment has been minimized.

Copy link
Contributor

curry684 commented Dec 5, 2017

Correct, PHPunit would inherently always remain in the dev tree for that reason. Tooling includes, such as PHP-CS-Fixer, code generators and other utilities would have to be inherently separated from the rest of the code. If something integrates with the project code it has to share its dependencies, no way around that.

@nicolas-grekas

This comment has been minimized.

Copy link
Contributor

nicolas-grekas commented Dec 6, 2017

Nice article from @theofidry with overlap on the topic:
https://twitter.com/tfidry/status/936949656021602304

UrGuardian4ngel added a commit to UrGuardian4ngel/grumphp that referenced this issue Dec 26, 2017

@UrGuardian4ngel UrGuardian4ngel referenced this issue Dec 26, 2017

Merged

Add Symfony 4 support #428

3 of 3 tasks complete
@dereuromark

This comment has been minimized.

Copy link

dereuromark commented Jan 3, 2019

Sounds also related to #7849 IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment