Sunday, August 14, 2022
dev@conference.yunohost.org
August
Mon Tue Wed Thu Fri Sat Sun
1 2 3 4 5 6 7
8 9 10 11 12 13
14
15
16
17
18
19 20
21
22
23
24 25 26
27
28
29
30
31
       
             

[18:48:24] <Aleks> TFW you ask for an user to share their diagnosis result, and they litterally have **33** domains configured....
[18:48:37] <Dante> Hi again 😄 so after my journey to bash debug hell, I found out passing arguments like `-n` `ynh_use_logrotate -n` and then parsing them like `arguments=("$@")` seems to give empty value... also happens with `-e`.
Also when trying to pass `--nonappend` it gets substituted to something like `y append` in this line: https://github.com/YunoHost/yunohost/blob/140e50253fac0d3c9aa6fcab9e392a462c914e98/helpers/getopts#L80
Because of the existing legacy params `[y]=non [a]=append`
So it seems there's no way to use `--nonappend` 😱 but what bugs me a bit it's that for other packages seems to work fine the option or there doesn't seem to be much reports on the subject...
[18:48:52] <Navan> > <@Alekswag:matrix.org> TFW you ask for an user to share their diagnosis result, and they litterally have **33** domains configured....

what the fuck
[18:49:23] <Navan> to be fair, you have to add dedicated domains for so many apps
[18:50:01] <Navan> > <@thardev:matrix.org> Hi again 😄 so after my journey to bash debug hell, I found out passing arguments like `-n` `ynh_use_logrotate -n` and then parsing them like `arguments=("$@")` seems to give empty value... also happens with `-e`.
> Also when trying to pass `--nonappend` it gets substituted to something like `y append` in this line: https://github.com/YunoHost/yunohost/blob/140e50253fac0d3c9aa6fcab9e392a462c914e98/helpers/getopts#L80
> Because of the existing legacy params `[y]=non [a]=append`
> So it seems there's no way to use `--nonappend` 😱 but what bugs me a bit it's that for other packages seems to work fine the option or there doesn't seem to be much reports on the subject...

uhhhhhh I don't have any context for your question
[18:50:39] <Aleks> ogod you ended up reading `getopts`
[18:50:45] *Aleks flies away
[18:50:56] <Dante> 😂
[18:51:11] <Aleks> https://www.youtube.com/watch?v=92gP2J0CUjc
[18:52:00] <Dante> > Hi 😄 does anyone know if the `ynh_use_logrotate --nonappend` option works on latest Yuno? I had this issue report recently https://github.com/YunoHost-Apps/mautrix_whatsapp_ynh/issues/59 I tried to fix it using `--nonappend` but it only seemed to work when I used `--non-append` which is supposed to be legacy option 🤷‍♂️ any help would be appreciated 😄

it is related to this question from yesterday
[18:52:26] <Navan> oh awesome, let me scroll back up
[18:52:59] <Aleks> ` [y]=non [a]=append)`
[18:53:02] <Aleks> wait what
[18:53:14] <Aleks> reaaaaaaallllyyyy
[18:53:33] <Dante> so for now the only solution I found is to remove those `[y]=non [a]=append`
[18:54:03] <Aleks> yeah
[18:54:32] <Aleks> i mean it's supposed to be parsed by those funky `if $1 == "--non-append"` later ...
[18:54:53] <Dante> I did a bit of searching and it seems that the legacy `--non-append` is not being used anywhere...
[18:55:27] <Dante> so maybe it is safe to remove those
[18:55:48] <Dante> > <@Alekswag:matrix.org> i mean it's supposed to be parsed by those funky `if $1 == "--non-append"` later ...

and also this
[18:56:01] <Aleks> we'll i'm grepping all the apps and there's a shitload of them still using it ...
[18:56:17] <Aleks> 141 matches...
[18:56:57] <Aleks> i mean since no code displays any warning about using that option i'm not surprised that nobody is aware that this is legacy and shouldnt be used
[18:56:58] <Dante> whoops haha I did a github search... thought it was more reliable😶‍🌫️
[18:57:15] <Aleks> github search is complete batshit unfortunately
[18:57:36] <Aleks> like sometimes you search for keyword that obviously exist in the code and it doesnt find anything
[18:57:43] <Navan> GitHub codesearch is so much better
[18:57:49] <Navan> it is in technology preview right now
[18:57:50] <Aleks> or rather **most of the time** it doesnt find anything
[18:58:00] <Aleks> oooh they have a new thingy ?
[18:58:03] <Navan> https://aria.im/_matrix/media/v1/download/navan.dev/ysOqrFSLNYtbGABJVcUSWaEq
[18:58:06] <Dante> yeah I experienced that...
[18:58:30] <Dante> looks much better yeah
[18:58:34] <Aleks> I find only 14 match for `--nonappend` ...
[18:59:22] <Aleks> i'm not even sure to understand why --non-append is supposed to be legacy ... Or is it that getopts is unable to properly parse option names with dashes in it ?
[19:00:55] <Dante> dunno maybe since this is not something that critical I guess I'll use `--non-append`... because at least it works and I don't think it matters that much...
[19:01:15] <Aleks> honestly I would just hack something like, before `getopt` is called, let's just loop over all the arguments, and `$arg == '--non-append'`, then replace the arg with `--nonappend` and voila
[19:01:41] <Dante> > <@Alekswag:matrix.org> i'm not even sure to understand why --non-append is supposed to be legacy ... Or is it that getopts is unable to properly parse option names with dashes in it ?

not really sure why it got changed :( I could check when it happened maybe we'll know the reason
[19:01:46] <Aleks> Dante: well it somewhat matters because currently it sounds like we have a bunch of apps with possibly fucked up logrotate configs
[19:01:47] <Aleks> i mean
[19:02:04] <Aleks> for example:
[19:02:17] <Aleks> `mautrix_telegram/scripts/upgrade:169:ynh_use_logrotate --non-append --logfile "/var/log/$app/$app.log"`
[19:02:34] <Aleks> here `--non-append` is in $1
[19:02:35] <Aleks> but
[19:03:14] <Aleks> ah nvm i though the code was `if [ $# == 1 ] && [ "$1" == "--non-append" ]` but in fact it's `if [ $# -gt 0 ] && [ "$1" == "--non-append" ]`
[19:03:23] <Dante> yeah all the apps using `--nonappend` are bugged I guess
[19:04:21] <Aleks> `mautrix_telegram/scripts/upgrade:169:ynh_use_logrotate --non-append --logfile "/var/log/$app/$app.log" `
[19:04:27] <Aleks> ah that one works too
[19:05:04] <Aleks> well whatever it's complete batshit crazy that you have to set `--non-append` either as first or second arg for it to work, and if you happen to set it as third or more arg it will be ignored
[19:05:27] <Aleks> let me check if the fix I'm thinking about works ...
[19:21:38] <Aleks> ```
[ERROR] Your IP or domain 80.67.172.144 is blacklisted on Hostkarma
- After identifying why you are listed and fixed it, feel free to ask for your IP or domaine to be removed on https://ipadmin.junkemailfilter.com/remove.php
```
[19:21:40] <Aleks> woopsies ...
[19:21:46] <Aleks> that's on Samurai / Saperlipopette
[19:35:25] <Aleks> Dante: https://github.com/YunoHost/yunohost/commit/8d1c75e732b59846f0799180217417cd3857fbf7
[19:35:27] <Aleks> #yolo ...
[19:36:06] <Aleks> hm i should have add a link to https://stackoverflow.com/questions/4827690/how-to-change-a-command-line-argument-in-bash to explain the `set --` magic ...
[19:37:47] <Dante> ooh 👏👏
[19:38:03] <Dante> thanks a lot for this!
[19:41:21] <Aleks> hm i'm suspiciours about the `shift` line 36
[19:41:23] <Dante> maybe if it's not too complicated add a warn so packagers start to migrate from `--non-append` to `--nonappend` and at some point we can get rid of the patch? 😬
[19:41:49] <Aleks> sounds like there's more hackish magic on line 41
[19:43:27] <kayou> yeah, i think getops can't parse option with a dash in his name
[19:43:45] <kayou> we could replace `--non-append` by `--non_append`
[19:44:13] <Aleks> (apparently `if [ $# -gt 0 ] && [ "$(echo ${1:0:1})" != "-" ]; then` seems to be about parsing if the first arg was directly a filename but nowadays all app seems to be using `--logfile=foobar` so we could probably get rid of that legacy bit too ...)
[19:44:40] <Aleks> > <@kayou:matrix.org> we could replace `--non-append` by `--non_append`

you mean introduce a *third* name for the same option ? :D
[19:44:44] <kayou> or patch `ynh_handle_getopts_args`, but meh, good luck
[19:45:52] <Aleks> and now i'm wondering if getopts could be rewritten in python somehow ...
[19:45:56] <kayou> it's just that we have plenty of args with `_` https://github.com/YunoHost/yunohost/blob/8d1c75e732b59846f0799180217417cd3857fbf7/helpers/backup#L8-L11
[19:49:10] <Aleks> yeah ... but like ultimately that option shouldnt even exists ... Everytime `use_logrotate` gets called for the first time in the script, you want to replace the entire file, and every subsequent calls, you want to append the file, we could just save that info using a global boolean and voila ...
[19:59:03] <Dante> yeah, that could be a solution as well 😄
[20:00:05] <Dante> btw I've been thinking that I could also try to maintain mautrix_signal and mautrix_instagram if you could give me access 👍️🙂
[20:01:19] <Navan> mautrix_instagram does not work well :(((
[20:02:35] <Navan> I am repackaging it right now, but I am more than happy to hand it over to you once the initial packaging is done
[20:03:24] <Dante> oh :( well I'll give it a shot as well 😄 I'll be glad to take over
[20:04:15] <Navan> I will ping you on the app developers channel when I am done with it
[20:04:47] <Dante> regarding Signal one seems to be handled already by Gredin67 so maybe not needed atm