[stable25] Fix 32bit support and add workflow for 32bits testing

[MichaIng]

• Resolves: [stable25] Quota value as float for 32-bit systems #35734 (the PR did not really fix it)

Summary

This backports most changes from:
#36120

Excludes union type, supported by PHP 8.x only
replaced with phpdoc in case.

TODO

• Verify that all types are aligned, since there was some conflicting quota related solution with float only instead of int|float.
• Discuss whether the new Utils function is fine here.
• Verify that nothing was backported which depends on other changes on NC26.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[come-nc]

It is problematic to merge type changes in OCP in a bugfix release.
Changing @return from int to int|float on some methods which were already silently returning float on 32bit is okay, but removing a strong type return of : int is not I think.
We cannot guess what will break or not as a result in the many Nextcloud applications outthere.

[come-nc]

I do not think this change is allowed in a bugfix release

[come-nc]

Same thing

[MichaIng]

Many thanks for your review.

but removing a strong type return of : int is not I think.

The problem is that exactly this strong types breaks things. I'm not sure which exact case of this breaks e.g. quota in NC25 currently (+ large trashbins, large files downloads and a few other long standing issues with 32-bit), but it works in NC26 where those return types have been changed to : int|float (not possible here due to PHP 7.4). And the way it is there are for sure many other subtile issues caused by them.

Removing strong return types cannot break anything which worked before, can it? I mean all calls which did return an int, keep returning an int. What does change is that there were failing calls (which wanted to return float), which do now work correctly. The negative is that theoretically invalid calls (which would return something non-numerical) would now not fail as of strict return type but at least in phpdoc.

[come-nc]

Removing strong return types cannot break anything which worked before, can it?

https://3v4l.org/jd7hM -> remove the return type here and you get a crash. Note that this only works with strong_type disabled and it does emit a deprecation warning on modern PHPs.

But there may be other subtle differences, and this is the kind of headaches one tries to avoid in a bugfix release.

I’d like to hear other opinions but for me we should not touch that on 25 branch. People wanting the return type change can update to 26.

[MichaIng]

remove the return type here and you get a crash.

But on 32-bit systems it crashes as well with int return type if a tries to return a too large number. Similarly leaving int in place where 32-bit cannot deal with it may cause subtile errors. Isn't CI assuring that there are no conflicting types?

I mean I understand the concerns: It is basically risking errors on 64-bit (or in general) vs leaving known (and likely additional unknown) errors on 32-bit. It is above my knowledge/insights to weight those against each other.

We could also try to leave the return types untouched and see whether the other changes still fixe at least the quota bug. But especially those getSize functions are actually so clearly a point of failure for 32-bit systems and they are used at so many places, and those int return types were added with NC25, where this major 32-bit breakage started with...

Btw, also here the return type has been removed in attempt to fix the quota: https://github.com/nextcloud/server/pull/35734/files#diff-b89bc6bd7d5336d68032bb5a5c1f309df16f27173f9f58d9f338ec495d3f951bR63

[come-nc]

Btw, also here the return type has been removed in attempt to fix the quota: https://github.com/nextcloud/server/pull/35734/files#diff-b89bc6bd7d5336d68032bb5a5c1f309df16f27173f9f58d9f338ec495d3f951bR63

Indeed this PR is doing the same thing. So I don’t know maybe we can change it. But at the time of this PR 25 users on 32bits were stuck, this is not the case anymore since 26 is out.

[MichaIng]

That NC26 does not support PHP 7.4 anymore is a reason why there are still many users stuck (who cannot or do not want to upgrade their distro/PHP yet). But yeah, let's wait for other opinions.

[blizzz]

what's the state about this PR?

[MichaIng]

@blizzz
Someone needs to decide whether changing return types of public interfaces to fix 32-bit issues on a stable Nextcloud version is okay or not.

[blizzz]

@blizzz Someone needs to decide whether changing return types of public interfaces to fix 32-bit issues on a stable Nextcloud version is okay or not.

I do agree with @come-nc that these are changes of the type you want to avoid on maintenance releases. They may change behaviour in unexpected ways, and even if something it broken, it is more stable and more predictable to have a known defect over potentially introducing yet unknown and hard to debug problems.

[MichaIng]

Okay, good argument. I wonder if there is anything else left in this PR which could possibly fix issues, or if we need to accept that NC25 won't be fully 32-bit compatible. I mean NC26 has been released and NC27 is in RC phase, so it is not such a major problem anymore, IMO.

[lapineige]

So this would be a won't fix ?
And fixed in later major versions ?

[MichaIng]

It is fixed in NC26 already. This PR (as the title indicates) is a backport of these fixes from NC26. But indeed, as it required potentially breaking API changes, it ist most likely a nofix.

Everything else is not really fixing something but mutes potential test errors and annotations or minor cleanup, not worth the efforts, so I'll close this PR.