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

[stable11] Prevent migration from ownCloud 10 to Nextcloud 11 #3415

Merged
merged 4 commits into from Mar 30, 2017

Conversation

Projects
None yet
4 participants
@nickvergessen
Member

nickvergessen commented Feb 9, 2017

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Feb 9, 2017

@nickvergessen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @blizzz, @LukasReschke and @icewind1991 to be potential reviewers.

mention-bot commented Feb 9, 2017

@nickvergessen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @blizzz, @LukasReschke and @icewind1991 to be potential reviewers.

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Feb 10, 2017

Member

I also put #3424 in here.

Member

MorrisJobke commented Feb 10, 2017

I also put #3424 in here.

$version = explode('.', $oldVersion);
$majorMinor = $version[0] . '.' . $version[1];
$currentVendor = $this->config->getAppValue('core', 'vendor', '');

This comment has been minimized.

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

This one was introduced with Nextcloud 11. Basically we would forbid to upgrade from nextcloud 10 to 11 🙈

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

This one was introduced with Nextcloud 11. Basically we would forbid to upgrade from nextcloud 10 to 11 🙈

This comment has been minimized.

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

Workaround: bring #3425 and the stuff during the upgrade also down to stable10 and write a lot of release notes how to manually add this ... otherwise we will get trouble. Or we try to get first this ported to stable10 and delay this PR for 11.0.3 to reduce the likeliness of this to happen.

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

Workaround: bring #3425 and the stuff during the upgrade also down to stable10 and write a lot of release notes how to manually add this ... otherwise we will get trouble. Or we try to get first this ported to stable10 and delay this PR for 11.0.3 to reduce the likeliness of this to happen.

This comment has been minimized.

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

I was wrong:

$this->config->setAppValue('core', 'vendor', $this->getVendor());

The vendor is set during upgrade. Still leaves us with the install scenario, but I will backport #3425 to stable10

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

I was wrong:

$this->config->setAppValue('core', 'vendor', $this->getVendor());

The vendor is set during upgrade. Still leaves us with the install scenario, but I will backport #3425 to stable10

This comment has been minimized.

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

It was added to 10.0.1: #1183

@MorrisJobke

MorrisJobke Feb 10, 2017

Member

It was added to 10.0.1: #1183

This comment has been minimized.

@nickvergessen

nickvergessen Feb 10, 2017

Member

I guess delaying this PR makes sense, we just need to make sure that this version is out, before oc releases their 10, or at least very close. So people don't update to oc10 and then migrate to our 11

@nickvergessen

nickvergessen Feb 10, 2017

Member

I guess delaying this PR makes sense, we just need to make sure that this version is out, before oc releases their 10, or at least very close. So people don't update to oc10 and then migrate to our 11

@MorrisJobke

MorrisJobke approved these changes Feb 10, 2017 edited

I fixed the return value of getAllowedPreviousVersion() and fixed the version number in the allow previous versions array to be able to upgrade from Nextcloud 10. 😉

Works now (together with #3427 for installs without and upgrade in between)

@nickvergessen

This comment has been minimized.

Show comment
Hide comment
@nickvergessen

nickvergessen Feb 10, 2017

Member

Braindump

  1. Install 11.0.1
  2. Try to update to this
  3. Vendor is not set yet, because no update ever happened
  4. return isset($allowedPreviousVersions[$currentVendor][$majorMinor]); will return vales because currentVendor === ''
  5. Update not possible? 😿

Possible fix

  1. Whitelist the 4 digit version numbers of packages we officially released (alpha, beta, rc, dailies?), since they do not have common versions with OC as far as I remember.
Member

nickvergessen commented Feb 10, 2017

Braindump

  1. Install 11.0.1
  2. Try to update to this
  3. Vendor is not set yet, because no update ever happened
  4. return isset($allowedPreviousVersions[$currentVendor][$majorMinor]); will return vales because currentVendor === ''
  5. Update not possible? 😿

Possible fix

  1. Whitelist the 4 digit version numbers of packages we officially released (alpha, beta, rc, dailies?), since they do not have common versions with OC as far as I remember.

@nickvergessen nickvergessen removed this from the Nextcloud 11.0.2 milestone Feb 10, 2017

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Feb 10, 2017

Member

Whitelist the 4 digit version numbers of packages we officially released (alpha, beta, rc, dailies?), since they do not have common versions with OC as far as I remember.

Could be a way to mitigate this. 👍

Member

MorrisJobke commented Feb 10, 2017

Whitelist the 4 digit version numbers of packages we officially released (alpha, beta, rc, dailies?), since they do not have common versions with OC as far as I remember.

Could be a way to mitigate this. 👍

@rullzer

This comment has been minimized.

Show comment
Hide comment
@rullzer

rullzer Mar 30, 2017

Member

@nickvergessen @MorrisJobke what is the plan here?

Member

rullzer commented Mar 30, 2017

@nickvergessen @MorrisJobke what is the plan here?

@nickvergessen

This comment has been minimized.

Show comment
Hide comment
@nickvergessen

nickvergessen Mar 30, 2017

Member

Let me finish this now

Member

nickvergessen commented Mar 30, 2017

Let me finish this now

nickvergessen and others added some commits Jan 20, 2017

Prevent migration from ownCloud 10 to Nextcloud 11
Signed-off-by: Joas Schilling <coding@schilljs.com>
Fix return value of getAllowedPreviousVersions()
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
fix version number
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Fix the update from NC 10 and 11 to the latest NC 11
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen

This comment has been minimized.

Show comment
Hide comment
@nickvergessen

nickvergessen Mar 30, 2017

Member

Fixed and rebased

Member

nickvergessen commented Mar 30, 2017

Fixed and rebased

@rullzer rullzer merged commit 00a023e into stable11 Mar 30, 2017

0 of 2 checks passed

continuous-integration/drone/pr the build failed
Details
continuous-integration/drone/push the build failed
Details

@rullzer rullzer deleted the backport-3184-migration-restriction-by-vendor-version branch Mar 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment