BC in php_uname() from 8.3 to 8.4-RC1

[that-guy-iain]

Description

The following code

<?php
$platformStr = php_uname('s r v m');

Resulted in this output:

  #message: "php_uname(): Argument #1 ($mode) must be a single character"

But I expected this output instead:

No error 

PHP Version

PHP 8.4-RC1

Operating System

Ubuntu 22.04

[damianwadley]

What's your expected output? The uname for the first character, s? Because that's all PHP would ever return.
https://3v4l.org/l60Xo

So the error is now pointing out a bug in the code. And the change is present in the appropriate files so that the documentation will be updated to mention the bug fix.
https://github.com/php/php-src/pull/15385/files

[that-guy-iain]

My expected outcome is no error. This is code is being used by a third-party library to identify PHP version usages. I personally, don't care about it or use it. But I do care that my code broke because of a breaking change that isn't mentioned in the documentation as php_uname('s r v m'); style mode is mentioned on the documentation page.

'a': This is the default. Contains all modes in the sequence "s n r v m"

[jimwins]

Elevating this from a silent failure to an explicit error just hadn't been added to the documentation yet. php/doc-en#3968

php_uname() has been documented as only taking a single character for $mode for twenty years.

php/doc-en@d34c952

[damianwadley]

My expected outcome is no error.

Obviously. I was thinking more like "The expected behavior is that calling php_uname() with the string s n r v m, as mentioned in the documentation, would return a string with those five values inserted into it, similar to how date() works with date and time formatting placeholders." Which makes sense... except that the function has never worked that way, and it returns anything at all (by only using the first character in the string) due to a quirk of the implementation.

Fact is, php_uname("s n r v m") does not work and has never worked. If anything, it being an error with PHP 8.4 has had the unexpected benefit of pointing out that the code was incorrect.

a breaking change that isn't mentioned in the documentation

PHP 8.4 hasn't released yet.

[that-guy-iain]

Elevating this from a silent failure to an explicit error just hadn't been added to the documentation yet. php/doc-en#3968

Let's not act like is only because I noticed the breaking change. Months later a PR is opened. As I said, in the pr you created quietly changing docs is even worse than haphazardly adding breaking changes.

Fact is, php_uname("s n r v m") does not work and has never worked. If anything, it being an error with PHP 8.4 has had the unexpected benefit of pointing out that the code was incorrect.

Breaking people's code for no other benefit is not a benefit. It should not be needed to be pointed out the importance of backwards compatibility on a project of this scale, yet here we are. PHP has a terrible reputation quite simply because of cowboy antics like this from decades ago where people would just go change stuff without any thought of others. There are RFCs for other issues like this but not this one.

There are tons of bugs in PHP which are valid and annoy the hell out of people but are left there because backwards compatibility. But here we are with simple code that is implied to work on the documentation no longer working and the breaking change not being removed, why? What is the benefit is there to break other people's code with this change? It should not be needed to be pointed out that you should only inconvenience others to when you're adding benefits and not when there are just cons.

Sorry if this seems abrasive but the absolute nonsense that goes on in this industry and the amount of work others are willing to make others do to save fixing an issue they created pisses me off. As it reads here, you don't care if it'll cause others problems.

[alecpl]

You might argue that a proper fix should have been to support such input and treat it as a format string.

Imo, this at least should be changed to a deprecation warning in 8.4.

[cmb69]

As PHP developer, I'm in favor of having the engine telling me that I'm doing something what may not work as expected. As php-src developer, I'm in favor of being able to understand that only taking the first character of the string into account is not an oversight, but a deliberate design decision.

That said, I agree that the change may be too drastic, and that we should reconsider to downgrade to a deprecation notice for now. @Girgias, what do you think?

[cmb69]

Oh, @php/release-managers-84 may have something to say about this, too.

[NattyNarwhal]

