Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump Nextcloud to v26.0.7 #2303

Closed
binarykitchen opened this issue Sep 17, 2023 · 16 comments
Closed

Bump Nextcloud to v26.0.7 #2303

binarykitchen opened this issue Sep 17, 2023 · 16 comments

Comments

@binarykitchen
Copy link
Contributor

binarykitchen commented Sep 17, 2023

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:

  • Better UI regarding background images and icons
  • More authentication options
  • They dropped support for PHP v7.4 but since we're already on PHP v8.0 since version 60, it's probably fine.
  • The API for background jobs has been refactored and needs careful testing
  • Likewise for address books
  • Tons of bugfixes

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: nextcloud/user_external#222

When testing, ensure the following:

  • In a docker image, ensure all existing data on the cloud is still available, such as photos, events and contacts
  • There are no new errors under /var/log/
  • Resolve any new warnings outlined under settings/admin/overview

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

@yodax
Copy link
Contributor

yodax commented Sep 17, 2023

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
Copy link
Contributor

yodax commented Sep 17, 2023

Here’s the link to the PR where I added the PPA before: #1140

@kiekerjan
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

matidau commented Sep 18, 2023

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
Copy link
Contributor Author

I've moved out the suggested PHP works to a separate ticket. This to keep this one pure about Nextcloud alone:
#2307

@kiekerjan
Copy link
Contributor

kiekerjan commented Oct 1, 2023

Here's a pull request upgrading php from 8.0 to 8.1: #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
Copy link
Contributor Author

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
Copy link
Contributor

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 binarykitchen changed the title Bump Nextcloud to v26.0.6 Bump Nextcloud to v26.0.7 Oct 8, 2023
@binarykitchen
Copy link
Contributor Author

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

@TheRedCyclops
Copy link

close?

@binarykitchen
Copy link
Contributor Author

Yep, shipped a long time ago :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants