[00:13:08]
<Yunohost Git/Infra notifications> [issues] wiggins-philip [commented](https://github.com/YunoHost/issues/issues/2404#issuecomment-2156916433) on [issue #2404](https://github.com/YunoHost/issues/issues/2404) Edge case in port availability check: Is there a workaround for this? Im running into the same problem.
[03:27:55]
<Yunohost Git/Infra notifications> [issues] tituspijean [commented](https://github.com/YunoHost/issues/issues/2404#issuecomment-2157128599) on [issue #2404](https://github.com/YunoHost/issues/issues/2404) Edge case in port availability check: Sure, apply the aforementioned fix over there: https://github.com/YunoHost/yunohost/blob/dd394e94dc2bc4582a2471f7fe90106...
[03:28:20]
<Yunohost Git/Infra notifications> [issues] tituspijean [commented](https://github.com/YunoHost/issues/issues/2404#issuecomment-2157128599) on [issue #2404](https://github.com/YunoHost/issues/issues/2404) Edge case in port availability check: Sure, apply the aforementioned fix over there: https://github.com/YunoHost/yunohost/blob/dd394e94dc2bc4582a2471f7fe90106...
[10:11:43]
<Yunohost Git/Infra notifications> [yunohost] Salamandar created new branch fix_goenv_again
[10:11:44]
<Yunohost Git/Infra notifications> [yunohost] Salamandar pushed 1 commit to fix_goenv_again: helpers/v1/go: fix call. goenv latest doesn’t call the plugin anymore, so i’m calling directly the plugin goenv-latest. ([f0727ebd](https://github.com/YunoHost/yunohost/commit/f0727ebdb4c7d968e1e08cf560072aaba31de921))
[10:11:59]
<Yunohost Git/Infra notifications> [yunohost] Salamandar opened [pull request #1868](https://github.com/YunoHost/yunohost/pull/1868): helpers/v1/go: fix call.
[10:15:32]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 2 commits to dev ([c6bda180b4e8...697a33574b21](https://github.com/YunoHost/yunohost/compare/c6bda180b4e8...697a33574b21))
[10:15:33]
<Yunohost Git/Infra notifications> [yunohost/dev] helpers/v1/go: fix call. goenv latest doesn’t call the plugin anymore, so i’m calling directly the plugin goenv-latest. - Félix Piédallu
[10:15:33]
<Yunohost Git/Infra notifications> [yunohost] alexAubin merged [pull request #1868](https://github.com/YunoHost/yunohost/pull/1868): helpers/v1/go: fix call.
[10:18:51]
<Yunohost Git/Infra notifications> [yunohost] alexAubin deleted branch fix_goenv_again
[10:27:53]
<Salamandar> so, uh, gotta release again ? ><’
[10:27:54]
<Salamandar> sorry…
[10:30:06]
<Aleks (he/him/il/lui)> yeah it's okay i'll do it
[10:30:34]
<Yunohost Git/Infra notifications> 🏗️ Starting build for yunohost/11.2.14+202406101030 for bullseye/unstable/all ...
[10:31:58]
<Yunohost Git/Infra notifications> ✔️ Completed build for yunohost/11.2.14+202406101030 for bullseye/unstable/all.
[10:37:45]
<Yunohost Git/Infra notifications> [yunohost] alexAubin created new tag debian/11.2.14.1
[10:37:54]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 2 commits to dev ([697a33574b21...c9324772f28b](https://github.com/YunoHost/yunohost/compare/697a33574b21...c9324772f28b))
[10:37:58]
<Yunohost Git/Infra notifications> [yunohost/dev] make_changelog: mark as stable by default because its been ages since we had real testings - Alexandre Aubin
[10:38:01]
<Yunohost Git/Infra notifications> 🏗️ Starting build for yunohost/11.2.14.1 for bullseye/stable/all ...
[10:38:53]
<Yunohost Git/Infra notifications> [yunohost] ✖️ Pipeline [#1325186926](https://gitlab.com/YunoHost/yunohost/-/pipelines/1325186926) canceled on branch dev
[10:39:09]
<Yunohost Git/Infra notifications> ✔️ Completed build for yunohost/11.2.14.1 for bullseye/stable/all.
[10:39:19]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 136 commits to migrate-to-bookworm ([383fd6f5d403...307ed10c411b](https://github.com/YunoHost/yunohost/compare/383fd6f5d403...307ed10c411b))
[10:40:29]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 14 commits to bookworm ([caa26ee0056f...bb4f9cc1da55](https://github.com/YunoHost/yunohost/compare/caa26ee0056f...bb4f9cc1da55))
[10:41:25]
<Yunohost Git/Infra notifications> [yunohost/bookworm] Update changelog for 11.2.14.1 - Alexandre Aubin
[10:45:45]
<Yunohost Git/Infra notifications> 🏗️ Starting build for yunohost/11.2.14.1+202406101045 for bullseye/unstable/all ...
[10:46:46]
<Yunohost Git/Infra notifications> ✔️ Completed build for yunohost/11.2.14.1+202406101045 for bullseye/unstable/all.
[10:46:57]
<Yunohost Git/Infra notifications> 🏗️ Starting build for yunohost/12.0.0+202406101045 for bookworm/unstable/all ...
[10:47:55]
<Yunohost Git/Infra notifications> ✔️ Completed build for yunohost/12.0.0+202406101045 for bookworm/unstable/all.
[12:24:42]
<Aleks (he/him/il/lui)> ... is it me or is the mongo helper implementation full nonsense ...
[12:24:47]
<Aleks (he/him/il/lui)> https://github.com/YunoHost/yunohost/blob/dev/helpers/helpers.v1.d/mongodb#L101
[12:24:53]
<Aleks (he/him/il/lui)> the actual call is `gosh --quiet $database --username $user --password $password --authenticationDatabase $authenticationdatabase --host $host --port $port --eval="$command"`
but in every example in the wild, `$user`, `$password`, `$authenticationdatabase`, `$host` and `$port` are empty ...
[12:24:57]
<Aleks (he/him/il/lui)> and in fact if you provide `--user=foobar`, then `$user` becomes equal to `--user=$user`, hence we end up with `--user=--user=foobar`
[12:24:57]
<Aleks (he/him/il/lui)> 🤦♂️
[12:24:59]
<Aleks (he/him/il/lui)> just cheked on wekan log and we do end up indeed with `mongosh --quiet --username --password --authenticationDatabase --host --port`
[12:25:02]
<Aleks (he/him/il/lui)> and anyway no app use either --user, --password, --authenticationdatabase, --database, --host nor --port ...
[12:25:04]
<Aleks (he/him/il/lui)> anybody understand anything about this ? can these be useful in some context ?
[12:25:43]
<Aleks (he/him/il/lui)> i guess i'll trash all this madness in helpers v2.1...
[12:29:31]
<Salamandar> ah ah
[13:14:45]
<Aleks (he/him/il/lui)> continuing my deep dive into helpers to simplify the mess and i'm retro-engineering the composer install script and basically all this does is download `https://getcomposer.org/download/$VERSION/composer.phar` to the install dir ... and check some signature but honestly hmpf we could just trust https and use `curl/wget` instead of running an install script as root every time somebody installs a php/composr app ...
[13:25:23]
<tituspijean> oooor add it as a source :p
[13:38:48]
<Yunohost Git/Infra notifications> [yunohost] 🔴 Pipeline [#1325221520](https://gitlab.com/YunoHost/yunohost/-/pipelines/1325221520) failed on branch bookworm
[13:44:27]
<Yunohost Git/Infra notifications> [yunohost] 🔴 Pipeline [#1325221523](https://gitlab.com/YunoHost/yunohost/-/pipelines/1325221523) failed on branch bookworm, migrate-to-bookworm
[15:01:24]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 12 commits to helpers-2.1 ([8117f438d4c9...4d5ae9d32c4f](https://github.com/YunoHost/yunohost/compare/8117f438d4c9...4d5ae9d32c4f))
[15:37:04]
<Yunohost Git/Infra notifications> [yunohost] Salamandar created new branch fix_multiple_apt_extra
[15:37:07]
<Yunohost Git/Infra notifications> [yunohost] Salamandar pushed 1 commit to fix_multiple_apt_extra: resources.py apt: Fix when multiple extras are passed A wrong indentation leads to code executed at every for loop iter... ([259c7ac4](https://github.com/YunoHost/yunohost/commit/259c7ac4a7f52c3c6ce597ca0106b2ebd8ce8f44))
[15:37:11]
<Yunohost Git/Infra notifications> [yunohost] Salamandar opened [pull request #1869](https://github.com/YunoHost/yunohost/pull/1869): resources.py apt: Fix when multiple extras are passed
[16:19:41]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 2 commits to dev ([c9324772f28b...fa848ff1c464](https://github.com/YunoHost/yunohost/compare/c9324772f28b...fa848ff1c464))
[16:20:15]
<Yunohost Git/Infra notifications> [yunohost] alexAubin deleted branch fix_multiple_apt_extra
[16:20:53]
<Yunohost Git/Infra notifications> [yunohost/dev] resources.py apt: Fix when multiple extras are passed A wrong indentation leads to code executed at every for loop iter... - Félix Piédallu
[16:30:26]
<Yunohost Git/Infra notifications> 🏗️ Starting build for yunohost/11.2.14.1+202406101630 for bullseye/unstable/all ...
[16:32:11]
<Yunohost Git/Infra notifications> ✔️ Completed build for yunohost/11.2.14.1+202406101630 for bullseye/unstable/all.
[16:42:01]
<Aleks (he/him/il/lui)> does it schocks anybody if I start working on a "mega" autopatch script similar to v1->v2 but to propagate https://github.com/YunoHost/yunohost/pull/1855 which I will merge at some point ?
(and then the intent is to automatically create a PR applying the change on every app)
[16:42:55]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 8 commits to helpers-2.1 ([4d5ae9d32c4f...0ceb77ec348b](https://github.com/YunoHost/yunohost/compare/4d5ae9d32c4f...0ceb77ec348b))
[16:43:00]
<Yunohost Git/Infra notifications> [yunohost/helpers-2.1] helpers2.1: remove old internal ynh_render_template, should use ynh_add_config --jinja instead - Alexandre Aubin
[16:47:48]
<Aleks (he/him/il/lui)> (the helpers 2.1 removes 20% of the code, with more improvements in terms of "less bloaty syntax" for many helper calls)
[16:53:01]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 1 commit to helpers-2.1: helpers2.1: YNH_APP_INSTANCE_NAME -> app ([8c3ca4a0](https://github.com/YunoHost/yunohost/commit/8c3ca4a0f4687e42eedc410c6507d2f2533effc1))
[16:53:07]
<Yunohost Git/Infra notifications> [yunohost] ✖️ Pipeline [#1325906234](https://gitlab.com/YunoHost/yunohost/-/pipelines/1325906234) canceled on branch helpers-2.1
[16:53:14]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 1 commit to helpers-2.1: helpers2.1: rename ynh_app_upgrading_from_version_prior_to X -> _before X ([d1e1fd5e](https://github.com/YunoHost/yunohost/commit/d1e1fd5e35e72d1aaf6868481db584e9c4a5ab4d))
[17:01:06]
<Yunohost Git/Infra notifications> [yunohost] ✖️ Pipeline [#1325918527](https://gitlab.com/YunoHost/yunohost/-/pipelines/1325918527) canceled on branch helpers-2.1
[17:02:03]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 1 commit to helpers-2.1: helpers2.1: ynh_get_debian_release -> YNH_DEBIAN_VERSION ([4dc59049](https://github.com/YunoHost/yunohost/commit/4dc59049ef87fb5addd6aef6f3ff6c50d9de491c))
[17:04:34]
<Yunohost Git/Infra notifications> [yunohost] alexAubin edited [pull request #1855](https://github.com/YunoHost/yunohost/pull/1855): Helpers 2.1
[17:08:12]
<Yunohost Git/Infra notifications> [yunohost] ✖️ Pipeline [#1325929061](https://gitlab.com/YunoHost/yunohost/-/pipelines/1325929061) canceled on branch helpers-2.1
[17:39:26]
<Yunohost Git/Infra notifications> [yunohost] alexAubin edited [pull request #1855](https://github.com/YunoHost/yunohost/pull/1855): Helpers 2.1
[17:53:31]
<Yunohost Git/Infra notifications> [yunohost] Thovi98 [commented](https://github.com/YunoHost/yunohost/pull/1855#issuecomment-2158960916) on [issue #1855](https://github.com/YunoHost/yunohost/pull/1855) Helpers 2.1: > Rename ynh_secure_remove --file to ynh_safe_rm --target For my knowledge, is there a specific reason for this? :)
[18:30:26]
<Yunohost Git/Infra notifications> [yunohost] alexAubin [commented](https://github.com/YunoHost/yunohost/pull/1855#issuecomment-2159009667) on [issue #1855](https://github.com/YunoHost/yunohost/pull/1855) Helpers 2.1: >For my knowledge, is there a specific reason for this? :) Not really, mainly personal taste I suppose, I just found it...
[18:42:08]
<Yunohost Git/Infra notifications> [yunohost] alexAubin [commented](https://github.com/YunoHost/yunohost/pull/1855#issuecomment-2159009667) on [issue #1855](https://github.com/YunoHost/yunohost/pull/1855) Helpers 2.1: >For my knowledge, is there a specific reason for this? :) Not really, mainly personal taste I suppose, I just found it...
[18:44:00]
<Yunohost Git/Infra notifications> [yunohost] alexAubin [commented](https://github.com/YunoHost/yunohost/pull/1855#issuecomment-2159009667) on [issue #1855](https://github.com/YunoHost/yunohost/pull/1855) Helpers 2.1: >For my knowledge, is there a specific reason for this? :) Not really, mainly personal taste I suppose, I just found it...
[20:48:22]
<Yunohost Git/Infra notifications> [yunohost] alexAubin pushed 2 commits to helpers-2.1 ([4dc59049ef87...ef924f7a1786](https://github.com/YunoHost/yunohost/compare/4dc59049ef87...ef924f7a1786))
[20:48:27]
<Yunohost Git/Infra notifications> [yunohost/helpers-2.1] helpers2.1: reintroduce the old ynh_restore as ynh_restore_everything because some apps are using it @_@ - Alexandre Aubin
[20:49:16]
<Yunohost Git/Infra notifications> [yunohost] alexAubin edited [pull request #1855](https://github.com/YunoHost/yunohost/pull/1855): Helpers 2.1
[20:57:32]
<Aleks (he/him/il/lui)> `--is_big` and `--not_mandatory` ~\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_~
[21:03:11]
<Aleks (he/him/il/lui)> lulz some apps call `ynh_backup --src_path="$data_dir"` without the `--is_big`
[21:03:44]
<Aleks (he/him/il/lui)> and some call `ynh_backup --src_path="$install_dir"` with `--is_big` resulting in the install dir not being backuped during the safety backup before upgrade
[21:04:08]
<Aleks (he/him/il/lui)> because of course the implication of "is_big" is "don't backup during the safety backup before upgrade"
[21:05:04]
<Salamandar> > <@Alekswag:matrix.org> `--is_big` and `--not_mandatory` ~\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_~
ah ah
[21:05:16]
<Salamandar> > <@Alekswag:matrix.org> lulz some apps call `ynh_backup --src_path="$data_dir"` without the `--is_big`
yeah that sometimes makes sense
[21:05:20]
<Salamandar> > <@Alekswag:matrix.org> and some call `ynh_backup --src_path="$install_dir"` with `--is_big` resulting in the install dir not being backuped during the safety backup before upgrade
that doesen't though
[21:06:25]
<Aleks (he/him/il/lui)> https://github.com/YunoHost/yunohost/blob/dev/helpers/helpers.v1.d/backup#L85
[21:07:03]
<Aleks (he/him/il/lui)> https://github.com/YunoHost/yunohost/blob/dev/src/app.py#L715
[21:08:38]
<Aleks (he/him/il/lui)> in fact the CI should have a test "restore works after a failed upgrade"
[21:08:56]
<Salamandar> > `do_not_backup_data=$(ynh_app_setting_get --app=$app --key=do_not_backup_data)`
omg no
[21:10:09]
<Aleks (he/him/il/lui)> that's advertised somewhere in doc and forum to disable entirely backing up the data of the app, otherwise your archive is gonna explode etc
[21:10:25]
<Aleks (he/him/il/lui)> (eg Nextcloud with a massive amount of data)
[21:10:38]
<Salamandar> > <@Alekswag:matrix.org> that's advertised somewhere in doc and forum to disable entirely backing up the data of the app, otherwise your archive is gonna explode etc
yeah but why that and not --is_big ?
[21:10:40]
<Aleks (he/him/il/lui)> to disable *manually* i mean
[21:10:45]
<Salamandar> ah !
[21:10:59]
<Salamandar> user-side not packager-side, i see
[21:11:36]
<Aleks (he/him/il/lui)> well is\_big is the "flag" that flags the item as "don't backup this if do\_not\_backup\_data is set, or BACKUP\_CORE\_ONLY(= safety backup before upgrade) context"
[21:11:56]
<Aleks (he/him/il/lui)> well is\_big is the "flag" that flags the item as "don't backup this if do\_not\_backup\_data is set, or BACKUP\_CORE\_ONLY(= safety backup before upgrade context)"
[21:12:07]
<Aleks (he/him/il/lui)> anyway, all the semantic is fucked up ...
[21:12:22]
<Aleks (he/him/il/lui)> `do_not_backup_data`, `BACKUP_CORE_ONLY`, `is_big`, `not_mandatory`, ...
[21:13:04]
<Aleks (he/him/il/lui)> like you are supposed to know that using `--is_big` in the backup *implies* that you should use `--not_mandatory` on that item in the restore script
[21:13:33]
<Aleks (he/him/il/lui)> and vice-versa, using `--not_mandatory` is you don't use `--is_big` in the backup script is probably useless
[21:13:41]
<Aleks (he/him/il/lui)> BUT there is also a `--not_mandatory` for ynh_backup
[21:14:06]
<Aleks (he/him/il/lui)> hmgnhmgn
[21:17:22]
<ljf> in some situation you want to backup something only if it exists (--not_mandatory) so you need the same on the restore side...
[21:18:38]
<Aleks (he/him/il/lui)> yeah but there's no consistency check between what's in the backup and restore script so it's a mess when trying to grep all apps T_T
[21:18:42]
<ljf> i guess `ynh_backup --src_path="$install_dir" --is_big` should never append
[21:19:49]
<Aleks (he/him/il/lui)> https://paste.yunohost.org/raw/uhixehikaw
[21:19:58]
<Aleks (he/him/il/lui)> not that many app in the end
[21:20:39]
<ljf> but we could imagine `ynh_backup --src_path="$data_dir"` without --is_big could append if the packager consider those data are very small and could be backup in the safety backup... However, it's pretty dangerous, cause sometimes for 1 app the dir will be small only for 95% of the usecase....
[21:20:59]
<Aleks (he/him/il/lui)> i'm pretty tempted to drop --is_big and instead have a rule "if the path starts with $data_dir, then it's data and shouldnt be backedup during safety-backup-before-upgrade or if do_not_backup_data is set"
[21:21:52]
<Aleks (he/him/il/lui)> ah yeah so those apps don't have the --is_big flag on their data_dir : https://paste.yunohost.org/raw/moxesoduhe
[21:21:52]
<ljf> ihatemoney: /var/log/$app
[21:22:10]
<ljf> synapse: /var/log/matrix-$app
[21:22:41]
<Aleks (he/him/il/lui)> hmmm, so "if the path starts with $data_dir or /var/log" ...
[21:23:40]
<ljf> There are just 2 apps h5ai and whitebophir, and i guess it could be fixed by storing data in $data_dir instead (eventually with mount bind logic)
[21:24:06]
<ljf> On my side i don't understand why thos package backup the logs...
[21:24:43]
<Aleks (he/him/il/lui)> yeah i'm not sure either ...
[21:25:11]
<ljf> I am affraid of that for synapse (whose sometimes make your logs groth quickly)
[21:25:23]
<Aleks (he/him/il/lui)> ogod turns out many apps do `rm` the log folder during the remove script ...
[21:25:59]
<Aleks (he/him/il/lui)> (which means in case of a failed upgrade, the app gets removed-then-restore, but you'll lose the logs in the process)
[21:27:08]
<Aleks (he/him/il/lui)> i don't even know what's the right policy for this x_x
should we treat the log dir the same as the data_dir and only rm it if --purge is used ?
[21:27:38]
<ljf> that's a difficult question
[21:27:56]
<emile> apparmor, maybe ?
[21:28:11]
<Aleks (he/him/il/lui)> a wild emile appearz :O
[21:28:25]
*Aleks (he/him/il/lui) launches a pokeball on emile
[21:30:56]
<Aleks (he/him/il/lui)> i should eat at some point instead of staring at those helpers @_@
[21:33:56]
<ljf> borgwarehouse should not store borg repo inside /home/yunohost.app => backup loop
[21:35:14]
*emile is going back in is soft pokeball, outside world look too scary
[21:37:36]
<Salamandar> i would suggest renaming --not-mandatory to --accept-missing
[21:37:40]
<Salamandar> for clarity
[21:37:52]
<Aleks (he/him/il/lui)> i'd just have `|| true` ...
[21:37:55]
<Salamandar> or --ignore-missing
[21:38:04]
<Salamandar> > <@Alekswag:matrix.org> i'd just have `|| true` ...
yeah but that way you don't know whatfailed
[21:38:17]
<Salamandar> and you allow typos
[21:38:19]
<Aleks (he/him/il/lui)> yes you do because it displays a warning
[21:38:20]
<ljf> > <@Alekswag:matrix.org> i'm pretty tempted to drop --is_big and instead have a rule "if the path starts with $data_dir, then it's data and shouldnt be backedup during safety-backup-before-upgrade or if do_not_backup_data is set"
considering, your grep in yunopaste, i agree with you. Seems all of that is due to historical reasons and doesn't seems needed anymore. So i guess `--is_big` could be replaced by a path detection.
[21:38:51]
<ljf> However, i am not sure we could do the same for --not_mandatory.
[21:39:00]
<Aleks (he/him/il/lui)> these are the occurences of --not_mandatory for something else than the data_dir : https://paste.yunohost.org/raw/aweyazaruy
[21:39:37]
<Aleks (he/him/il/lui)> `jenkins/scripts/backup:20:ynh_backup --src_path="$install_dir" --not_mandatory` dah heck
[21:40:42]
<ljf> we could imagine to apply automatically not_mandatory with some path, and let packager adding the param in some specific situation. Typically, when a file or a dir could exists but not in all setup.
[21:41:32]
<Salamandar> > <@Alekswag:matrix.org> i don't even know what's the right policy for this x_x
>
> should we treat the log dir the same as the data_dir and only rm it if --purge is used ?
> should we treat the log dir the same as the data_dir and only rm it if --purge is used ?
yeah maybe
[21:41:34]
<Salamandar> although
[21:41:41]
<Salamandar> that should be described in the manifest :|
[21:41:47]
<Aleks (he/him/il/lui)> yeah ...
[21:41:49]
<Aleks (he/him/il/lui)> packaging v3 etc ...
[21:42:09]
<Salamandar> eheh
[21:42:57]
<Aleks (he/him/il/lui)> right now practices are a mess, some apps dont seem to have a propre /var/log/$app because their log is only through systemd ... some have a systemd config which binds stdout to /var/log/$app ... some apps probably have the log path configured in their own config instead of writing to stdout ...
[21:43:47]
<Salamandar> btw
[21:43:57]
<Salamandar> can services both write to journal AND a file ?
[21:44:03]
<Aleks (he/him/il/lui)> not to mention the weird deal with logrotate and `--specific_user` (????)
[21:44:05]
<Salamandar> although it duplicates things, and disk space…
[21:44:12]
<Aleks (he/him/il/lui)> probably i guess
[21:44:16]
<Salamandar> > <@Alekswag:matrix.org> not to mention the weird deal with logrotate and `--specific_user` (????)
yeah there's a reason behind specific_user
[21:44:21]
<ljf> log retention have legal implication. I guess in some country thoses logs should be backup (depending of the app) and rotate with accurate values
[21:44:23]
<Salamandar> logrotate is a bit agressive
[21:44:47]
<Salamandar> so if file permissions aren't the ones logrotate expects, it refuses to rotate
[21:45:10]
<Aleks (he/him/il/lui)> :|
[21:45:56]
<Salamandar> that's somewhat understandable : there are sensitive data, and we don't want to copy pasta everywhere files we don't know the origin
[21:46:36]
<Aleks (he/him/il/lui)> but then shouldnt we _always_ have that --specific\_user option in every ynh\_logrotate call considering app logs are supposed to be owned by $app and not root ?
[21:46:58]
<ljf> what about php-fpm logs ?
[21:48:12]
<Aleks (he/him/il/lui)> apps dont call ynh_use_logrotate to handle fpm logs
[21:48:40]
<Salamandar> > <@Alekswag:matrix.org> but then shouldnt we _always_ have that --specific\_user option in every ynh\_logrotate call considering app logs are supposed to be owned by $app and not root ?
in some cases that is root
[21:48:55]
<Salamandar> but i agree with you, that should be rare cases
[21:49:43]
<Aleks (he/him/il/lui)> ```
> grep -nr "ynh_use_logrotate" */scripts/install | grep -v specific_user | wc -l
209
> grep -nr "ynh_use_logrotate" */scripts/install | grep specific_user | wc -l
22
```
@_@
[21:50:32]
<Aleks (he/him/il/lui)> who understands what --specific_user does in the room, raise your hand 😬
[21:52:28]
<Aleks (he/him/il/lui)> me rn https://i.imgflip.com/8tftuy.jpg
[22:09:55]
<Aleks (he/him/il/lui)> ```
su user group
Rotate log files set under this user and group instead of
using default user/group (usually root). user specifies
the user used for rotation and group specifies the group
used for rotation (see the section USER AND GROUP for
details). If the user/group you specify here does not
have sufficient privilege to make files with the ownership
you've specified in a create directive, it will cause an
error. If logrotate runs with root privileges, it is
recommended to use the su directive to rotate files in
directories that are directly or indirectly in control of
non-privileged users.
```
hmmmmm i still don't understand what's the big deal about it running as root and "downgrading" to a user ? It's only supposed to copy/create files anyway ?
[22:12:57]
<Aleks (he/him/il/lui)> `error: skipping "/path/to/log" because parent directory has insecure permissions (It's world writable or writable by group which is not "root") Set "su" directive in config file to tell logrotate which user/group should be used for rotation.`
🤔
[22:15:46]
<Aleks (he/him/il/lui)> sooooo it's only an issue if the group has +w ?
[22:16:13]
<Aleks (he/him/il/lui)> (i'm checking e.g forgejo on saperlipopette and the logs are perfectly rotated despite the lack of `su` directive)
[22:51:34]
<Aleks (he/him/il/lui)> https://github.com/YunoHost/yunohost/pull/511
https://github.com/YunoHost-Apps/horde_ynh/blob/master/scripts/install#L132
https://github.com/YunoHost-Apps/horde_ynh/blob/master/scripts/_common.sh#L65
[22:53:01]
<Aleks (he/him/il/lui)> tl;dr: zzzzz horde has its log in the `$install_dir`, but there's a recursive `chmod` that adds +w to the group, hence the log directory has +w for the group too, and logrotate complain ... but imho the fix is rather to tweak the log dir perm than introduce this boring option >_>
[22:55:59]
<Aleks (he/him/il/lui)> and [this](https://github.com/YunoHost/yunohost/blob/dev/helpers/helpers.v1.d/logrotate#L91) acts on `/var/log/$app` instead of `$(dirname $logfile)` T_T
[23:23:48]
<Émy - OniriCorpe> > <@Alekswag:matrix.org> https://github.com/YunoHost/yunohost/pull/511
> https://github.com/YunoHost-Apps/horde_ynh/blob/master/scripts/install#L132
> https://github.com/YunoHost-Apps/horde_ynh/blob/master/scripts/_common.sh#L65
Was already fixed by https://github.com/YunoHost/yunohost/pull/1774
[23:25:31]
<Aleks (he/him/il/lui)> eeeh yeah but not for horde for which the logs are not in /var/log/$app
[23:25:31]
<Émy - OniriCorpe> A
[23:25:41]
<Émy - OniriCorpe> (Why a package would put logs elsewhere, tho?)
[23:25:48]
<Aleks (he/him/il/lui)> but yeah thank god that fix is there because otherwise that means the packager should know about the fact that the log dir should be `750` which is totally not obvious >_>
[23:26:09]
<Aleks (he/him/il/lui)> > <@oniricorpe:im.emelyne.eu> (Why a package would put logs elsewhere, tho?)
you tell me but there are many examples, sometimes it's because the upsream works like this ...
[23:26:12]
<Aleks (he/him/il/lui)> nextcloud, shaarli, ...
[23:26:31]
<Aleks (he/him/il/lui)> though supposedly they should at least make this configurable somehow
[23:26:32]
<Émy - OniriCorpe> Uh
[23:26:52]
<Aleks (he/him/il/lui)> but so far yunohost packaging doesn't try to enforce packager to force the app to write its shit to /var/log/$app sooo...
[23:27:02]
<Émy - OniriCorpe> It’s configurable for Nextcloud (I did it)