nextcloud / news

:newspaper: RSS/Atom feed reader
https://apps.nextcloud.com/apps/news
GNU Affero General Public License v3.0
856 stars 183 forks source link

News update 16.0.0 with not fulfilled dependencies #1423

Closed bcutter closed 3 years ago

bcutter commented 3 years ago

IMPORTANT

Read and tick the following checkbox after you have created the issue or place an x inside the brackets ;)

Explain the Problem

What problem did you encounter? Tried to update NC News to version 16.0.0 with occ app:update --all. Result after needed occ upgrade is

Exception: App "News" cannot be installed because the following dependencies are not fulfilled: 64bit or higher PHP required.

No way to get out of maintenance mode. NC News update blocks whole Nextcloud.

Steps to Reproduce

See "Explain the Problem" section above.

By disabling news the lock situation could be resolved. But now NC News is not available anymore (occ app:enable news gives App "News" cannot be installed because the following dependencies are not fulfilled: 64bit or higher PHP required.).

System Information

Contents of nextcloud/data/nextcloud.log ```json Possible sensitive information (!) ```
Contents of Browser Error Console Read http://ggnome.com/wiki/Using_The_Browser_Error_Console if you are unsure what to put here ``` Not necessary for sure! ```

What are the explicit v16.0.0 requirements? 64-Bit OS? That would´ve be a total kill. Higher PHP version? Which one? More details please. Not the nice way to discover BY/AFTER running an update that it breaks the whole app. And YES, I´ve read (because accidentally discovered) https://github.com/nextcloud/news/issues/935.

bcutter commented 3 years ago

Update: I discovered in change log https://apps.nextcloud.com/apps/news/releases?platform=21#21

News now requires a 64bit OS

OK wait a minute I´ll upgrade my whole server. 🤣 Hell no, that´s a hell of a "breaking change" guys!

So as emergency immediate response: How to downgrade from started update (16.0.0) back to 15.4.5???

Grotax commented 3 years ago

As you already discovered, news requires 64bit starting with version 16. It is also mentioned in the changelog.

The reason is that news requires a high resolution of time to be more accurate with synchronization.

It worked in older versions because there was some workaround created that didn't store timestamps as numbers but as strings in the DB, but programmatically this is an unclean solution and it broke due to changes applied to the code not long ago, so for 15.x we decided to create another workaround but to drop it with 16.

And yes I know that raspberry pi OS is still stuck on 32bit and that there is only a beta for 64bit. But there is also a Ubuntu version which already provides 64bit as default.

That the upgrade failed and got stuck is unfortunate but nothing we can fix, it would be a server issue as the app can't control the install process. I would expect the updater of the sever to check the dependency before downloading the app.

You should be able to downgrade manually to 15.x. By deleting the news folder from apps/ and downloading a appropriate version from the app store. Unpack it and activate the app via OCC. And of course don't update it anymore if possible (maybe do a backup in case the sever decides to run the update again)

bcutter commented 3 years ago

Thanks for explanations.

1) First things first: managed to downgrade to 15.4.5 and will try to conserve that version until a final solution is been found. For the moment NC News is back operational.

2) Nextcloud app update mechanism: that one is probably bringing in some more users running in the same situation like I was/am. Maybe you as app maintainer can use your impact and propose app update mechanism changes so that dependencies are actually checked before updating apps. That´s so basic stuff, I´m really surprised to run in such an situation nowadays. The current mechanism AND messages are not very helpful. Even the current ...because the following dependencies are not fulfilled: 64bit or higher PHP required. raises false hopes: a higher PHP version would require quite some effort leaving latest stable APT repository of the OS behind, but would be manageable with a specific amount of time and with known and probably acceptable side effects.

3) The 64 bit dependency itself is really a huuuuuuuuuge major - how to say in friendly words, skipping all the others coming to my mind at first - horror scenario. Imagine running Nextcloud next to plenty other services plain without dockerization on a Pi. Now one single app of one single service needs a 64 bit OS... there´s no stable version of the 64 bit Pi OS yet. And even if it would be (and probably will be until the end of the year I guess) - there´s no upgrade path from 32 to 64 bit, basically in almost no OS. That means: complete reinstall. Starting from scratch. In my case even with a precise planning phase of at least 1/2 a day before the actual update, it would roughly take me AT LEAST one weekend for the update itself. Based on my experiences I double that, expecting other things to fail which need to be fixed and so on. Oh man... WORST CASE SCENARIO ⚠, really.

LeSpocky commented 3 years ago

As a user: Okay, I just clicked update in nextcloud web ui like with every other app, which always worked flawlessly and now I'm stuck, because the web ui does not offer me to rollback nor deactivate that plugin. I simply can not use nextcloud, because requirements were not checked before update. Upgrading the WHOLE server to 64bit is no option, because that would mean a fresh installation of the whole operating system.

As a programmer (C/C++) I'm confused, 64bit integer types do work on 32bit systems, and if you need even higher precision (you don't for time, struct types exist, you can store seconds and nanoseconds side by side), there are libs for integers with arbitrary precision. Forcing the WHOLE environment to change is an absolute NO GO. There are 32bit only architectures (armv7a, like beaglebone black or tons of other SBC) out there and a single plugin breaking nextcloud to run on those platforms at all? Get your stuff together, find a solution that works on 32bit architectures!

grumpy

LeSpocky commented 3 years ago

As a user: Okay, I just clicked update in nextcloud web ui like with every other app, which always worked flawlessly and now I'm stuck, because the web ui does not offer me to rollback nor deactivate that plugin. I simply can not use nextcloud, because requirements were not checked before update. Upgrading the WHOLE server to 64bit is no option.

I could disable the news app like this from commandline:

sudo -u www-data php ./occ app:disable news
bcutter commented 3 years ago

As a user: Okay, I just clicked update in nextcloud web ui like with every other app, which always worked flawlessly and now I'm stuck, because the web ui does not offer me to rollback nor deactivate that plugin. I simply can not use nextcloud, because requirements were not checked before update. Upgrading the WHOLE server to 64bit is no option.

I could disable the news app like this from commandline:

sudo -u www-data php ./occ app:disable news

Yes that´s a quick fix to get rest of Nextcloud back working. BUT: during a (I´m quite sure: EVERY) usual Nextcloud version update (like I did some minutes ago from 21.0.2.1 to 21.0.3.1) we´ll run into this issue. Because "occ update" is performed during an update and here NC News app got updated once again.

So I performed manual app downgrade twice today. I´m prepared to do this on every future NC version update...

That´s my whole downgrade process, maybe someone else can adjust (important, every environment is a different) and use this.

sudo -u www-data php /var/www/nextcloud/occ app:disable news
sudo -u www-data php /var/www/nextcloud/occ maintenance:mode --on
cd /home/user/downloads
wget https://github.com/nextcloud/news/releases/download/15.4.5/news.tar.gz
tar -xvzf news.tar.gz
sudo mv /var/www/nextcloud/apps/news /var/www/nextcloud/apps/news_broken
sudo mv /home/user/news /var/www/nextcloud/apps/
sudo ll /var/www/nextcloud/apps/ | grep news
sudo chown www-data:www-data -R /var/www/nextcloud/apps/news
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/AUTHORS.md
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/CHANGELOG.md
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/COPYING
sudo ll /var/www/nextcloud/apps/ | grep news
sudo ll /var/www/nextcloud/apps/news
sudo -u www-data php /var/www/nextcloud/occ app:list | grep news
sudo -u www-data php /var/www/nextcloud/occ maintenance:mode --off
sudo -u www-data php /var/www/nextcloud/occ upgrade
sudo -u www-data php /var/www/nextcloud/occ app:enable news
sudo -u www-data php /var/www/nextcloud/occ upgrade
[check NC News is working on the browser]
sudo rm -R /var/www/nextcloud/apps/news_broken
sudo ll /var/www/nextcloud/apps/ | grep news
bcutter commented 3 years ago

As a user: Okay, I just clicked update in nextcloud web ui like with every other app, which always worked flawlessly and now I'm stuck, because the web ui does not offer me to rollback nor deactivate that plugin. I simply can not use nextcloud, because requirements were not checked before update. Upgrading the WHOLE server to 64bit is no option.

As a programmer (C/C++) I'm confused, 64bit integer types do work on 32bit systems, and if you need even higher precision (you don't for time, struct types exist, you can store seconds and nanoseconds side by side), there are libs for integers with arbitrary precision. Forcing the WHOLE environment to change is an absolute NO GO. There are 32bit only architectures (armv7a, like beaglebone black or tons of other SBC) out there and a single plugin breaking nextcloud to run on those platforms at all? Get your stuff together, find a solution that works on 32bit architectures!

grumpy

As I can understand the emotional power behind your words I´m not expert enough to share/support all details.

But I fully support your core message (which currently is my only hope to avoid 3 to 4 full (holi)days of OS upgrade and bugfixing): there must be another way to continue using NC News on 32 bit OS.

SMillerDev commented 3 years ago

Maybe you as app maintainer can use your impact and propose app update mechanism changes so that dependencies are actually checked before updating apps.

We don't have more power than normal users just because we maintain an app. You should create an issue for this since it affected you.


To everyone mentioning that we're maintaining the app wrong, you can always fork it and maintain a 32bit compatible version. We've decided that this was the best approach but you don't have to.

Grotax commented 3 years ago

News 15.x This version is now end of life, it won't receive any updates anymore. It will also not run on Nextcloud 22 by default, you may force to enable.

32bit vs 64bit We discussed this with users and internally already and we came to the decision that we will only support 64bit, this decision is finite.

The updater We didn't intend for the updater to still attempt the update even though the dependencies are not fulfilled, if you want this fixed you need to open an issue in nextcloud/server.

I can understand that for you this decision is annoying and requires more work, I hope that everyone will be able to upgrade to 64bit soon without major issues.

And in your discussions please keep in mind that the maintainers of this app do all this work in their free time as a hobby, all of us have regular jobs and other hobbies and social interactions. So keep it calm otherwise I will have to kick you out.

LeSpocky commented 3 years ago

I hope that everyone will be able to upgrade to 64bit soon without major issues.

This won't be possible for everyone. There are modern 32bit only architectures and people use them. Upgrade would not only mean a fresh operating system installation, but buying new hardware. You can still decide to not support 32bit, but 32bit-only hardware won't vanish just by hoping it will. 🤷🏼‍♂️

Grotax commented 3 years ago

We know that, I personally don't care if 32bit hardware stays or not. But a 64bit Integer simply doesn't work on 32bit and there is nothing we can do to change that, the original issue came up because people reported integer overflows. As you can imagine there is no automated testing on a 32bit system in place.

And I'm aware that this means for some users that they have to buy new hardware but there is no way around that.

shane-kerr commented 3 years ago

I've got a not-ancient 32-bit system (Helios4, something like 2 years old), and am not super eager to replace the hardware.

If the issue really is resolution of time on 32-bit, then I'm happy to try to find a solution. Is this a PHP issue? (Keep in mind that doing math with numbers bigger than the word-size of a computer is something that has been done for at least 50 years, so we don't need any radical new technology.)

Grotax commented 3 years ago

Yes it is a PHP issue, see https://github.com/nextcloud/news/issues/1320 for the details.

LeSpocky commented 3 years ago

The updater We didn't intend for the updater to still attempt the update even though the dependencies are not fulfilled, if you want this fixed you need to open an issue in nextcloud/server.

See: https://github.com/nextcloud/server/issues/27797

velaja commented 3 years ago
sudo -u www-data php /var/www/nextcloud/occ app:disable news
sudo -u www-data php /var/www/nextcloud/occ maintenance:mode --on
cd /home/user/downloads
wget https://github.com/nextcloud/news/releases/download/15.4.5/news.tar.gz
tar -xvzf news.tar.gz
sudo mv /var/www/nextcloud/apps/news /var/www/nextcloud/apps/news_broken
sudo mv /home/user/news /var/www/nextcloud/apps/
sudo ll /var/www/nextcloud/apps/ | grep news
sudo chown www-data:www-data -R /var/www/nextcloud/apps/news
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/AUTHORS.md
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/CHANGELOG.md
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/COPYING
sudo ll /var/www/nextcloud/apps/ | grep news
sudo ll /var/www/nextcloud/apps/news
sudo -u www-data php /var/www/nextcloud/occ app:list | grep news
sudo -u www-data php /var/www/nextcloud/occ maintenance:mode --off
sudo -u www-data php /var/www/nextcloud/occ upgrade
sudo -u www-data php /var/www/nextcloud/occ app:enable news
sudo -u www-data php /var/www/nextcloud/occ upgrade
[check NC News is working on the browser]
sudo rm -R /var/www/nextcloud/apps/news_broken
sudo ll /var/www/nextcloud/apps/ | grep news

thanks for this. It works except feeds do not refresh automatically anymore. I have to use occ news:updater:update-feed userid feedid

How do we refresh feeds automatically?

ArnaudD-FR commented 3 years ago

Glad to hear that 64bits is now mandatory!!!!!

I use raspberry pi OS and now my instance is blocked in maintenance mode!

Can you put in red/bold this new important requirement when upgrading news?!!! Who read the notes?

Grotax commented 3 years ago

thanks for this. It works except feeds do not refresh automatically anymore. I have to use occ news:updater:update-feed userid @feedid

How do we refresh feeds automatically?

That is probably a known bug in the nextcloud job system that sometimes jobs stale for ~24h. You might also want to check here https://nextcloud.github.io/news/faq/#feeds-not-updated

Grotax commented 3 years ago

Glad to hear that 64bits is now mandatory!!!!! I use raspberry pi OS and now my instance is blocked in maintenance mode! Can you put in red/bold this new important requirement when upgrading news?!!! Who read the notes?

We never intended that the update gets rolled out on your server if you don't fulfill the requirements but there is nothing we can do about that, there is no way to test this.

And no we can't make a big red warning anywhere as it's not something supported by nextcloud. We can't interact with users of news in any way.

SMillerDev commented 3 years ago

Who read the notes?

If you don't read them, would it even matter if we put it in bold/red?

coelner commented 3 years ago

Could you explain the background to this time resolution problem? Do you need a 64bit OS because of the features of the 64bit hardware extension, or because it is mandatory for the stack on which the news app runs or is it the database? If I get this right, you saved the time as string to omit the unixtimestamp?

I'm curious to understand this problem. As an embedded hardware developer I didn't see the flaw

ArnaudD-FR commented 3 years ago

Who read the notes?

If you don't read them, would it even matter if we put it in bold/red?

Somewhere else than the notes, maybe during upgrade process, some prechecks...

But as Grotax said you have no way to interact with users. Bad point for nextcloud!

Nextcloud does not provide information about global server installation profiles like architecture before taking this kind of decision?

As far as I know NC didn't deprecated 32bits so I was expecting all apps to support the same requirements... That's why this was a huge surprise to discover this new requirement!

ArnaudD-FR commented 3 years ago

I've just checked and the official architecture when downloading raspbian OS is still armhf.... I would be curious to know how many users who are using raspberry pi as home server are impacted by this decision.

SMillerDev commented 3 years ago

Could you explain the background to this time resolution problem? Do you need a 64bit OS because of the features of the 64bit hardware extension, or because it is mandatory for the stack on which the news app runs or is it the database? If I get this right, you saved the time as string to omit the unixtimestamp?

I'm curious to understand this problem. As an embedded hardware developer I didn't see the flaw

https://github.com/nextcloud/news/issues/1320

Nextcloud does not provide information about global server installation profiles like architecture before taking this kind of decision?

Nextcloud doesn't do much for this app, other than host it. And the maintainers of apps do not have access to Nextcloud install statistics AFAIK.

bcutter commented 3 years ago

I've just checked and the official architecture when downloading raspbian OS is still armhf.... I would be curious to know how many users who are using raspberry pi as home server are impacted by this decision.

I bet a lot. The more interesting question is: how many (of those) use the News app. Don't think there is overall metrics on that (privacy by design).

Even I internally hope there are a lot more users affected by this really bad / controversial 64 bit decision, I can only hope the maintainer team will rethink it.

It's like "Hey citizens, starting tomorrow we exchanged your right hand drive cars by left hand drive cars (or vice versa depending on the place on earth you live). It's only needed for one area of your country and we won't tell you bout this in advance. DEAL WITH IT." 😂

SMillerDev commented 3 years ago

Even I internally hope there are a lot more users affected by this really bad / controversial 64 bit decision, I can only hope the maintainer team will rethink it.

Honestly if y'all are in agreement that I make bad decisions as a maintainer I'm fine stopping all together and just keeping my own private copy of news that works just for me. Drive-by negativity like this is actively killing open source, good job!

ghtux commented 3 years ago

For me it's like fuck, maybe in future I can't run news anymore on my Pi 32. To be honest news is my most used daily thing. It's more sadness but for sure not offending any maintainer. Guess no one here hates foss or doesn't appreciate the work of the devs.

joeheaven commented 3 years ago

Honestly if y'all are in agreement that I make bad decisions as a maintainer I'm fine stopping all together and just keeping my own private copy of news that works just for me. Drive-by negativity like this is actively killing open source, good job!

I guess many people are mixing up two things:

1) By design NC has the problem that apps can more or less break the whole installation, because they cannot pre-check requirements. That is not the fault of the maintainers here, while the consequences are (at least for some) monumental. In my case I cannot even revert to a working news-version, still getting errors after downgrade

2) A decision has been teaken by the maintainers that affects a lot of people, but not themselves. I guess one of the issues here is that not everyone is able to follow the decision, i.e. the rationale behind is not convincing. To me it seems obvious that the 32bit integer limitations are problematic and had to be solved. However, maybe like others it is not clear to me why that had to lead to this dramatic consequnece. If only the resolution of the time stamp is the issue, I have the feeling that this could have been solved otherwise.

@SMillerDev: It is not against you but the way you might have underestimated the consequences (as combination of 1) and 2)). Again, the first one is not your fault.

anoymouserver commented 3 years ago

@coelner:

Could you explain the background to this time resolution problem? Do you need a 64bit OS because of the features of the 64bit hardware extension, or because it is mandatory for the stack on which the news app runs or is it the database? If I get this right, you saved the time as string to omit the unixtimestamp?

The majority of the issue is already described here: #1320. PHP only has one integer type which is the same size as the underlaying CPU architecture (32 bit/64 bit), once the value gets bigger and overflows, the type switches to float as long as it's not explicitly casted (which results in an undefined behaviour). The database already uses an explicit bigint (int64). The app uses this precise timestamp (microseconds since epoch) to avoid race-conditions with multiple clients for e.g. updating. This has been the case since we took over the maintenance of the app, but now it's no longer a mix of int, string and float that goes through the backend.


And YES it is a breaking change, but it was handled as far as we could control it ... raising major version (as per SemVer) and marking the requirement in the NC metadata file. In my opinion the NC server should handle such cases if the metadata file allows it. https://github.com/nextcloud/news/blob/02cbdc9635a63c1789c7a747e67a3db8676abf2d/appinfo/info.xml#L42 And since we all have no 32-bit systems and also the used CI is 64-bit only, so no automatic testing.

joeheaven commented 3 years ago

Is it correct to assume if you go for 1/100th second time resolution and not milliseconds, the problem is gone?

bcutter commented 3 years ago

Of course no one wants that, every user of NC News likes/loves or even needs this app.

But that decision already put all 32 bit users on the siding, like the project would've been stopped/unmaintained. How long can we survive with latest 15 release? I bet already next Nextcloud major release update will break it (need for hacking appinfo to fake NC version compatibility) besides other things we can't oversee, maybe introduced by new NC server version.

What I don't like is: you state the killing of 32 bit support as one without any alternative. And as I see only in this issue few others with dev knowledge having some ideas makes me thinking/hoping of "Well, probably there is another solution too."

Fair enough. If you can make 100 % sure there is no other alternative at all to fix this timing or variable size thing which seems to be the root cause, fine. Until that point I simply don't like the current decision to only support 64 bit OS.

anoymouserver commented 3 years ago

Is it correct to assume if you go for 1/100th second time resolution and not milliseconds, the problem is gone?

Seconds overflow an int32 (max value 2 147 483 647) in Y2038. Every higher precision (milliseconds, microseconds, ..) already overflowed.


Until that point I simply don't like the current decision to only support 64 bit OS.

As already said, there is no way we could test this automatically or verify it without setting up a 32-bit server for each developer. And that's exactly why we cannot ensure that it still works at all. The users would then always have to find out with each new release and report whether it still works at all.

SMillerDev commented 3 years ago

If you can make 100 % sure there is no other alternative at all to fix this timing or variable size thing which seems to be the root cause, fine

There is no other alternative that I'll be writing so, unless someone else does, there isn't.

In terms of technology everything is possible, but someone still has to fix and maintain it.

shane-kerr commented 3 years ago

I guess the question is, would a PR that fixes this for 32-bit processors be considered or rejected out of hand? I realize that there's a support burden for any workaround, but hopefully a compromise can be found.

SMillerDev commented 3 years ago

It will be considered the same as all other PRs. Weighing maintenance burden against usefulness.

kesselb commented 3 years ago

https://github.com/nextcloud/news/blob/8a7428a8614d4658db7dde2a61b55d9ce9e6e55a/appinfo/info.xml#L42

min-int-size="64" => min-int-size="8".

https://github.com/nextcloud/server/blob/415d7049591d6b5c92f682450941373e1ae34872/lib/private/App/DependencyAnalyzer.php#L168-L173

SMillerDev commented 3 years ago

What does that mean @kesselb?

kesselb commented 3 years ago

@SMillerDev please ignore my comment ;)

coelner commented 3 years ago

The majority of the issue is already described here: #1320. PHP only has one integer type which is the same size as the underlaying CPU architecture (32 bit/64 bit), once the value gets bigger and overflows, the type switches to float as long as it's not explicitly casted (which results in an undefined behaviour). The database already uses an explicit bigint (int64). The app uses this precise timestamp (microseconds since epoch) to avoid race-conditions with multiple clients for e.g. updating. This has been the case since we took over the maintenance of the app, but now it's no longer a mix of int, string and float that goes through the backend.

To sum it up:

  1. php cannot cast float to integer properly, but it is a feature not a bug. (? https://bugs.php.net/bug.php?id=80101 )
  2. this app expands the last modified timestamp to an absolute unix milliseconds timestamp?
  3. You do not want to change those integers to the datetime class?
  4. there is a reason that paddedLastModified contains this 'blown absolute timestamp' instead of just the padding to the LastModified variable?

I try to understand your solution.

ferderm commented 3 years ago

Does anyone know how to export the feeds after the automatic update to 16.0.0 via command line? Or do I need to downgrade the way @bcutter described to export via browser?

SMillerDev commented 3 years ago

this app expands the last modified timestamp to an absolute unix milliseconds timestamp?

We use a timestamp in miliseconds to represent last modified times, yes.

You do not want to change those integers to the datetime class?

Databases don't provide the data in classes so we'd still have this issue somewhere.

there is a reason that paddedLastModified contains this 'blown absolute timestamp' instead of just the padding to the LastModified variable?

I'm not sure what paddedLastModified is, but splitting data that represents a single thing over multiple data fields makes code harder to understand and therefore harder to maintain. I don't think that's really a solution to the initial problem "we don't have a good way to use precise timestamps in PHP on 32-bit OSes"

coelner commented 3 years ago

paddedLastModified: https://github.com/nextcloud/news/blob/f7a5b8e6907abe21b37bb0b88c233c3b1780b415/lib/Controller/ItemApiController.php#L96-L99

As I do not understand php, you merge the lastModified value with a microseconds multiplier if the value is smaller then 10. My intention would be, that both values get transmitted, the lastModified and the paddedLastModfied. But where the paddedLastModfied just contains the difference to the LastModified timestamp in milliseconds.

I see the problem that php does not know any long integer. This leads to the casting problem from float to int to uint64 DB format. And there is a gap by converting those datetime php format to the datetime mysql format (https://dev.mysql.com/doc/refman/5.7/en/fractional-seconds.html) ?

anoymouserver commented 3 years ago

https://github.com/nextcloud/news/blob/f7a5b8e6907abe21b37bb0b88c233c3b1780b415/lib/Controller/ItemApiController.php#L96

This is for compatibility with older clients which only provied an unixtimestamp (with seconds)

shane-kerr commented 3 years ago

I think we can probably get something that works by using a floating point value for timestamps, treating it as a bigger integer. The idea would be to continue to use the Unix epoch timestamp converted to microseconds as the code has now, just to store it in a float instead of an int.

Apparently PHP uses a IEEE 754 double precision format for floating point numbers:

https://www.php.net/manual/en/language.types.float.php

A floating point double can store integers up to 254 without losing precision:

https://stackoverflow.com/a/12445693/814127

Multiplying the timestamp by 1 million consumes 20 bits of space:

$ python3
>>> import math
>>> math.log2(1000000)
19.931568569324174

This means that we can store a epoch value up to 54-20 = 34 bits, which is enough for the next few hundred years.

h4b4n3r0 commented 3 years ago

As a user: Okay, I just clicked update in nextcloud web ui like with every other app, which always worked flawlessly and now I'm stuck, because the web ui does not offer me to rollback nor deactivate that plugin. I simply can not use nextcloud, because requirements were not checked before update. Upgrading the WHOLE server to 64bit is no option.

I could disable the news app like this from commandline:

sudo -u www-data php ./occ app:disable news

Yes that´s a quick fix to get rest of Nextcloud back working. BUT: during a (I´m quite sure: EVERY) usual Nextcloud version update (like I did some minutes ago from 21.0.2.1 to 21.0.3.1) we´ll run into this issue. Because "occ update" is performed during an update and here NC News app got updated once again.

So I performed manual app downgrade twice today. I´m prepared to do this on every future NC version update...

That´s my whole downgrade process, maybe someone else can adjust (important, every environment is a different) and use this.

sudo -u www-data php /var/www/nextcloud/occ app:disable news
sudo -u www-data php /var/www/nextcloud/occ maintenance:mode --on
cd /home/user/downloads
wget https://github.com/nextcloud/news/releases/download/15.4.5/news.tar.gz
tar -xvzf news.tar.gz
sudo mv /var/www/nextcloud/apps/news /var/www/nextcloud/apps/news_broken
sudo mv /home/user/news /var/www/nextcloud/apps/
sudo ll /var/www/nextcloud/apps/ | grep news
sudo chown www-data:www-data -R /var/www/nextcloud/apps/news
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/AUTHORS.md
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/CHANGELOG.md
sudo chmod u-x,g-x,o-x /var/www/nextcloud/apps/news/COPYING
sudo ll /var/www/nextcloud/apps/ | grep news
sudo ll /var/www/nextcloud/apps/news
sudo -u www-data php /var/www/nextcloud/occ app:list | grep news
sudo -u www-data php /var/www/nextcloud/occ maintenance:mode --off
sudo -u www-data php /var/www/nextcloud/occ upgrade
sudo -u www-data php /var/www/nextcloud/occ app:enable news
sudo -u www-data php /var/www/nextcloud/occ upgrade
[check NC News is working on the browser]
sudo rm -R /var/www/nextcloud/apps/news_broken
sudo ll /var/www/nextcloud/apps/ | grep news
  1. Does this maintain my current feeds, or can I export them anyhow before doing the rollback?

  2. Even if I was surprised by the dependency requirements, I want to thank you @SMillerDev and everybody who does contribute as well. News is my most used App and I really like it! So thank you for all your energy and power! 👍 ❤️

  3. Since there is a workaround to roll back I can deal with it. And I hope many of the complainers as well. Thanks you @bcutter for the commands!

bcutter commented 3 years ago

@Id3aFly

  1. As long as the database hasn't been touched (which happens when 'occ upgrade' is successfully run - which won't happen because of the not fulfilled dependencies, so at least before this final step Nextcloud has some kind of "safety mechanism") all your feeds and elements and structure and so on is "safe". At least that's what my experience is, going through this 3 times meanwhile. So app and database tables are separated which allows to downgrade the app while database is untouched. Otherwise a roll-back of the database would be a lot more work and needs a fresh database backup. Your second question: I'm pretty sure looking at the right column of the database table where feeds are stored would quite easily allow to "export" them - directly from the database. I know no other way without a working News app. But as stated there's no need for that operation. Just in case... regularly backup your database ;-)

  2. Copy that, fully agree.

  3. Yes but as already mentioned downgrading only buys us some time, for the moment. In a few days, weeks or maybe months something won't work and will probably be fixed in a new series 16 release. Et voila: dead end.

Fingers crossed the dev experts talking on eye level here can figure out a way to not loose all 32 bit users. Hope dies last ;-)

usrjense commented 3 years ago

Is it correct to assume if you go for 1/100th second time resolution and not milliseconds, the problem is gone?

Seconds overflow an int32 (max value 2 147 483 647) in Y2038. Every higher precision (milliseconds, microseconds, ..) already overflowed.

Until that point I simply don't like the current decision to only support 64 bit OS.

As already said, there is no way we could test this automatically or verify it without setting up a 32-bit server for each developer. And that's exactly why we cannot ensure that it still works at all. The users would then always have to find out with each new release and report whether it still works at all.

The 32 / 64 bit discussion will end at least in 17 years? https://github.com/nextcloud/server/issues/27765

roteiro commented 3 years ago

I had to revert to 15.4.5 as well. Is it enough to manually set the 'installed_version' of news in the 'appconfig' table to '99.9.9' to prevent automatic/accidental updates of the app or is there a better way?

Just to add a quick feedback: Although I have 64bit-capable hardware (RPi 4) I use the very solid Nextcloudpi images. At the moment they are 32 bit only. Switching to 64bit would mean that I have to reinstall and administrate the server all by myself, which is not really an option for me. I use news on a daily basis and I'm very thankful for this great app and of course maintainers can do what they think is best, especially considering the fact that they develop the app in their free time. Still I hope the decision to block out all Nextcloudpi and other 32bit users will be reevaluated. I have the feeling that the impact of this decision was quite huge. Did I understand correctly that #1337 would solve the problem for now but is not applied because it might complicate maintenance of the code in the future? Why not applying it now and go for the 64bit solution when problems actually occur? This would at least buy some time for 32bit users.

SMillerDev commented 3 years ago

Why not applying it now and go for the 64bit solution when problems actually occur? This would at least buy some time for 32bit users.

Because cleaner code means less maintenance and means you might actually get updates when bugs occur because it isn't a 8 hour task to understand the code.

And if the end of life for python 2 has taught the open source community anything it should be this: delaying hard support choices makes them worse, not better.

shane-kerr commented 3 years ago

Would a solution using only float be accepted? That is, rather than using microseconds, use actual seconds? The code would actually be simpler, since the existing special-case handling for seconds could be removed.

The complication would be that it would require a schema change (converting INT to DOUBLE), as well as changing the currently stored microseconds to seconds. This is not a super complicated schema change though, and could be handled safely with any transactional database.

idovitz commented 3 years ago

This news application does not have nextcloud just as a underlying library, but as a whole framework. If the nextcloud project decided to only support 64-bit, this choice was completely understandable. Since Nextcloud does not require 64bit, it makes sense to take this as the starting point for this featured app. Thanks all for your work on this app, hope i can use it again on my rpi4.