mail-in-a-box / mailinabox

Mail-in-a-Box helps individuals take back control of their email by defining a one-click, easy-to-deploy SMTP+everything else server: a mail server in a box.
https://mailinabox.email/
Creative Commons Zero v1.0 Universal
13.98k stars 1.44k forks source link

Bump Nextcloud to v26.0.7 #2303

Closed binarykitchen closed 3 months ago

binarykitchen commented 1 year ago

Because the Nextcloud development is rapid (or too rapid?), v26 and 27 are already out and v28 is around the corner

In a summary, v26 doesn't come with many visual changes but many backend refactorings:

To be safe, let's not rush too quickly and bump slowly one version after each in separate PRs for best security and test all well. PRs for v27 and v28 can come afterwards.

All the breaking changes for v26 are mentioned in: https://docs.nextcloud.com/server/latest/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_26.html

Also, within the same PR, bump Nextcloud's user-external, calendar and contact apps to their latest version. user-external is ready, they fixed the bug for v26 recently: https://github.com/nextcloud/user_external/issues/222

When testing, ensure the following:

Feedback welcome before I'll do the PR in the next days :)

yodax commented 1 year ago

Version 27 is now on .2. It is stable on my MIAB. I’d recommend going directly to that version to prevent the double effort of validating two versions.

Version 28 will require us to go to a different PPA again for a higher PHP version. All the required steps for that are in a the repo from before the switch to Ubuntu 22. I’d hold out on that as long as possible since it wasn’t a pleasant change last time I did that.

yodax commented 1 year ago

Here’s the link to the PR where I added the PPA before: https://github.com/mail-in-a-box/mailinabox/pull/1140

kiekerjan commented 1 year ago

I have been working on a similar PR, but combined it with the upgrade to php 8.1 , see here https://github.com/kiekerjan/mailinabox/tree/nextcloud26 It has been tested for installation, but I did not yet have the time to test some different upgrade paths. I combined both updates, because I don't see a nice clean way to just introduce php 8.1 also taking into account upgrades from the older mail in a box installations. Consider this a source of inspiration 😉

binarykitchen commented 1 year ago

@kiekerjan Thanks for the inspiration. I like the idea of putting in the PHP version into a variable. May I ask why the PHP v8.1 bump when Nextcloud v26 runs fine on PHP v8.0? I'd do them in separate PRs.

binarykitchen commented 1 year ago

@yodax Which one PPA? This one ppa:ondrej/php? Why do you recommend adding this? And shouldn't all the PHP work go into a separate PR? This is solely about Nextcloud.

binarykitchen commented 1 year ago

@yodax also, the bump to Nextcloud v27 will deprecate PHP v8.0, therefore it's not that simple and requires bumping PHP beforehand. That's why I prefer doing all that in smaller steps and PRs.

Furthermore, Nextcloud v27 requires nginx changes.

kiekerjan commented 1 year ago

I like the idea of putting in the PHP version into a variable.

I stoleborrowed that from PowerMIAB 😉

May I ask why the PHP v8.1 bump when Nextcloud v26 runs fine on PHP v8.0? I'd do them in separate PRs.

In my mind the upgrade from PHP 8 to 8.1 had to be done during a Nextcloud upgrade. But when I try to explain it, I realize that it actually might not be necessary. I will rebase my branch on your Nextcloud v26 PR and try to come up with something nice.

binarykitchen commented 1 year ago

Alright @kiekerjan, there won't be any PHP upgrade in my upcoming PR for Nextcloud v26.

But I really like the idea of putting the PHP version into a variable. We will need this sooner or later, especially for Nextcloud v27 with different PHP requirements.

@kiekerjan would you mind submitting a second PR solely for this, for having the PHP version in one variable?

matidau commented 1 year ago

Do we need to replace the existing variable in functions.sh?

PHP_VER=8.0

https://github.com/mail-in-a-box/mailinabox/blob/e419b620347e7d3e4782466d540056a212d28ae1/setup/functions.sh#L7C4-L7C4

Just adding my 2 cents 😊

binarykitchen commented 1 year ago

I've moved out the suggested PHP works to a separate ticket. This to keep this one pure about Nextcloud alone: https://github.com/mail-in-a-box/mailinabox/issues/2307

kiekerjan commented 1 year ago

Here's a pull request upgrading php from 8.0 to 8.1: https://github.com/mail-in-a-box/mailinabox/pull/2309 I have not yet put in a great deal of testing effort into this, but I wanted to put it out there so people can look and review it, and perhaps avoid some double work. I would appreciate any input on this, as I don't have a lot of time to put into this right now. In time, I want to test several upgrade paths to ensure that this works as expected.

binarykitchen commented 1 year ago

Thanks, mate @kiekerjan

The way I see is that we have to be careful here. For MiaB stability and robustness is the number one priority.

So, only introduce changes which are really required. We can still live without PHP v8.1 for now. It's not a priority now I think. But yes, in due time, we will have to bump PHP, just not now.

That said, I'd like to suggest the following strategy:

  1. We have hard-coded the PHP version in too many places. My PR (2307) solves this technical debt under the hood without any functional changes, but will make any future PHP upgrades easier. Your fork inspired me, and I've copied + tested some good changes from you (thank you!)

  2. The PHP upgrade to v8.1 (your PR) isn't really required right now, nor for the next Nextcloud version v26

  3. I intend to do another PR to bump Nextcloud to v26 alone with all its apps.

  4. At that point, with less tech debts, we can bump PHP to v8.1 before the Nextcloud v27 upgrade (your PR maybe, when rebased)

  5. A little bit later, do the bump to Nextcloud v27

If we do these four PRs in isolated commits, all becomes easier to manage any bugs, to investigate problems and to rollback if needed. Thoughts?

kiekerjan commented 1 year ago

Sure, it's no problem to rebase once #2308 is merged. I'll try to review that one soon then. Indeed, the mailinabox quality is key. Therefore some time is needed to properly review and test this, I wanted to put it out there early, so we have that time.

binarykitchen commented 1 year ago

@kiekerjan @yodax @matidau PR is up for Nextcloud v26 - your feedback very welcome

TheRedCyclops commented 3 months ago

close?

binarykitchen commented 3 months ago

Yep, shipped a long time ago :)