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

Restore Roundcube's password reset tool by removing PRAGMA journal_mode = WAL from Roundcube source #2199

Merged
merged 3 commits into from
May 13, 2023

Conversation

kiekerjan
Copy link
Contributor

This pull request disables the use of PRAGMA journal_mode=WAL from the Roundcube source. Thus avoiding the issue that postfix cannot handling a sqlite db with that pragma set.

@jvolkenant
Copy link
Contributor

Is this the fix that would make the password change plugin function again?

@kiekerjan
Copy link
Contributor Author

kiekerjan commented Jan 29, 2023

You also need #2194 to correctly configure roundcube.
[edit]: also, undo this #2198

# Because Roundcube wants to set the PRAGMA we just deleted from the source, we apply it here
# to the roundcube database (see https://github.com/roundcube/roundcubemail/issues/8035)
# Database should exist, created by migration script
sqlite3 $STORAGE_ROOT/mail/roundcube/roundcube.sqlite 'PRAGMA journal_mode=WAL;'
Copy link
Contributor

Choose a reason for hiding this comment

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

Might this need to be set to DELETE or TRUNCATE instead if we are to not use WAL anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. This specific database is owned by roundcube and not used by postfix. Because roundcube would like to use WAL, and we just removed it from the source, we set it here explicitly.

A possible addition is setting the DELETE pragma for the MiaB database users.sqlite. Just to be sure it is set to a value that postfix understands.

Copy link
Member

Choose a reason for hiding this comment

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

I think this may break new installations. The roundcube.sqlite file isn't created by the migration script. It checks if the file exists before mangling it (and only does this once in migration_12).

This also emits "wal" to the console so it would be nice to silence it with e.g. > /dev/null (if it's to STDOUT and not STDERR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Line 207 states "Run Roundcube database migration script (database is created if it does not exist)", so I kinda depend on that. There's also the chmod and chown commands that are applied to the database. Perhaps the comment about migration should mention the updatedb script? I'll take a closer look.

I will also look into silencing the command.

Copy link
Member

Choose a reason for hiding this comment

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

Ahha! I'm sorry. I thought you meant the Mail-in-a-Box migration system (migrate.sh). I didn't look a few lines above. Yes ok then we are all good.

@kiekerjan
Copy link
Contributor Author

"@kiekerjan force-pushed the roundcubesqlitemod branch from 61f9ad5 to 86933d2 1 minute ago"
So I did something silly (merged my master into this branch), but hopefully it is fixed now. However, perhaps it is better if this branch is not merged directly, but a new branch created with these changes? I don't know enough about git, there might be some history of these actions remaining.

@JoshData JoshData changed the title Remove journal PRAGMA from Roundcube source Restore Roundcube's password reset tool by removing PRAGMA journal_mode = WAL from Roundcube source May 13, 2023
@JoshData
Copy link
Member

Thanks! I force-pushed, also, to consolidate all of the changes needed to restore the password plugin functionality.

But I am not sure if it's quite right - I left a comment on the code. Hopefully we can resolve this quickly so I can include it in a MiaB release.

@JoshData JoshData merged commit fb0a3b0 into mail-in-a-box:main May 13, 2023
downtownallday pushed a commit to downtownallday/mailinabox-ldap that referenced this pull request May 13, 2023
…ode = WAL` from Roundcube source (mail-in-a-box#2199)

(cherry picked from commit fb0a3b0)
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.

3 participants