Skip to content

Conversation

@ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Jan 19, 2023

Fix nextcloud/related_resources#170
Close #18673

Should fix nextcloud/all-in-one#1787 (comment)

There is an edge use case where custom apps folder seems ignored during server upgrade.

When upgrading the server:

  • version of apps included with server (bundle) are compared with appstore,
  • in case of new versions, last available version is downloaded,
  • if custom_apps is used, the downloaded app will be installed in the writable app folder (and not the default app folder),
  • unfortunately, upgrade of apps installed in custom_apps are not triggered during this process,
  • upgrade finalize and an admin will need to start a second upgrading process to trigger the upgrade of apps downloaded in custom_apps.

While it seems a minor issue because it only require to initiate a second upgrading process, it locks AIO Nextcloud because the upgrade cannot be initiated from the webclient (but only by running ./occ upgrade)

This PR will trigger the upgrading process of the apps installed in custom apps during the upgrading process of the server.

@szaimen szaimen added this to the Nextcloud 26 milestone Jan 19, 2023
@szaimen szaimen added the 2. developing Work in progress label Jan 19, 2023
@ArtificialOwl ArtificialOwl added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 19, 2023
@ArtificialOwl ArtificialOwl requested review from a team, blizzz, come-nc and icewind1991 and removed request for a team January 19, 2023 20:56
@ArtificialOwl
Copy link
Member Author

/backport to stable25

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to fix the issue in my testing. Thanks a lot for fixing this @ArtificialOwl! :)

However I am not sure if this has some negative performance implications...

@blizzz
Copy link
Member

blizzz commented Jan 20, 2023

I would not say it is the final fix, for it may have performance impact (not 100% sure, but otherwise the static var would not make sense). Alternatively it could be an override parameter, for instance, that would avoid the runtime cache in update scenarios.

@ArtificialOwl
Copy link
Member Author

I would not say it is the final fix, for it may have performance impact (not 100% sure, but otherwise the static var would not make sense). Alternatively it could be an override parameter, for instance, that would avoid the runtime cache in update scenarios.

please check last commit; it should ignores cached data only during install/upgrade

@szaimen szaimen force-pushed the fix/noid/single-upgrade-on-custom-apps branch from fc7eac7 to e52f931 Compare January 27, 2023 12:59
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as well in my testing (but cannot judge the code). Thanks Maxence! :)

@szaimen szaimen self-requested a review January 27, 2023 13:03
@szaimen szaimen requested a review from nickvergessen January 27, 2023 13:04
@ArtificialOwl ArtificialOwl force-pushed the fix/noid/single-upgrade-on-custom-apps branch from e52f931 to 4cac49c Compare February 1, 2023 12:08
@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish bug and removed 3. to review Waiting for reviews labels Feb 1, 2023
@szaimen
Copy link
Contributor

szaimen commented Feb 1, 2023

CI failure unrelated

@szaimen szaimen merged commit 41148ac into master Feb 1, 2023
@szaimen szaimen deleted the fix/noid/single-upgrade-on-custom-apps branch February 1, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recent upgrade broke Nextcloud Reset app dir on upgrade with non-writable apps paths.

4 participants