I think turning it into a warning (not necessarily deprecation since we've never allowed it?) makes sense. It is a minor change to user code if we keep it as a ValueError at least.

That said, it seems thinking this function supported multiple flags is a common mistake. I could see it with how the uname command works. Maybe we could consider changing that for 8.5/9.0.

[jimwins]

Elevating this from a silent failure to an explicit error just hadn't been added to the documentation yet. php/doc-en#3968

Let's not act like is only because I noticed the breaking change. Months later a PR is opened. As I said, in the pr you created quietly changing docs is even worse than haphazardly adding breaking changes.

This change was documented in the UPGRADING and NEWS files in the release (since 8.4.0beta3), and right now we're in the interregnum where those notes get folded into the documentation proper. There's a meta-issue tracking this. php/doc-en#3872

Yes, of course, I prioritized this particular bit of documentation because of this issue, but nobody is doing anything quietly here.

[Girgias]

As PHP developer, I'm in favor of having the engine telling me that I'm doing something what may not work as expected. As php-src developer, I'm in favor of being able to understand that only taking the first character of the string into account is not an oversight, but a deliberate design decision.

That said, I agree that the change may be too drastic, and that we should reconsider to downgrade to a deprecation notice for now. @Girgias, what do you think?

It should be a warning at worse, not a deprecation, as this behaviour was never supported.

And once again I am baffled that erroring on a clear programming bug is considered a BC break instead of telling people to fix the obviously broken code.

PHP has a terrible reputation because it doesn't report errors like these, not because of its lack of BC.

The most recent time the documentation of the php_uname() was changed was 2009, and it has indicated it is a single character since 2004. (Ignoring adding a useless changelog that got reverted soon after in 2018).

ValueErrors are only added for objective programming errors, which this is, and we have done this without RFCs because extension maintenance can do this.

I don't care that this is changed to an E_WARNING,

Sorry if this seems abrasive but the absolute nonsense that goes on in this industry and the amount of work others are willing to make others do to save fixing an issue they created pisses me off. As it reads here, you don't care if it'll cause others problems.

Yes, you are being abrasive, which does not make me want to be at all helpful. This cause "problems" by pointing out a bug in your code as you have written it. Not a runtime bug that depends on the environment, one that is consistently reproducible.
I, as a recent language maintainer, care about preventing dumb bugs by blowing in people's face ASAP, not having them scream at their computing trying to understand why it is not working as expected. Yes this is apparently in stark contradiction to some prior way of maintaining PHP, but a project is allowed to evolve and how it deals with such topics.

You are claiming we changed the documentation under your nose, which we didn't.
You rant that we are breaking code for no good reason, which I find to be complete nonsense in this case, as exposing a bug has obvious good reasons.

The only valid claim here, is that the documentation for it hasn't been updated yet, well this is open source and as everything, lacks maintainers.
Frankly I am getting fed up with people ramming into maintainers, being abrasive and overall acting in a way that warrants a block and mute, especially by people that do not contribute back to, but heavily rely on, OSS that seem entitled to have everything go their way and anything that is an inconvenience is somehow an affront to their personal right to use the software.

As a reminder, PHP is provided:

[...] BY THE PHP DEVELOPMENT TEAM ``AS IS'' AND
ANY EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE PHP
DEVELOPMENT TEAM OR ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
OF THE POSSIBILITY OF SUCH DAMAGE.

[cmb69]

@Girgias wrote:

This cause "problems" by pointing out a bug in your code as you have written it.

@that-guy-iain wrote:

This is code is being used by a third-party library to identify PHP version usages.

And I think this is the problem here. Fixing your own code is trivial; having to deal with a break in an upstream library might not.

Anyhow, being abrasive is illogical, since it rarely helps to reach a goal.

[Girgias]

We need to deal with upstream breaks (and bugs) too, libxml2 being the main one recently.
And sometimes it points to a bug in our code that we need to fix, rather than force userland to dodge around our bug in the implementation.

I don't really understand why the first complaint is to go to php-src to rant instead of reporting the bug to the third party library, and/or submitting a PR to fix it.

This is like someone using the DOM in JavaScript and encountering a browser bug, going all the way to complain to WHATWG about a bug in the DOM spec instead of filing a bug against the browser.

[that-guy-iain]

@Girgias my first port of call is the bug tracker for the project that added a breaking change. It was then confirmed then met with shrugs. Then you get a rant. The third-party library didn't change their code and add a bug. Their code was working. Then PHP decided to add a breaking change without telling anyone and when told they added a breaking change they pretended like the code that caused the issue to did not come directly out of the docs.

This is a project that literally powers more than half the internet, being "hack and slash" on a project like that should be scolded. It should be expected that they at least mention breaking changes. But realistically discussing breaking changes in a project like this should be a standard practice and a requirement. I checked internals, rfcs, etc and saw other discussions about breaking changes such as adding throwing errors to things that didn't throw errors.

And PHP has a terrible reputation mostly because of the good ole days where things just got done by people just doing things without any discussion and impact. PHP will increase it's terrible reputation if it's seen to be a language where libraries break from version to version because no one cares about BC and they can hack and slash breaking changes whenever they want.

[cmb69]

Then PHP decided to add a breaking change without telling anyone […]

All relevant changes regarding new minor/major PHP versions are supposed to be documented in UPGRADING (which is part of the official tarballs). In this case

php-src/UPGRADING

Line 125 in d5e6dd8

. php_uname() now throws ValueErrors if the $move parameter is invalid.

This is not documented in the PHP manual yet, because the doc team is catching up; and after all, PHP 8.4.0 has not yet been released.

[that-guy-iain]

All relevant changes regarding new minor/major PHP versions are supposed to be documented in UPGRADING (which is part of the official tarballs). In this case

Ok, I screwed up on that point. I'll hold my hands up there.

[Girgias]

The third-party library didn't change their code and add a bug. Their code was working.

This is a wild assumption. It is "working" by the definition of "returning some output", not by "actually doing what I want the function to be doing".

The bug has always existed in their code, the fact it got exposed by an upgrade is not something that is bad. I am utterly baffled that you continue to argue the code in question was "working" and was "bug free" when it clearly is not.

PHP will increase it's terrible reputation if it's seen to be a language where libraries break from version to version because no one cares about BC and they can hack and slash breaking changes whenever they want.

We do not do breaking changes in patch versions, PHP does not and has never followed semver (and there have been BC breaks in every single minor version of PHP going all the way back to PHP 4.1). It is widely known that you cannot just "blindly" upgrade without having a cursory glance at the migration guide.
PHP has a policy of what can and cannot be done in a minor version, and we allow minor BC breaks. Throwing an Error on inputs that are complete nonsense and should never happen unless you have a bug, like in the case of that library, is fine from our PoV. You can disagree about the policy, but it does not give you any right to rant that we are not being serious about BC and just do stuff YOLO.

The "nice" migration guide has somewhat been written, and it mentions the new ValueError. It is just not being advertised because we are still in RC. Which is also the point to give library maintainers to go and fix their stuff before GA, but even then they are under no obligation to support the latest PHP version the moment it comes out.