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

Add more information why copy could fail during backup #179

Merged
merged 1 commit into from
Aug 31, 2018
Merged

Add more information why copy could fail during backup #179

merged 1 commit into from
Aug 31, 2018

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Aug 11, 2018

As described in nextcloud/server#10627 backup during update could fail when file permission or owner is not sufficient. Because a exception is thrown anyway some more information could given to make error finding easier.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@MorrisJobke
Copy link
Member

@danielkesselberg Thanks for the contribution. Is it possible that you also add this code to lib/Updater.php. Usually there is also a check in place to fail the CI job once those two files are not in sync.

Could I maybe also ask you to have a look at the other open PRs in this repo and check them out? I'm a little bit overwhelmed with work right now and it would be awesome to have a helping hand in this crucial part of Nextcloud. 👍 Thanks in advance.

Beside that the change looks really nice 👍

@kesselb
Copy link
Contributor Author

kesselb commented Aug 23, 2018

@MorrisJobke I've added changes to lib/Updater.php as well and build is green now.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

👍 Makes total sense

@MorrisJobke MorrisJobke merged commit a2ab9c5 into nextcloud:master Aug 31, 2018
@MorrisJobke
Copy link
Member

Thanks a lot for the contribution.

@MorrisJobke
Copy link
Member

I updated the phar with that: 6c8cdae

@MorrisJobke
Copy link
Member

And I back ported it to stable13: a578827 & acaa42a

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

Successfully merging this pull request may close these issues.

None yet

2 participants