search

YunoHost-Apps / mautrix_signal_ynh

Matrix signal package for YunoHost
GNU Affero General Public License v3.0
4 stars 10 forks source link

Fix logfile permission #102

Closed CodeShakingSheep closed 4 weeks ago

CodeShakingSheep commented 1 month ago

Fix permission for logfile (chmod & chown)

Problem

After upgrading I had this error May 26 18:11:04 mautrix-signal[2284450]: Failed to initialize logger: failed to parse config for writer #2: can't open new logfile: open /var/log/mautrix_signal/mautrix_signal.log: permission denied which I could fix manually. See https://github.com/YunoHost-Apps/mautrix_signal_ynh/issues/101 .

Solution

Added 2 chmod commands and one chown command as in WhatsApp bridge upgrade script. See https://github.com/YunoHost-Apps/mautrix_whatsapp_ynh/blob/master/scripts/upgrade#L274

PR Status

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

CodeShakingSheep commented 1 month ago

!testme

yunohost-bot commented 1 month ago

:v: Test Badge

yunohost-bot commented 1 month ago

:bug: Test Badge

nathanael-h commented 1 month ago

This PR looks good to me, I let you merge @MayeulC as you self assigned it :) Thanks also @CodeShakingSheep.

MayeulC commented 4 weeks ago

Yes, looks good to me too, sorry for taking so long to review it.

I wasn't entirely sure about applying both chmods instead of doing a u+rwX go-rwx or chmod -R /var/log/$app/* but this is fine too, and it's better to keep close to other mautrix bridges :)

If you could just use a more descriptive commit message next time, something like "Fix log file permissions", that would be perfect :)

CodeShakingSheep commented 3 weeks ago

Yes, looks good to me too, sorry for taking so long to review it.

I wasn't entirely sure about applying both chmods instead of doing a u+rwX go-rwx or chmod -R /var/log/$app/* but this is fine too, and it's better to keep close to other mautrix bridges :)

If you could just use a more descriptive commit message next time, something like "Fix log file permissions", that would be perfect :)

Thanks for merging @MayeulC . I added a description to the commit message. You have to click the 3 little points to open it. See image

Is that good with you for the next time or do you prefer to have the description directly in the commit message?