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

Is it possible to include a package based upon the version of PHP running? #6041

Closed
rquadling opened this issue Jan 4, 2017 · 24 comments

Comments

@rquadling
Copy link

@rquadling rquadling commented Jan 4, 2017

Hi.

I want to use PHPStan. This package is for PHP7 only.

But the library I am using it in is tested in Travis for many different versions of PHP (5.3, 5.4, 5.5, 5.6, 7.0, 7.1, nightly and HHVM).

As such, when Travis runs composer install, it fails due to a mismatch for PHP5.x/HHVM as PHPStan wants PHP7.

I've added --ignore-platform-reqs to the composer install which works fine for PHPStan, but then causes problems with PHPUnit as the latest version gets installed for PHP5.6 and lower.

My next option was to install PHPStan when running Travis for PHP7 only as part of the .travis.yml setup, but that means not having PHPStan configured locally in my composer.json.

Conditional dependencies would be very useful.

@Jean85

This comment has been minimized.

Copy link
Contributor

@Jean85 Jean85 commented Jan 4, 2017

You just need to issue a composer require vendor/phpstan in your Travis build, but only on PHP >= 7.0.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 4, 2017

Yes, that's what I did when integrating PHPStan into CakePHP. But a conditional require in composer.json would be nice!

@alcohol

This comment has been minimized.

Copy link
Member

@alcohol alcohol commented Jan 5, 2017

But a conditional require in composer.json would be nice!

Never.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

@alcohol That's constructive.

@stof

This comment has been minimized.

Copy link
Contributor

@stof stof commented Jan 5, 2017

@ondrejmirtes they make it impossible to write a lock file meant to be shared elsewhere when requirements are conditional.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

@stof Yep, that's a possible hurdle, but manageable.

@stof

This comment has been minimized.

Copy link
Contributor

@stof stof commented Jan 5, 2017

Dependency resolution is already a hugely complex system, that most Composer contributors are unable to work on (I think there are at most 3 guys knowing really how it works, and it may be less than that if we move the level to "understanding it well enough to change it"). So addign more complexity in the solver for a "nice" case is not something I would call manageable today.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

@stof Well, you can make it more manageable and tested in the codebase today to make future changes easier. Technical debt likes this that causes 'do not touch this' situation should be eliminated.

@alcohol

This comment has been minimized.

Copy link
Member

@alcohol alcohol commented Jan 5, 2017

Implementing features just because some random person thinks they would be great to have for their specific use-case is exactly how you wind up with more technical debt.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

The discussion should be about "is this a good idea?" from usability, developer experience and convenience standpoints, not about "we can't do this because the codebase is a mess and no one understands that". If we can talk whether this is a good idea or not, that would be great.

@alcohol

This comment has been minimized.

Copy link
Member

@alcohol alcohol commented Jan 5, 2017

It is not a good idea.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

Why? The main problem is that PHP-7 only packages cannot be put in composer.json because the project then cannot be installed on PHP 5.x. Because the maintainer has to find another way how to make it work (putting composer require) in the build script, which is an inconveniece, the package numbers are skewed and lower than the reality (like the depenents list on packagist.org) because crawlers do not see its as a dependency.

if we found a DSL how to add this condition to the version string, I think this would make it a smaller problem.

@alcohol

This comment has been minimized.

Copy link
Member

@alcohol alcohol commented Jan 5, 2017

Your scenario makes no sense. A PHP package that requires PHP 7 should have this as a requirement and should be installed on PHP 7 systems only. I don't see what a conditional require has to do with this.

@alcohol

This comment has been minimized.

Copy link
Member

@alcohol alcohol commented Jan 5, 2017

The original post mentions the use of a package that is not a dependency, but merely a tool, similar in how phpunit and behat and such are tools. They are not dependencies.

@stof

This comment has been minimized.

Copy link
Contributor

@stof stof commented Jan 5, 2017

@ondrejmirtes the issue is not that the solver is a mess. The issue is that solving dependencies is already a really hard problem (try to reimplement Composer from scratch if you don't believe me, and then come back discussing it in a few years when you're done...)

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

I'm not discounting the fact that dependency resolving is really complex (and that Composer is awesome).

@Jean85

This comment has been minimized.

Copy link
Contributor

@Jean85 Jean85 commented Jan 5, 2017

@ondrejmirtes if your problem is the skewed statistic of PHP 7 only package, maybe you should use the "suggest" section, that is well reported in Packagist.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

@Jean85 That's actually a good idea.

@ondrejmirtes

This comment has been minimized.

Copy link

@ondrejmirtes ondrejmirtes commented Jan 5, 2017

@Jean85 But there's no suggest-dev section, is there?

@Jean85

This comment has been minimized.

Copy link
Contributor

@Jean85 Jean85 commented Jan 5, 2017

Nope, but you can add a description, so you can explain that it's only for dev.

@rquadling

This comment has been minimized.

Copy link
Author

@rquadling rquadling commented Jan 8, 2017

Another use case for conditional requirements is where package x is for PHP < 7 and package y is for PHP >= 7. With tools like PHPUnit, we cannot realistically supply a lock file that includes the correct version of PHPUnit as different versions are included depending upon the version of PHP that is being used.

For packages like the one I working with, we don't supply the lock file as it would be different for each version of PHP.

It is just that in PHP7+, we cannot exclude an incompatible package.

The other option is to include the package in composer.json, but then uninstall it for Travis builds less than PHP 7 (and HHVM).

And again, a lock file doesn't help us. The lock file is only useful if the packages don't care about the version of PHP. And more and more packages are saying so, and so when we develop on PHP7+ and still want to support older versions ... we are sort of stuck.

I think the use case is perfectly valid. As long as the assumption is no lock file. Which seems to be the case nowadays for when the package supports multiple versions of PHP.

@xabbuh

This comment has been minimized.

Copy link
Contributor

@xabbuh xabbuh commented Jan 8, 2017

@rquadling What is your use case for having lock files in a library (or any other package being reused)? Everyone depending on your package will do not see any difference whether you supply one or not. The composer.lock file is mainly useful for applications, but there you have control over the environment where it is actually used.

@rquadling

This comment has been minimized.

Copy link
Author

@rquadling rquadling commented Jan 8, 2017

That's what I'm saying. No lock files mean that the versions that get installed are already environment dependent - composer does its job very well.

What I'm asking is to include additional/alternative packages/extensions based upon environment, so I can declare the tools/packages needed for PHP7+ and expect them to be installed if the environment is appropriate.

@Seldaek

This comment has been minimized.

Copy link
Member

@Seldaek Seldaek commented Jan 22, 2017

This has been considered, discussed at length and ultimately rejected in #751 - TL;DR: I understand the use case would be nice in some cases but it's just not worth the complexity IMO.

@Seldaek Seldaek closed this Jan 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.