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

Re-add support for php 5.5.x #1237

Closed
ebernhardson opened this issue Jan 23, 2017 · 25 comments
Closed

Re-add support for php 5.5.x #1237

ebernhardson opened this issue Jan 23, 2017 · 25 comments

Comments

@ebernhardson
Copy link
Contributor

I would like to request you re-consider supporting the EOL'd 5.5.x series, and possibly 5.4.x, of PHP. This is important for packages using elasticsearch that want to target LTS releases of popular distributions. Certainly everyone has to make a cutoff somewhere, but i would argue that a large enough set of currently supported linux distribution versions use 5.5 and 5.4 that it's worth supporting as long as the maintenance burden is not too heavy.

  • Ubuntu 14.04 will be supported until mid 2019. This ships with PHP 5.5.9
  • Debian 7 "Wheezy" will be supported until mid 2018. This ships with PHP 5.4.45
  • Debian 8 "Jessie" will be supported until mid 2020. This ships with PHP 5.6.29
  • CentOS 6 will be supported until late 2019. This ships with PHP 5.3.3
  • CentOS 7 will be supported until mid 2024. This ships with PHP 5.4.16

In the commit message removing support you mention the ability to use new features (imo the potentially relevant ones are constant scalar expressions and variadics), and the ability to depend on libraries that require 5.6. I would like to argue that after a quarter of a year no features of 5.6 have made their way into the code base. They are useful in certain circumstances, but don't seem in any way necessary for Elastica to function. The other argument about library dependencies seems fairly minor, as elastica only has a single non-dev dependency, aws-sdk-php. The latest version of aws-sdk-php still supports php 5.5.

@ruflin
Copy link
Owner

ruflin commented Jan 25, 2017

@ebernhardson I was kind of waiting until this pops up :-) I think we should make a difference between what is tested and what works. I'm quite confident that Elastica works with PHP 5.4 and 5.5 as you also mentioned above. The only dependencies that require PHP 5.6 are in the dev (like phpunit).

If you want to install Elastica with PHP 5.4 or 5.5 you can do that quite easily with --ignore-platform-reqs. I think this should work in your case.

Requiring 5.6 is not for the code that is already in, but the code and changes that could come. Assuming we can heavily simplify the code because of a library requiring 5.6 we can introduce it without having to wait for a major release. I agree it is unlikely that this happens, but I don't want to exclude it. Also supporting 5.5 prevents #944 from happening.

Happy to discuss further, definitively worth a discussion.

@p365labs
Copy link
Collaborator

As @ruflin stated If you want to install Elastica with PHP 5.4 or 5.5 you can do that quite easily with --ignore-platform-reqs. I think this should work in your case. .

@ebernhardson you're right LTS of many Linux distributions still ships with older versions of PHP, but this won't mean you cannot update PHP to at least the 5.6 version. I know that this should sound a bit rude, but I think (it is just my opinion) that you have to keep server distribution up to date for security matters. And also we should consider that PHP 5.6 Active Support has been dropped 8 days ago ;) .

my 2 cents :)

@ebernhardson
Copy link
Contributor Author

I'm not as worried about my case of personally installing Elastica, but about the difficulty of the wide variety of users, from users on shared hosting up to users on their own hardware and everything in between, that run the software i work on. --ignore-platform-reqs ends up being a bigger problem in this case, because then other packages of the wider software platform which should rightfully pull in older versions, or non-php7 versions, will grab the wrong code. This is perhaps a shortcoming of composer that platform requirements cannot be ignored on a per-package basis but only globally. Using older versions of other packages is often less of a problem because they are typically more self contained as opposed to Elastica where an upgrade is needed to talk to the upgraded external service. How some sites can manage to install new versions of elasticsearch but not php I can't fully explain, but it is certainly a regular occurance.

@p365labs You are certainly welcome to an opinion about what users should run, but in terms of distributing open source software I have much less control of what the users actually do use. Debian is actually quite good about backporting security fixes even after php itself stops, I'm not as familiar with the others but I wouldn't expect much less.The php minimum requirements are set after a large amount of hand wringing between dozens of developers debating the tradeoffs made, and are then generally set until the next LTS version of our software is released (to keep backports, including security backports, sane among other reasons).There are a lot of conflicting needs that are taken into account when choosing minimum requirements, feel free to peruse https://phabricator.wikimedia.org/T118932 if you are curious how our particular organization does it.

For my use case, the best option currently seems to be overriding the ruflin/elatica package in the "repositories" section of my composer.json. This essentially means copying the elatica composer.json into my own, removing the php requirement, and adding a dist section that describes where to get the code. This certainly works although is a bit verbose. I can't say I agree with adding these roadblocks in for releasing software, but i can certainly see where releasing a single library has much different requirements than releasing a complete platform.

@merk
Copy link
Collaborator

merk commented Jan 28, 2017

You're going to be fighting a losing battle trying to get libraries and frameworks to continue to support really-old php versions for a long time: volunteers arent going to want to bother with such compatibility.

I think the only viable solution is to keep the last supported php 5.5 branch open for bug fixes, and if you have users who are stuck on old php versions, they're quite welcome to contribute back bug fixes until they're in a position to upgrade to a new distribution. The burden of LTS releases does not fall to individual packages, especially when PHP itself has dropped support for these versions.

If the PHP version is frozen in your environment, elasticsearch should be frozen as well - and given this package tracks ES versions, you should be freezing the major versions as well. If the environment is using Elastic.co's apt respositories, why cant that same user adopt the ondrej PHP ppa?

@ruflin --ignore-platform-reqs isnt viable, unfortunately - because you'll get packages that are bumped to 5.6 and only support 5.6+, and composer wont warn you about missing pecl modules.

@p365labs
Copy link
Collaborator

@ebernhardson I understand ur point of view. As the developer|maintainer of a software is your responsibility giving the "user base" a path they should follow; you can help them in some way but before and after they should update their OS. I really don't understand why they shouldn't.

PHP Versions May 2016 November 2016
PHP 5.6 39.67% 37.46%
PHP 7.0 20.24% 35.01%
PHP 5.5 29.56% 18.93%
PHP 5.4 7.64% 5.40%
PHP 5.3 2.43% 1.60%
PHP 7.1 / 1.36%
  • We should also not forget that PHPUnit 5.x has dropped support to php 5.5
  • there are many frameworks that this year will drop support to php 5.5.x : NEOS 3.0, PHPUnit6, TYPO3 8 LTS, PhpSpec, Laravel 5.5, Xdebug 2.6, Symfony 4.0

@ruflin
Copy link
Owner

ruflin commented Jan 30, 2017

Thanks everyone for chiming in here. I really appreciate all the comments and share thoughts above.

Instead of having a very generic discussion where I'm quite confident we don't find a common ground lets focus on the specific project(s) that @ebernhardson faces the issue and lets see if there are solutions for this problem. Perhaps then we can find a more generic solution.

@ebernhardson Are we talking about wikimedia installations here (just to be concrete about the project)? If yes, didn't know it is already at ES 5.x 👍

@ebernhardson
Copy link
Contributor Author

I'm working on CirrusSearch, which is the extension to mediawiki that integrates with elasticsearch and powers search on wikimedia sites (wikipedia, etc). We are currently in the process of upgrading the production cluster to 5.x, hence how I've come across this. One of the reasons for upgrading to 5.x is to have access to the updated completion suggester API, but i digress. Production in our case runs hhvm so the 5.6 minimum requirement isn't a problem there. It's a problem for our external users, as we have the minimum php versions set to 5.5 until the next LTS release (mid 2018, we do LTS every 2 years)

As mentioned above the most direct solution is re-creating the elastica composer.json within my own package. Composer allows adding a repositories section which can override package definitions coming from packagist. This works for my use case, so i'm not blocked on this but I thought it worth bringing up as we had some internal debate about if it was better to fork and re-publish elastica with the lower version, or override the package definition.

@ruflin
Copy link
Owner

ruflin commented Jan 31, 2017

Some ideas here:

  • It would be nice if we could define "officially" supported PHP versions and version which we know that work but have a * that says not tested. As far as I know, something like this is not supported at the moment in composer.
  • Interestingly this seems all about composer, so potentially something that could be brought up with the composer guys as an enhancement. Because this problem is not going away (ever ...).
  • Is it a good idea to switch on 5.5 again without testing it (phpunit require 5.6)?
  • Could we have a special "release branch" that has the modified composer file inside? Something like 5.0.0-php55-use-at-your-own-risk or just 5.0.0-php55? If possible I kind of would like to prevent forks if not really needed.

@merk
Copy link
Collaborator

merk commented Jan 31, 2017

Additionally:

What level of support are you actually going support "unofficial" php versions? It is reasonable to do a feature freeze and only apply bug fixes to a legacy branch.

IE: Return PHP 5.5 support to 5.0.x, then release 6.0.x without PHP 5.5 - and all you'd need to do is have a "5.0 release manager" backport/cherry pick any bug fixes or security issues that may apply.

Keeping LTS releases feature complete is not necessary.

@ruflin
Copy link
Owner

ruflin commented Feb 1, 2017

My personal experience on "release manager". So far in the history of Elastica I think I created 2-3 times a branch for people to backport bug fixes to an older release. The only request I had is that they open PR's themself. The end result: Not a single PR was ever opened against one of these branches. So if someone is out there that will really take on responsibility of "backporting/maintaining", happy to provide such a branch.

@merk
Copy link
Collaborator

merk commented Feb 1, 2017

Which is my main point: those who are stuck with PHP 5.5 due to politics are the ones that should have the burden of maintenance of an LTS branch of Elastica.

These issues increase the maintenance burden for volunteer contributors for a segment of the population that because of politics, they're stuck on unsupported PHP versions for years after those versions became unsupported.

If you take Centos 6 as an example: there is no Elasticsearch package in its repositories, you're required to download versions from elastic.co - which breaks the reason for bothering with LTS releases: https://www.saotn.org/install-elasticsearch-on-centos-6-7/

@ebernhardson
Copy link
Contributor Author

I'd be happy to take on the requirements of backporting / maintaining a 5.5 branch. With that published to packagist seems like a reasonable way forward to keep things simple on both ends.

@merk Having or not having specific packages for LTS is neither here nor there. While our (mediawiki) support of 5.5 is specifically for the thousands of installations outside wikimedia's production usage, our production servers all run LTS releases of their distributions. We (and I imagine most organizations with hundreds to thousands of servers) maintain an apt repository with backports, such as elasticsearch, where necessary. When running low thousands of servers with a wide variety of services it's quite nice to leave things in a 'just works' situation. Some services such as elasticssearch may have teams dedicated to the single service+integration and able to keep up to date to continually improve results for thousands of search requests per second, while other services that are still important but not under active development may be simply delegated to an ops team that can't go around upgrading major software versions every year. In our server farm even upgrades from one LTS to the next are often done on a service by service basis over the course of a year or two due to the amount of work involved making sure everything continues smoothly with appropriate logging, monitoring, expected failure cases, etc. etc.

@merk
Copy link
Collaborator

merk commented Feb 2, 2017

Oh, totally agree with you - we're in the same circumstance with LTS, but with regards to PHP our organisation made the choice to track newer PHP. It took quite a bit of work to make sure everything would be okay, as you say.

I guess the main questions is now, does Elastica 5.0 get PHP 5.5 support back (since it doesnt really need to drop that support), and we drop with another major bump now or later?

@ruflin
Copy link
Owner

ruflin commented Feb 6, 2017

Based on the discussions above I suggest to move forward as following:

  • Create a branch from 5.0.0 with the name 5.x-php55
  • @ebernhardson will open the necessary PR's to this branch.

This has the following advantages:

  • This will move the burden of maintenance to @ebernhardson and hopefully other collaborators
  • It will not limit any development in master
  • No need to change the testing in master because the phpunit version used only supports 5.6

Questions

  • Do we need to tag releases or this the branch enough?

I would like to hear some 👍 or 👎 to this.

@merk
Copy link
Collaborator

merk commented Feb 6, 2017

I dont agree with the approach. Semver exists for a reason to make libraries easy to use.

bump 5.0 to 6.0, release 5.0.x to revert php 5.5 support, maintain 5.x branch alongside master/6.x

@ebernhardson
Copy link
Contributor Author

ebernhardson commented Feb 6, 2017

I think the approach sounds reasonable. While semver is nice, i don't think it applies in this use case. Since the elasticsearch API's change in incompatible ways between major releases I think it's worthwhile for elastica major versions to map to elasticsearch major versions.

@ruflin
Copy link
Owner

ruflin commented Feb 7, 2017

@merk The reason I propose the above is that so I can use PHP 5.6 features also already in 5.1 or 5.2. This would not be the case if we keep a common 5.x branch. Also I want to make it "obvious" that it is a special version people are using based on the naming.

Alternative suggestion could be to create a 5.0 branch, release 5.0.1 with 5.5 support but not have it in 5.1. Problem with this is, people will be confused when they try to upgrade form 5.0.1 to 5.1.

If we have a 5.x-php55 branch I'm also ok to do some release tagging on it in case someone would need it. But I think composer also allows to use branches?

@merk
Copy link
Collaborator

merk commented Feb 7, 2017

Elastica is in this middle-ground problem area where you're trying to do to incompatible things at once:

  1. Follow semver
  2. Lock your major versions to match ES releases

If that wasnt the case, you could just bump the current release to 6.0 and continue working, and it would just work for users who are installing Elastica in PHP 5.5, because they'll get ~5.0. Instead of a 5.1, you just release a 6.1. For updates to 5.x, you'd just continue to tag 5.x versions, which would be handled automatically by composer.

You can definitely just use branches - and release tags in that instance are a good idea too, but I dont think they'd work for the semver composer calculations (you wouldnt be able to do ~5.0-php55 or something to just get the latest supported version for PHP 5.5).

@p365labs
Copy link
Collaborator

p365labs commented Feb 9, 2017

Even if I'm still not sure this is the right solution, 5.5 it's too old and there are evidence that people are moving to new versions (see composer stats on the website). I support the @ruflin solution, the maintenance of that branch is on @ebernhardson and others that want to support. This branch should not limit the development on master.... so for me 👍

@ruflin
Copy link
Owner

ruflin commented Feb 10, 2017

@merk Yeah, it's fun versioning problem :-)
@ebernhardson Would you need release tags or is a branch enough?

@ebernhardson
Copy link
Contributor Author

Hmm, as far as I know composer needs a release tag to be able to point to it as a specific version, so that would probably be best.

@ruflin
Copy link
Owner

ruflin commented Feb 14, 2017

Lets continue in the following way:

  • I will release 5.1.0 because it contains lots of cleanup which I think would be also nice to have for @ebernhardson
  • I will create a branch 5.1-php55 from 5.1
  • @ebernhardson Will open the needed PR's to make it compatible against this branch.
  • If needed, I will tag 5.1.0-php55

This will break semversioning, but I hope people using 5.5 will anyways tag a specific release. I hope it does not break composer ...

@ruflin
Copy link
Owner

ruflin commented Feb 16, 2017

@ebernhardson The 5.1-php55 branch is available here: https://github.com/ruflin/elastica/tree/5.1-php55 Please open PR's to bring back PHP 5.5 against this one.

@ebernhardson
Copy link
Contributor Author

Thanks for setting it up. I've put up a PR that alows the branch to operate against (only) php 5.5.

@ruflin
Copy link
Owner

ruflin commented Feb 22, 2017

@ebernhardson Thanks. I left some comments. I'm closing this thread as I think we managed to "fix" it.

@ruflin ruflin closed this as completed Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants