Fix big file download for 32-bit servers

[fwsmit]

When a folder larger than ~2GB is downloaded, zip32 doesn't cut it. Since zip64 doesn't work on 32-bit clients, fall back to generating a tarball on 32-bit servers. This PR doesn't change anything for 64-bit servers, since they can just use zip64

Fixes #12422 and #15117

Ref #16636: it only fixes the warning, but doesn't fix the issue of broken zip file generation.

Tested this change on the odroid-xu4 (armv7l, 32-bit)

• arch linux arm
• nextcloud 24.0.2-1

[fwsmit]

I don't think the two failed tests are because of the code change.

[szaimen]

Hi, thanks for the PR but 32bit is officially not supported anymore. See nextcloud/documentation#9071

[fwsmit]

I saw that as well. But does that mean no PR are accepted that improve 32 bit? As I said, this should not affect 64 bit

[szaimen]

this is not acceptable

[szaimen]

See #16636

[fwsmit]

I've read that issue. When size is bigger than a 32 bit int can handle, the size overflows to a float. This will not be as accurate, but there is an error margin.
As for 64 bit systems you are only losing the safety that the type hint provides. It's that unacceptable? If so, is there another way of fixing this that would be accepted? For example, checking if 32 bit and calling another function all together?

EDIT: I could also overload the streamer class to provide a 32 bit variant

[fwsmit]

Hi, is there any way a fix for this issue can be accepted upstream? I've mentioned a few ways to do it in the review comments.
32 bit not being officially supported does not mean no patches can be accepted :)

[fwsmit]

Hi @szaimen, I've asked for clarification about the 32-bit support on the nextcloud forums: https://help.nextcloud.com/t/clear-up-situation-around-32-bit/143779. After discussion with the developers, it seems that 32-bit patches could be accepted:

Hello hello,
So I discussed this situation with the relevant developers and the director of engineering. We agreed to accept pull requests for 32 bit fixes from the community if the PR’s seem of a good quality and will pose little risk for causing problems. (Please note we do not test the PR’s ourselves locally against 32 bit because we don’t have those test environments anymore, so an accepted PR doesn’t mean we really guarantee the intended functionality, and if the PR’s do cause problems and we reject it, we can’t dig to give more specific pointers on how to improve the PR.) I hope this is good with you?

So as for the actual PR, I get that it's a bit of a hack. If that's not acceptable to you, I see only 2 alternatives:

1. Add a library like https://github.com/brick/math and use BigInteger for size attributes. (Remove type for size arg on Streamer constructor #16636 (comment))
2. Use a separate function for 32-bit downloads and only use the hack there

Seeing as 32-bit is not officially supported anymore, I think that adding a library is not the way to go. So that leaves use with making a separate function for 32-bit downloads. I don't see any problems with that. There is no risk of regressing the 64-bit servers (except if adding a single 32-bit check would be a big performance hit, but I don't think so) and the hack stays confined in the 32-bit space.

Let me know what you think.

[szaimen]

Honestly, I don't think adding any workaround for 32bit is acceptable now that 32bit is completely broken with NC25 which happened because we do not test against 32bit which was the main reason why we dropped 32bit support in the first place.