YunoHost / issues

General issue tracker for the YunoHost project
71 stars 8 forks source link

Include yq as dependency #2146

Open thardev opened 1 year ago

thardev commented 1 year ago

Since I worked on maintaining and implementing the config panel in mautrix_whatsapp app, I thought that it would be a good idea to include yq either as deps of the packages that use it (like every app that has a YAML file as config file) or as Yunohost core dependency since YAML seems to be pretty widespread. For now the alternative is to use sed which works fine but is much slower to implement and it is much bug prone in my opinion, also much less intuitive.

I know I asked this on the Yunohost App Developers Matrix channel but I want to have a proper agreement and opinions from people that know more about the Yunohost core structure. My opinion is that I would be fine to include it with the apps that need it, not necesarily include it as Yunohost dependency (that could be more challenging I guess) so, what are your thoughts about it? 😄

Thank you!

alexAubin commented 1 year ago

I'm confused, can you elaborate exactly what use case you want to cover with this ?

The config panel thngy already has it's own (kinda dirty honestly) mechanism which get/set the correspond key/value. Doesn't this work ?

ynh_add_config works with the __FOOBAR__ syntax

Soooo I'm not sure exactly why you refer to sed, in both those mechanism you ain't supposed to use sed

thardev commented 1 year ago

Yes, I'm refering more to this kind of multiline config options, e.g. https://github.com/YunoHost-Apps/synapse_ynh/blob/2a0a99362ac013181074d86e2364549e30391ae3/conf/homeserver.yaml#L189-L208

Also this function I had to do for mautrix_whatsapp would be much easier with yq as it is meant to do this kind of stuff. I don't know if this ynh_add_config detects the extension of the config file and I don't know about the complexity of wrapping yq using a Yunohost helper, I guess you want to keep the helpers extension agnostic but could be an option as well. I know this might not be a priority but since more and more packages use YAML as config file, this would be a good addition to help the maintainers and to make packaging and maintaining easier. As I said, I don't necessarily think it should be included in Yunohost core but we could standarize to include it with the packages that have a YAML config file at least... 🙂

I don't know if it is relevant at all but yq is also included in the Github Actions runner images: https://github.com/actions/runner-images/blob/ubuntu20/20221119.2/images/linux/Ubuntu2004-Readme.md

alexAubin commented 1 year ago

It's not a huge deal to add yq as a dependency (though I just noticed that it's only available in debian bookworm repo, though we could also manually add that .deb to our own repo) : https://packages.debian.org/fr/bookworm/amd64/yq

Indeed the config panel mechanism doesnt really handle list/arrays well ;)

Though I'm wondering if yq will keep the comments ?

thardev commented 1 year ago

That's nice 🙂 I think it has to preserve them, I did a search in the issues of the repo and there could be some issues with some operations but I think the default is to keep them. https://github.com/mikefarah/yq/issues?q=is%3Aissue+is%3Aopen+comment

There is also this comment operator to handle them as well. https://mikefarah.gitbook.io/yq/operators/comment-operators

zamentur commented 1 year ago

Use yq for json/xml/yaml file could be a good idea to be able to edit array properly. In a perfect world, we could find a syntax to be able to edit the value from the bind property of a config_panel.toml file without the need of a config script

But the first step is to add yq in our repo indeed

Gredin67 commented 5 months ago

Regarding the matrix stack, I think we can wait for bookworm to get yq. Most bridges, and synapse are in the middle of refactorings. Which means it will take time before we can get clean packages with v2, and config panel etc. That said, we should all work keeping in mind to switch to yq while refactoring, installing it locally in our development environments.