Don't enable market app on upgrade from 9.1.x or lower when app store… #28711
Conversation
DeepDiver1975
added
3 - To Review
backport-request
p1-urgent
lib/private/Repair/Apps.php
Outdated
There was a problem hiding this comment.
Will this code rerun during the next upgrade or only ever when upgrading from any OC < 10.0 version ?
Because I remember that we removed the "appstoreenabled" key after upgrading to 10.0.
There was a problem hiding this comment.
this will trigger this step only for upgrade from oc <10.0 - https://github.com/owncloud/core/blob/6a545053e763a5846cc887a0f66ff15ead7e4f69/lib/private/Repair/Apps.php#L91-L96
is this what you are looking for?
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
👍 looks good otherwise, if the answer to my comment is correct
lib/private/Repair/Apps.php
Outdated
There was a problem hiding this comment.
but this check is used here too ?
There was a problem hiding this comment.
if you leave it then it will be possible to continue using the obsolete "appstoreenabled" config to prevent loading the market app, even if enabled already
There was a problem hiding this comment.
coming from 9.1 to 10.0.x which ships market app will auto enable the market.
so it is necessary to not load the market app and trigger the update scenarios
There was a problem hiding this comment.
my worry is that this part of the code also runs when updating 10.x to 10.x+1 and might have side effects if someone has "appstoreenable => false" in their config.php despite the fact that we explicitly delete it later. (no $requiresMarketEnable check for this piece).
Unless I misunderstood and this repair step only ever runs once ?
There was a problem hiding this comment.
I guess I'm getting your point - shall we explicitly disable the market on the initial migration from 9.x.x to 10.x.x ?
DeepDiver1975
force-pushed
the
respect-appstoreenabled-when-enabling-marketapp
branch
from
6a54505 to
5a0d1ba
Compare
|
also looks like I didn't backport that one... |
DeepDiver1975
force-pushed
the
respect-appstoreenabled-when-enabling-marketapp
branch
from
5a0d1ba to
c52b513
Compare
lib/private/Repair/Apps.php
Outdated
There was a problem hiding this comment.
@deepdiver hmm, so if people keep "appstoreenabled" to false in config.php, every update will re-disable the market app ?
maybe we should delete the key after all?
unless you make sure that this block only every runs when upgrading from 9.x
then never again
There was a problem hiding this comment.
let me try something ....
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
Approach looks good 👍
Please fix the wording and optionally the minor issue about doc link
lib/private/Repair/Apps.php
Outdated
There was a problem hiding this comment.
hard-coded 10.0 ? I think we have a router method somewhere for pointing at docs ?
Minor issue, feel free to ignore
lib/private/Repair/Apps.php
Outdated
… was disabled in the past - fixes owncloud/market#118 Disable market app if 'appstoreenabled' was set to false in config Handle 'has_internet_connection' and cleanup of market app handling
DeepDiver1975
force-pushed
the
respect-appstoreenabled-when-enabling-marketapp
branch
from
e214e91 to
4fcbdc7
Compare
DeepDiver1975
deleted the
respect-appstoreenabled-when-enabling-marketapp
branch
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
… was disabled in the past
Description
Prio to 10.0 appstoreenable was used as config option to disable installation from apps.owncloud.com
In migration scenarios we respect this value now and do not enable the market app-
Related Issue
fixes owncloud/market#118
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: