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

Proposed fix for #773 #780

Merged
merged 1 commit into from
Jul 22, 2014
Merged

Proposed fix for #773 #780

merged 1 commit into from
Jul 22, 2014

Conversation

anno1337
Copy link
Contributor

Split up check_setup.php into two files. The new file check_essentials.php takes care of stuff like the PHP version and is executed before the config files are included which are needed by check_setup. This patch addresses issue #773

…s.php takes care of stuff like the PHP version and is executed before the config files are included which are needed by check_setup. This patch addresses issue #773
// Check if /cache is writeable
if (! is_writable('cache')) {
die('The directory "cache" must be writeable by your web server user');
}

// Check if /db is writeable
if (! is_writable('db')) {
if (! is_writable('db') && STORAGE === 'sqlite') {
Copy link
Member

Choose a reason for hiding this comment

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

Just to be sure, if we choose mysql or postgres on install screen, no worry with this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I've tested it with mysql and sqlite. Did behave as it should. The only time this may be problematic is if another file based database would be added. Then this would need to be extended by another statement or converted into a regex.

Copy link
Member

Choose a reason for hiding this comment

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

Great. I'll try your PR and merge it.

@nicosomb nicosomb merged commit 9c67b1b into wallabag:dev Jul 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants