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

Revert "Remove repair steps for broken updater repair" #25343

Merged
merged 1 commit into from
Jul 6, 2016

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Jul 4, 2016

This reverts commit 58ed5b9.

Reverts #24438

Fixes owncloud/updater#358

The problem is as follows: whenever that PR was merged, it was assumed that the updater will provide the update path 9.0.x->9.0.2->9.0.3 in which case the repair would happen in 9.0.2.

However, due to another bug that was found later on related to encryption, we can't provide a smooth update path through 9.0.2 for people with encryption.

So this PR reverts the fix to make sure that updates like 9.0.0->9.0.4, 9.0.1->9.0.4 will be possible, skipping 9.0.2.

Now... need to double check whether we should even allow skipping 9.0.2 in the first place.

  • double check whether skipping 9.0.2 could cause other troubles
  • ~add a version check in the repair step (when coming from OC <= 9.0.1) to avoid unneeded operations~~

@VicDeo @DeepDiver1975

@mention-bot
Copy link

@PVince81, thanks for your PR! By analyzing the annotation information on this pull request, we identified @DeepDiver1975, @nickvergessen and @icewind1991 to be potential reviewers

@PVince81
Copy link
Contributor Author

PVince81 commented Jul 4, 2016

Note: I tested this PR locally by patching 9.0.3 and putting it in a local update server and the integrity check issues did disappear after updating.

@davitol
Copy link
Contributor

davitol commented Jul 5, 2016

@PVince81 I've tried to update via webUI but applying the patch it seems that the open updater button is no longer useful

screen shot 2016-07-05 at 11 12 52

Did you update via CLI or via webUI?

@PVince81
Copy link
Contributor Author

PVince81 commented Jul 5, 2016

@davitol I tested with the web update. Maybe something went wrong with your patching and broke the button ?

@VicDeo
Copy link
Member

VicDeo commented Jul 5, 2016

@PVince81 version is already checked indirectly by comparing /composer/autoload_classmap.php hash

// Check the hash for the autoload_classmap.php file, if the hash does match
// the expected value then the third-party folder has already been copied
// properly.
if(hash_file('sha512', $thirdPartyDir . '/composer/autoload_classmap.php') === 'abe09be19b6d427283cbfa7c4156d2c342cd9368d7d0564828a00ae02c435b642e7092cef444f94635f370dbe507eb6b2aa05109b32d8fb5d8a65c3a5a1c658f') {
Copy link
Member

Choose a reason for hiding this comment

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

@pvince this is a version check

@PVince81
Copy link
Contributor Author

PVince81 commented Jul 6, 2016

@VicDeo so... do you think we can merge this PR ?

@davitol
Copy link
Contributor

davitol commented Jul 6, 2016

@VicDeo @PVince81 It did not work for me
screen shot 2016-07-06 at 16 11 13
I tried to update via daily channel from oC 9.0.3 version via daily
For this update i set an update server (you can as me for the credentials via IRC) where I settled a stable9 oC patched file with this PR and in my oC server i changed the following data:

  • delete from oc_appconfig where configkey='lastupdatedat';
  • edit /opt/owncloud/updater/src/Utils/Fetcher.php file and set const DEFAULT_BASE_URL = 'myupdateserver';
  • edit /opt/owncloud/version.php and set daily as channel
  • edit config.php file and set the value of "update.server.url" to the location of the deployed update server.

I'm available in the oC IRC as davitol if it is needed more info.

@davitol
Copy link
Contributor

davitol commented Jul 6, 2016

@VicDeo @PVince81 It finally worked for me. Thanks @VicDeo for the info via IRC

👍

@PVince81 PVince81 merged commit 116a6f3 into stable9 Jul 6, 2016
@PVince81 PVince81 deleted the stable9-revertrepairrevert branch July 6, 2016 15:49
@PVince81
Copy link
Contributor Author

PVince81 commented Jul 6, 2016

Note: no master PR because we still intend to enforce the upgrade path 9.0.x->9.0.4 before providing 9.0.4->9.1

@VicDeo
Copy link
Member

VicDeo commented Jul 7, 2016

@PVince81 Here are the results with the fresh stable9-daily

Encryption was enabled for all tests:
9.0.0 -> stable9-daily

  • 3rdparty updated
  • integrity check passed

9.0.1 -> stable9-daily

  • 3rdparty updated
  • integrity check passed

9.0.2 -> stable9-daily

  • 3rdparty updated
  • integrity check passed

9.0.3 -> stable9-daily

  • 3rdparty updated
  • integrity check passed

P.S. Known issue for 9.0.0 and 9.0.1:
Logging is done updater/update.log instead of data/update.log
It has no effect on integrity check though.

@PVince81
Copy link
Contributor Author

PVince81 commented Jul 7, 2016

@VicDeo great! Thanks for testing!

CC @owncloud/qa

@lock
Copy link

lock bot commented Aug 5, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants