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

Use ">=" for the "php" requirement #36876

Merged
merged 1 commit into from
May 20, 2020
Merged

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 20, 2020

Q A
Branch? 4.4
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

As explained in https://twitter.com/nicolasgrekas/status/1263023258938548225:

Using "^7.x" in our composer.json has been a mistake. We should always use ">=7.x"! 3 reasons:

  1. it's either planned obsolescence xor a strong promise to maintain in the long run. None is sustainable.
  2. if you actually end up maintaining in the long run (not by promise but by fact), your latest versions will work with PHP 8 by definition.
  3. meanwhile, "^7.x" prevented all your ecosystem from experimenting with PHP 8, which means they increased the workload on you the core maintainer.

Conclusion: always use ">=" for the "php" requirement. Hope for the best (it mostly happens) and enable your community to experiment with the next major asap without adding useless impediments.

@fabpot
Copy link
Member

fabpot commented May 20, 2020

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 8f6afdd into symfony:4.4 May 20, 2020
@localheinz
Copy link
Contributor

We don't know yet whether any component will be compatible with PHP 9000.

Running

$ composer require php:"^7.1.3 || ^8.0.0"

would have been sufficient.

@driesvints
Copy link
Contributor

Just my 2 cents but I think this is a bad idea (see reasons stated here and in the replies). But hoping that it goes well for you 👍

@Slamdunk
Copy link
Contributor

I respect the right of Symfony maintainers to merge a PR only after half an hour it's opened, but I think you're shooting your feet if you don't wait enough time for a constructive discussion to emerge from the community.

@drupol
Copy link
Contributor

drupol commented May 20, 2020

Doing this with my package since the beginning, I couldn't agree more with this change.
Thanks!

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented May 20, 2020

For historical reference, all versions before and including 3.4 already use >= with absolutely zero problem.

All prophecies you're scared of would have happened by the time. These are old releases that gathered a lot of experience - much more than recent tags, by definition.

@nicolas-grekas nicolas-grekas deleted the php-req branch May 20, 2020 10:26
@localheinz
Copy link
Contributor

localheinz commented May 20, 2020

@nicolas-grekas

For historical reference, all versions before and including 3.4 already use >= with absolutely zero problem.

All prophecies you're scared of would have happened by the time. These are old releases that gathered a lot of experience - much more than recent tags, by definition.

I recommend https://danluu.com/wat/.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented May 20, 2020

@localheinz so, ppl you don't agree with are deviants? Isn't that a bit extreme?

@localheinz
Copy link
Contributor

@nicolas-grekas

@localheinz so, ppl you don't agree with are deviants? Isn't that a bit extreme?

That would be unreasonable, and I never said that.

@drealecs
Copy link
Contributor

drealecs commented May 20, 2020

Just to mention my opinion:

I can't really understand why we can't use ">=7.1.3 <8.1" or "^7.1.3 || ~8.0.0" that updates only once a year, once the pipeline is enabled for the new version, 8.0 in this case along the other 7.1, 7.2, 7.3 and 7.4, released in a new patch version across all major.minor maintained versions.
And the next year, after 8.1 pipeline passes, it will be ">=7.1.3 <8.2" or "^7.1.3 || ~8.1.0"

Is this update, similar with the one you just did, a very hard job on the core maintainer?

But once you do a release with ">=7.1.3", you can not go back so easily to having a max version.

The problem is not now, the problem might arrive in 2024 when 4.4 will not be supported anymore and an BC break with PHP 8.4 will allow the installation.

I've migrated older applications that didn't had a lot of automated tests to newer version of php.
One step here is to understand what libraries you need to upgrade.
A library that mentioned ">=5.3.0" is not helping at all and, even if you find references that it might work on 7.3, you can't be really sure about it.
Testing the application is another pain, a big one if production execution flows are not easy to replicate or well known.

Mentioning an upper bound eliminates this stress on the user's of the library/framework, in my opinion. And that's a big cost sometimes.
And to me, it looks like the effort here is just a once an year update, once you have the pipeline running for the new supported version.
Of course, this it's core maintainer effort and the other one is not. But isn't this what we do here in open source?: "doing effort so others don't have to do it".

@goetas
Copy link
Contributor

goetas commented May 21, 2020

Seeing this pull request already merged after half the internet is disagreeing with it makes me think that this project is not that much about community but about few people who do whatever they think is best, ignoring any other opinion. That's the opposite of open community.

@nicolas-grekas
Copy link
Member Author

This very PR has been created after spending hours to try adding a PHP nighlty job to the CI of Twig (which is a much simpler project than Symfony.)

This helped realize the mistake that using ^ is. Give it a try if you need experience on the topic to forge your opinion.

I wish all this energy to downvote the free work of others could be turned into PRs.

Let's continue working on #36872.

@fabpot
Copy link
Member

fabpot commented May 21, 2020

We respect all opinions. We listen to all comments. But at some point, we need to make decisions. If we look at the thumbs up and down, we can see that there is no clear consensus on what to do.

I won't re-iterate the reasons for this change as Nicolas listed them quite well. I could add more reasons, but I will refrain from doing so to not start a controversial conversation.

Voting on a PR is easy, finding solutions for real-world problem is hard. Before voting, I hope everyone has tried to understand our reasons and why we think this is the best thing for Symfony and the community (not just the developers using the full-stack framework but also the consumers of our components).

Also, and as mentioned by Nicolas, this is NOT a rushed PR created out the blue. It is the result of working on making sure Twig was compatible with PHP 8. It is the result of thinking about how to make it happen. It is the result of working on a real-world use case. I can tell you that we suffered. A lot. And this PR is the conclusion of that work. Not really the conclusion, but an open path to make Twig and Symfony compatible with PHP 8 is an easier way. Not just Twig, it is about helping the whole ecosystem to embrace PHP 8 as soon as possible. It is about helping adoption of newest PHP versions.

Now, in a few months, if we realize that it was a mistake, we will re-evaluate. Nothing is settled in stone. As a matter of fact, this very strategy was used before Symfony 4.

As a conclusion, I suggest that you do help us making Symfony compatible with PHP 8. Two choices: you create a PR with one needed change branching from the commit before this PR or branching on current 3.4 (so after this PR) depending on what you think is best. Or do the same for a bundle or a library using Symfony. At the end of day, that's our only goal: making sure that Symfony and the whole ecosystem is compatible with PHP 8 as soon as possible and as smoothly as possible.

@drupol
Copy link
Contributor

drupol commented May 21, 2020

Seeing this pull request already merged after half the internet is disagreeing with it makes me think that this project is not that much about community but about few people who do whatever they think is best, ignoring any other opinion. That's the opposite of open community.

That was rough. I found such comment completely unconstructive and demotivating.
I used to work with some other community before and ever since, Symfony's community is the best community I ever worked with.

Like @fabpot said, this change is for the greater good of the community, I don't think that their purpose is to introduce s**t in their softwares. Nothing is fixed, if there is a good reason to revert, they will do it.

Remember, they do this for a living ... and many people use what they do for free to make a living.

@driesvints
Copy link
Contributor

Now, in a few months, if we realize that it was a mistake, we will re-evaluate. Nothing is settled in stone. As a matter of fact, this very strategy was used before Symfony 4.

Definitely respect the decision here. You have to do what you feel is best. Just want to stress that this cannot be reversed as soon as a version is tagged with these changes. That version will forever keep these new constraints. So that's more or less set in stone.

@Slamdunk
Copy link
Contributor

Just want to stress that this cannot be reversed as soon as a version is tagged with these changes. That version will forever keep these new constraints. So that's more or less set in stone.

Re-tagging is allowed. Dirty practice, high unlikely to be accepted, stressful, but allowed and effective.

@pfrenssen
Copy link

pfrenssen commented May 27, 2020

I think the best approach is to consider the version range as the "currently supported range" for a given release. If a project declares that it currently supports ^7.1 then it should set up a test matrix in their CI for PHP 7.1, 7.2, 7.3 and 7.4. This gives the best possible indication that the software will work on any environment that Composer resolves the dependency for.

There is a risk in this approach in that there might be a hypothetical 7.5 release that might contain some breaking changes that we cannot yet test for. This risk is mitigated thanks to PHP using semantic versioning, they have a promise to not break B/C in minor versions. This is no hard guarantee but it is a reasonable assumption that any software working in 7.4 will continue working in 7.5 and beyond.

Now, declaring that the software will work on any future PHP version, without any test coverage, and without any knowledge what might change in the far future for PHP is a different matter. There is no promise of not breaking backwards compatibility.

Composer or any other package manager that we might be using in the future will still happily resolve >= 7.1.0 for PHP 14 in the year 2040. We can be pretty sure that our software from today will NOT work in PHP versions 20 years from now.

The PHP major release schedule is very much manageable, there is sufficient time between releases to allow any project to update their version restrictions. There is ~6 months between feature freeze (aka 8.0-beta1) and final release (ref https://wiki.php.net/todo/php80#timetable). In this period we can take the following steps:

  • Create a PR "PHP 8 compatibility"
  • Set the version to ^7|^8
  • Set up the CI test matrix to cover both PHP 7 and PHP 8
  • Fix any problems arising on PHP 8
  • Merge the PR only when tests are green on PHP 7 as well as PHP 8

When the next major version comes around, rinse and repeat.

Since this work can be undertaken by any contributor it doesn't put any extra effort on the shoulders of the project maintainers. In fact it will shift the work to the early adopters who are keen to try out the next PHP version.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented May 27, 2020

the best approach is to consider the version range as the "currently supported range" for a given release

The idea that >= declares compatibility with any higher version is where our reasoning diverges.

I'm going to take a fictional example here: let's say we could lock the operating system, and the version of it. Would you lock on Windows ^10 because you never tested on Windows 22? Nobody would be totally surprised if a software developed in the Windows 10 era doesn't work anymore on Windows 22. But I can run old MS-DOS programs that were developed in the 80s! If software were compiled with an arbitrary rule to bind them to some specific versions of some OSes, users from this future could be rightfully upset. This would basically be a DRM system: the editor of the software preempted usage rights from end-users.

Using ^ is basically doing DRM: the author takes over end-user's rights.
But as with any DRM system, the end-users is still always in control, because they're the one able to work around any DRM systems.

The only practical effect of a DRM system is to be annoying, and ^ is no different.

@pfrenssen
Copy link

pfrenssen commented May 27, 2020

What you are describing is attempting to run software outside of their designed boundaries. You are basically ignoring the platform requirements of the software. this is also possible in our case, by setting the --ignore-platform-reqs flag in Composer. The version constraints are not preventing anyone from running the software on any version if they choose to do so.

But I think the version constraint should match what is actually being tested in CI and is known to work. If we don't have green tests on PHP 8 it is too early to declare compatibility.

@wouterj
Copy link
Member

wouterj commented May 27, 2020

Please note that Symfony's promise is to have support for all new PHP versions in all versions that aren't yet end-of-life. This means that if 3.4.40 doesn't support PHP 8, 3.4.41 will. There is no point in installing 3.4.40 if 3.4.41 is also released. (and as we can see, adding support for newer versions is added before their stable PHP release, so you won't ever have 3.4.40 on a PHP 8.0 unless you're not on the latest patch release)
Yes, 3.3 is no longer supported so if 3.3.11 doesn't support PHP 8, you might have some trouble running it... but who is running completely outdated (we're talking about years outdated) PHP libraries on the cutting edge PHP versions?

Let's be practical here: Symfony used >= from 2.0 till 4.0, without any issues reporting problems. The "theorical" problems are almost unimaginable & using >= makes life easier for any package maintainer relying on Symfony.

I think the bike-shed is discussed enough and I trust Nicolas and Fabien to not make decisions to destroy their life project/company. If you have some more time, please help reproducing reported bugs instead :)

@gmponos
Copy link
Contributor

gmponos commented May 27, 2020

Symfony used >= from 2.0 till 4.0

For the record.. I have shared (not here) that I do find some reasoning behind this move (although overall I am not convinced) but the above argument IMHO is one of the most invalid one.. The state of the PHP community was not the same then.. as it is now in order to use this experience.

OFC Same thing can be said counterwise.. meaning that more projects now have CI, static analysis etc etc.. but my point is that IMHO previous experience can not be used as argument (at least for this kind of issue)...

@rask
Copy link

rask commented May 28, 2020

This puts developer convenience before user safety. Who thinks it is more important to be able to develop stuff in a certain way because the alternative way is oh so boring and a burden and slow and I dont have time for this pls, instead of making sure users are kept safe in terms of working software and ability to do their tasks?

@fabpot
Copy link
Member

fabpot commented May 28, 2020

Do you realize that this change makes things more convenient for users and very much more difficult for us, maintainers? Anyway, I think I will stop commenting here as there is no point. I wish people would be more constructive and more willing to help on fixing issues and improving the framework instead of ranting.

@ndench
Copy link
Contributor

ndench commented May 28, 2020

End users are free to put ^7.1.3 or whatever php version constraint they want in their own composer.json. It's not Symfony's job to prevent you from running your code on an unsupported version. By allowing Symfony users to run the framework on 8.0 (or any future version) gives more people the opportunity to find and raise issues related to 8.0 compatibility.

If the constraint stayed at ^7.1.3, then in order to successfully update to 8.0, the core maintainers need to have a long running branch where they need to find at least most of the compatibility issues themselves before tagging a release.

This allows distributing of effort to find bugs, and actually allows the whole ecosystem to move forward faster.

Again, if you don't like it, restrict yourself to ^7.1.3 in your own composer.json and you won't have an issue.

@emullaraj
Copy link

I personally can't see the controversial point on this PR. We need a minimum stability for sure. This is a clear declaration of the maintainers to keep up with the ecosystem improvements.
Any personal opinion on this decision can be easy tweaked by a constraint in user land to reflect their own choice. This is open sorce democracy spirit at the highest level.

@derrabus
Copy link
Member

derrabus commented Jun 6, 2020

Related: symfony/psr-http-message-bridge#79

@drupol drupol mentioned this pull request Jun 1, 2021
4 tasks
@drupol drupol mentioned this pull request Dec 7, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet