-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
do not fail update or installation when .htaccess file is not writable #34478
Conversation
@@ -472,11 +468,11 @@ public static function updateHtaccess() { | |||
|
|||
if ($content !== '') { | |||
$fileWriteResult = @\file_put_contents( | |||
$setupHelper->pathToHtaccess(), $htaccessContent . $content . "\n" | |||
self::pathToHtaccess(), $htaccessContent . $content . "\n" | |||
); | |||
if ($fileWriteResult === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - the "real" exception is still thrown here, if we did get here and still the htaccess file could not be written.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I will let @PVince81 give the final approval.
Codecov Report
@@ Coverage Diff @@
## master #34478 +/- ##
============================================
- Coverage 64.84% 64.83% -0.01%
- Complexity 18323 18325 +2
============================================
Files 1198 1198
Lines 69576 69575 -1
Branches 1283 1283
============================================
- Hits 45115 45112 -3
- Misses 24087 24089 +2
Partials 374 374
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #34478 +/- ##
============================================
- Coverage 64.84% 64.83% -0.01%
- Complexity 18323 18325 +2
============================================
Files 1198 1198
Lines 69576 69575 -1
Branches 1283 1283
============================================
- Hits 45115 45112 -3
- Misses 24087 24089 +2
Partials 374 374
Continue to review full report at Codecov.
|
tried hard to add more unit tests but the whole install/update code is hard to test |
@@ -425,11 +425,7 @@ public static function updateHtaccess() { | |||
$webRoot = !empty(\OC::$WEBROOT) ? \OC::$WEBROOT : '/'; | |||
} | |||
|
|||
$setupHelper = new \OC\Setup($config, \OC::$server->getIniWrapper(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the class exists in this form due to unit testing.
I'm unsure whether we should remove it like this but I don't mind if it doesn't break anything...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
setup works again on my env
@individual-it please backport |
Backport |
Description
since #32230 an exception is thrown when the .htaccess is read-only but it has to be possible to install or update oC when the .htaccess file is not writeable
but when
occ maintenance:update:htaccess
and the file cannot be updated the command should failRelated Issue
Motivation and Context
see issue
How Has This Been Tested?
.htaccess
.htaccess
.htaccess
occ maintenance:update:htaccess
with R/O.htaccess
occ maintenance:update:htaccess
with R/W.htaccess
Types of changes
Checklist:
Open tasks: