[17:52:41]
<Salamandar> Looks good, some of my comments are mere suggestions, nothing mandatory
[17:52:46]
<Salamandar> I did not have the time to read install/upgrade scripts but I put some comments on the rest 🙂
[17:52:47]
<Yunohost Git/Infra notifications> App z-push rises from level 4 to 6 in job [#22494](https://ci-apps.yunohost.org/ci/job/22494) !
[18:02:55]
<Yunohost Git/Infra notifications> [apps] @alexAubin pushed 2 commits to master ([89c68a958c59...852ea432e74a](https://github.com/YunoHost/apps/compare/89c68a958c59...852ea432e74a))
[18:03:01]
<Yunohost Git/Infra notifications> [apps/master] Allow + as version separator in manifest.toml schema - Félix Piédallu
[18:04:04]
<Yunohost Git/Infra notifications> [apps] @alexAubin merged [pull request #1971](https://github.com/YunoHost/apps/pull/1971): Allow + as version separator in manifest.toml schema
[18:04:04]
<Aleks (he/him/il/lui)> > <@titus:pijean.ovh> f. that, let's compile all the apps with https://github.com/nexe/nexe 🙃
sounds pretty cool in fact ? I wonder how different this is from like AppImage etc (i know nothing about these stuff except it's like a self-contained executable somehow)
[18:04:04]
<Thomas> > <@Salamandar:matrix.org> I did not have the time to read install/upgrade scripts but I put some comments on the rest 🙂
Waouh nice thanks! Will take a look soon
[18:05:41]
<tituspijean> we should try it actually. I read again the specs, and if it can really cross-compile, we could have our source builder 🙂
[18:18:16]
<Aleks (he/him/il/lui)> 100% accurate, Doctor Strange = Haskell
[18:18:19]
<Aleks (he/him/il/lui)> https://htmx.org/img/memes/extinction.png
[18:33:44]
<Salamandar> IMHO spiderman should be rust. The new kid on the block, everyone loves him.
[18:34:29]
<lapineige> > <@orhtej2:matrix.org> I would consider updating install/update scripts to just delete the `.cache` folder if it's not referenced when running the app. the `--no-dev`/`--production`/`NODE_ENV=production` trick is also obviously a way to go as it'll drastically reduce fetch size when installing the app. Great findings! 👍️
Oh, is that something usable in any NodeJs based app package ?
[18:36:44]
<Salamandar> (i already have a local branch for that fyi)
[18:36:48]
<Salamandar> > <@orhtej2:matrix.org> I would consider updating install/update scripts to just delete the `.cache` folder if it's not referenced when running the app. the `--no-dev`/`--production`/`NODE_ENV=production` trick is also obviously a way to go as it'll drastically reduce fetch size when installing the app. Great findings! 👍️
We should have a "languages" resource with nodejs, python (venv), rust, ruby maybe, that pre-installs everything we need and sets some sane defaults
[18:51:12]
<tituspijean> lapineige: what's the yologen address again?
[18:51:12]
<tituspijean> I've got an "urgent" (self-inflicted) need for Apache Superset for data analysis at $dayjob. I tried to package it a few months ago and deemed it too complex, but now they have a pip install that just worked almost out of the box.
[18:51:42]
<lapineige> > <@Alekswag:matrix.org> can anybody confirm that it should be okay to pre-fetch all the node modules, tar them into an archive, and ship them to other users ? (assuming the same architecture)
What about updates ?
[18:52:22]
<tituspijean> untar and crush any overlapping data 😈
[18:53:30]
<lapineige> > <@titus:pijean.ovh> lapineige: what's the yologen address again?
yologen.lapineige.fr
[18:53:30]
<tituspijean> I *think* it can be fairly simple to adapt the source helper to preserve any config file for instance (like it does now actually)
[18:53:49]
<Mateusz Szymański> > <@titus:pijean.ovh> lapineige: what's the yologen address again?
https://yologen.lapineige.fr/
[18:54:47]
<lapineige> > What about updates ?
I mean update of those dependencies, for instance with security fixes
[18:55:08]
<tituspijean> (I only previously tried it at work, and somehow the domain name gets flagged as "gaming" xD)
[18:55:35]
<lapineige> (You've unmasked me !)
[18:57:38]
<tituspijean> ah yeah, my theory it's because it contains *pine* 😏
[18:57:39]
<lapineige> It's because bunnies are very playful
[19:13:03]
<orhtej2> > <@titus:pijean.ovh> ah yeah, my theory it's because it contains *pine* 😏
Try https://pine64.org/
[19:13:04]
<tituspijean> ... or it's due to *neige* ❄️
[19:13:04]
<tituspijean> (sorry about that, I forgot that element had an animation for it xD)
[19:13:04]
<tituspijean> Will do tomorrow!
[19:14:04]
<lapineige> (Do it AGAIN ❄️)
[19:30:10]
<Aleks (he/him/il/lui)> > <@Alekswag:matrix.org> can anybody confirm that it should be okay to pre-fetch all the node modules, tar them into an archive, and ship them to other users ? (assuming the same architecture)
now that I think about it, I wonder why this doesn't exists :
A service where you can feed a package-json.lock thing
- it generates the node_modules tree (possibly for every archive)
- it tar.gz the whole thing
- it makes the archive publicly available using the hash of the package.json.lock file
Now if you want to deploy an app dependencies you can just hash package.json.lock, hit the API. obtain the .tar.gz, untar and voila
[19:30:16]
<Aleks (he/him/il/lui)> or maybe i don't understsand something about npm idk
[19:30:45]
<Aleks (he/him/il/lui)> maybe it's already what npm ci does in some way but it feels like it's taking ages to just download a bunch of files
[19:37:56]
<Yunohost Git/Infra notifications> [apps] @tituspijean [commented](https://github.com/YunoHost/apps/pull/1799#issuecomment-1906796628) on [issue #1799](https://github.com/YunoHost/apps/pull/1799) Add a Yunohost App Generator (alias Yologen): - [ ] "Sources" field should be optional (Python app can only rely on pip to download sources)
[19:42:01]
<Yunohost Git/Infra notifications> App civicrm_drupal7 failed all tests in job [#22055](https://ci-apps.yunohost.org/ci/job/22055) :(
[19:46:00]
<Yunohost Git/Infra notifications> [apps] @tituspijean [commented](https://github.com/YunoHost/apps/pull/1799#issuecomment-1906796628) on [issue #1799](https://github.com/YunoHost/apps/pull/1799) Add a Yunohost App Generator (alias Yologen): - [ ] "Sources" field should be optional (Python app can only rely on pip to download sources) - [ ] "multi_instance" an...
[19:46:41]
<Yunohost Git/Infra notifications> [apps] @tituspijean [commented](https://github.com/YunoHost/apps/pull/1799#issuecomment-1906796628) on [issue #1799](https://github.com/YunoHost/apps/pull/1799) Add a Yunohost App Generator (alias Yologen): - [ ] "Sources" field should be optional (Python app can only rely on pip to download sources) - [ ] "multi_instance" an...
[19:57:52]
<Yunohost Git/Infra notifications> App gemserv rises from level 0 to 8 in job [#22122](https://ci-apps.yunohost.org/ci/job/22122) !
[19:57:55]
<Yunohost Git/Infra notifications> [apps] @tituspijean [commented](https://github.com/YunoHost/apps/pull/1799#issuecomment-1906796628) on [issue #1799](https://github.com/YunoHost/apps/pull/1799) Add a Yunohost App Generator (alias Yologen): - [ ] "Sources" field should be optional (Python app can only rely on pip to download sources) - [ ] "multi_instance" an...
[20:22:15]
<tituspijean> https://yunohost.org/oc/packaging\_apps\_resources#database
> NB: only one DB can be handled in such a way (is there really an app that would need two completely different DB ?...)
Use case: Superset needs a database to store its internal thingies. It is made to connect to external databases to display graphs and so on. I'm guessing it would be nice to deploy one with the app for the user to be able to use the app out of the box
[20:22:17]
<orhtej2> > <@titus:pijean.ovh> https://yunohost.org/oc/packaging\_apps\_resources#database
>
> > NB: only one DB can be handled in such a way (is there really an app that would need two completely different DB ?...)
>
> Use case: Superset needs a database to store its internal thingies. It is made to connect to external databases to display graphs and so on. I'm guessing it would be nice to deploy one with the app for the user to be able to use the app out of the box
`syncserver-rs` uses 2 databases (one for storage, the other for tokens)
[20:29:45]
<kayou> Thomas: https://github.com/YunoHost-Apps/weblate_ynh/pull/103#pullrequestreview-1839858799 I review the code, sorry, I'm on holidays, and away from my computer
[21:11:42]
<Thomas> > <@kayou:matrix.org> Thomas: https://github.com/YunoHost-Apps/weblate_ynh/pull/103#pullrequestreview-1839858799 I review the code, sorry, I'm on holidays, and away from my computer
Thank you!
[21:27:10]
<Krakotte> Continuation from my question from yesterday about missing parameters before being sauvagely disconnected : yes, to be exhaustive, I'm using if [ -z "${$param:+x}" ], as it allows to test both no variable and empty variable, but that's not the point : this work only in v1. In v2, if "param" is not in the settings of the app (because that's a setting that was optionnal in v1), then it will break the app during upgrade as provisionning fail as it cannot instantiate __PARAM__ in the resources
[21:31:26]
<Aleks (he/him/il/lui)> are you sure you mean `${$param:+x}` and not `${param:-}` ...?
[21:32:09]
<Aleks (he/him/il/lui)> i mean can you share the code that is crashing x_x
[21:33:31]
<Aleks (he/him/il/lui)> as said the syntax you're looking for is something like https://github.com/YunoHost-Apps/nextcloud_ynh/blob/master/scripts/upgrade#L17
[21:34:04]
<Krakotte> nope
[21:34:12]
<Krakotte> that's the provisionning which is crashing
[21:34:22]
<Krakotte> not my script
[21:34:32]
<Aleks (he/him/il/lui)> can't help without the logs and code
[21:35:08]
<Krakotte> I can do that 🙂
[21:35:46]
<Krakotte> give me some minutes
[21:52:46]
<Aleks (he/him/il/lui)> https://i.imgflip.com/8dcmjp.jpg
[21:54:56]
<Krakotte> yeah, I know 🙂
[21:55:13]
<Krakotte> in the meantime, I use this for bash :
[21:55:24]
<Krakotte> https://stackoverflow.com/questions/3601515/how-to-check-if-a-variable-is-set-in-bash
[21:55:38]
<Krakotte> with a nice table with all the syntax to test
[21:57:13]
<Aleks (he/him/il/lui)> dude just ... look at the example i linked in nextcloud's upgrade script ...
[21:58:05]
<Aleks (he/him/il/lui)> but then you're saying it happens "during provisioning" (of the resources?) and "not your script" but we don't have the error message so can't magically help
[21:58:50]
<Krakotte> https://paste.yunohost.org/raw/otagapaveh
[21:58:55]
<Krakotte> at least
[21:59:23]
<Krakotte> sorry, I add to rerun the upgrade a few times and so it took some times
[22:00:08]
<Aleks (he/him/il/lui)> right : ` Failed to provision permissions : Domaine '__MQTT_DOMAIN__' inconnu`
[22:00:10]
<Krakotte> code is here : https://github.com/YunoHost-Apps/domoticz_ynh/blob/testing/manifest.toml
[22:00:24]
<Aleks (he/him/il/lui)> so you're trying to use a variable in a permission
[22:00:57]
<Krakotte> yes, when the variable is set, no pb
[22:01:17]
<Krakotte> in v1, I just added it in the upgrade script if it was missing
[22:01:28]
<Krakotte> prior to initialize the permission
[22:02:15]
<Aleks (he/him/il/lui)> so yes this has nothing to do with bash
[22:02:35]
<Krakotte> maybe some regex magic could do something ?
[22:03:15]
<Krakotte> didn't think about that before
[22:05:03]
<Krakotte> nope, forget about that
[22:06:29]
<Aleks (he/him/il/lui)> well there's two distinct issues there
[22:06:36]
<Aleks (he/him/il/lui)> the first is that maybe the setting was not defined in the past and now you need it to be defined, but you can only define it after the upgrade bash script start, which is too late
[22:07:20]
<Aleks (he/him/il/lui)> but that one could be worked around with a PRE_UPGRADE.d/ notification asking people to manually set the setting with `yunohost app setting ...`, bit boring and technical but that'd work
[22:08:00]
<Krakotte> that's what I did...
[22:08:03]
<Aleks (he/him/il/lui)> the real issue is that this is like a "conditional"/"optional" permission because MQTT_DOMAIN may be legitly empty as for what I see in the help for this question
[22:08:51]
<Krakotte> yes, so for now, the permission is created AND I remove it in the install script if needed
[22:08:56]
<Krakotte> dirty, I know
[22:09:21]
<Krakotte> but it goes through the CI flawlessly 😇️
[22:09:57]
<Aleks (he/him/il/lui)> @_@
[22:09:57]
<Aleks (he/him/il/lui)> yes and "I lost my keys so I removed the door" also pass the CI flawlessly
[22:10:25]
<Aleks (he/him/il/lui)> imho it should be the other way around : don't create the permission in the manifest, just add it when relevant in the install and upgrade script (and restore i guess)
[22:10:26]
<Krakotte> you did that too?
[22:10:29]
<Aleks (he/him/il/lui)> but it's a bit "eh"
[22:10:47]
<Krakotte> well, this don't go throught the CI
[22:10:58]
<Aleks (he/him/il/lui)> why doesn't it go through the CI then
[22:11:00]
<Krakotte> as the helpers are deprecated and that downgrade the app level
[22:11:57]
<Krakotte> that's why I bring it here before pushing the new update as I was feeling it was becoming a little akward...
[22:13:20]
<Aleks (he/him/il/lui)> tbf your current app only "pass the CI" because you have no tests for the case where the user enter "empty string" for the MQTT domain
[22:13:21]
<Aleks (he/him/il/lui)> yes the helpers are deprecated but it doesn't mean it doesn't pass the CI, it just means there's some hack going on, we have this for other apps such as Nextcloud too
[22:13:38]
<Krakotte> ynh_permission_create & ynh_permission_exists trigger a warning, ynh_permission_delete doesn't. I assumed there was some kind of logic that I couldn't guess
[22:14:15]
<Krakotte> I made a preupgrade "yunohost app setting ...' so that it doesn not fail
[22:14:40]
<Aleks (he/him/il/lui)> i don't know much about this app but can you explain the full story about "why this domain" in the first place
[22:15:22]
<Krakotte> Sure : That's not the smartest things I did to be honest...
[22:15:30]
<Krakotte> First domain is for the main app
[22:15:35]
<Aleks (he/him/il/lui)> yes but the manifest contains: `MQTT server domain. Set blank or the main domain if you don't wish to use it. See the doc for more info`
so what if the user leaves the field blank ?
[22:16:04]
<Krakotte> second domain is used for the mosquitto broker, which at the time I did it was not yet packaged
[22:16:35]
<Aleks (he/him/il/lui)> i'm also a bit confused because sometimes in tests you're using `mqtt_domain="sub.domain.tld` (= same as "domain") and some other times `mqtt_domain="mqtt.domain.tld"` (= different domain)
[22:17:00]
<Krakotte> I overthought this, and now I'm a little stuck with it, I planned to decommissioned it but doing that at the same time as v2 was too much
[22:17:30]
<Aleks (he/him/il/lui)> so it's kind of a "your app depends on another app" situation
[22:17:42]
<Krakotte> If the field is left blanck yunohost somehow re-set the first domain. That was not the case in v1, I just took it
[22:18:30]
<Krakotte> In my test, I have sub.domain.tld and mqtt.domain.tld so I can test all the upgrade situation, that's how I noticed this issue in the first place
[22:19:04]
<Krakotte> yes, initially it was just an optionnal package, then I added a domain, and then the shit hit the fan
[22:19:31]
<Krakotte> 😅️
[22:20:16]
<Krakotte> So your advice would be : forget the CI and do it the "good ol' way" with the helpers?
[22:21:03]
<Aleks (he/him/il/lui)> i'm still confused ... from what I understand, `mqtt_domain` can be either 1) the same as $domain, 2) a different domain than $domain, 3) empty - my point being that if you were to test the "empty value" case, it will probably trigger the same error of `Domaine '__MQTT_DOMAIN__' inconnu` during install
[22:21:57]
<Krakotte> you mean in the test.toml scenarii?
[22:22:04]
<Aleks (he/him/il/lui)> > <Krakotte> So your advice would be : forget the CI and do it the "good ol' way" with the helpers?
at this stage i'd say : if mosquitto is packaged as a separate app, just tell people to install mosquitto beforehand, otherwise you'll endup with a bunch of dirty hacks related to the fact that you want to provision permissions for virtually two apps at the same time, and same for ports
[22:22:09]
<Aleks (he/him/il/lui)> > <Krakotte> you mean in the test.toml scenarii?
yes
[22:22:23]
<Krakotte> You're right, I didn't think to test it
[22:25:29]
<Krakotte> yep, will do as this whole "two app for the price of one" stuff is also causing me headache
[22:26:48]
<Aleks (he/him/il/lui)> it's a bit annoying because it involves refactoring the app a bit but it sounds much cleaner on the long run
[22:26:52]
<Krakotte> I will push the upgrade as-is, so that it at last move to v2, with the warning for the users to add manually the setting if required, then I will work on decommissioning the mosquitto part
[22:27:25]
<Aleks (he/him/il/lui)> there's a `type = "app"` option which may be worth having a look into to tell people to select the mosquitto app to bind to
[22:28:00]
<Krakotte> in the catalog? Good to know
[22:28:17]
<Aleks (he/him/il/lui)> in the manifest.toml install questions i mean
[22:28:43]
<Krakotte> ah ok, still better!
[22:29:10]
<Krakotte> you guys do so many improvement all the time, I'm sometimes overwhelmed 🙂
[22:29:53]
<Aleks (he/him/il/lui)> haha it's probably here since a while it's just not everyday that you need to ask for another app in install questions
[22:30:25]
<Krakotte> Well, I must be working on a special app, because I have a third one 🙂
[22:30:35]
<Krakotte> that I packaged separately this time 🙂
[22:31:19]
<Krakotte> anyway, thanks for your time, advice and patience!
[22:31:44]
<Krakotte> and sorry if my question was unclear at the begininng
[22:35:06]
<Salamandar> https://i.stack.imgur.com/T2Fp8.png
[22:35:07]
<Salamandar> > <Krakotte> https://stackoverflow.com/questions/3601515/how-to-check-if-a-variable-is-set-in-bash
FYI here is my bible
[22:37:35]
<Aleks (he/him/il/lui)> > Albert Camus, in his 1942 essay The Myth of Sisyphus, saw Sisyphus as personifying the absurdity of human life, but Camus concludes "one must imagine Sisyphus happy" as "The struggle itself towards the heights is enough to fill a man's heart."
plot twist Sisyphus was happy because he knew he didn't get sentenced to asking users for context and full logs
[22:39:17]
<Krakotte> Well : it seems we have the same bible, this make us coreligionist...
[22:40:04]
<Yunohost Git/Infra notifications> [apps] @orhtej2 opened [pull request #1972](https://github.com/YunoHost/apps/pull/1972): Support for GitLab autoupgrade.
[22:40:04]
<Aleks (he/him/il/lui)> > <@Salamandar:matrix.org> https://i.stack.imgur.com/T2Fp8.png
what teh frak
[22:40:24]
<Salamandar> > <@Alekswag:matrix.org> > Albert Camus, in his 1942 essay The Myth of Sisyphus, saw Sisyphus as personifying the absurdity of human life, but Camus concludes "one must imagine Sisyphus happy" as "The struggle itself towards the heights is enough to fill a man's heart."
>
> plot twist Sisyphus was happy because he knew he didn't get sentenced to asking users for context and full logs
We're in the wrong chan, the Log bot isn't there
[22:40:41]
<Aleks (he/him/il/lui)> x)
[22:40:48]
<Yunohost Git/Infra notifications> [apps] @orhtej2 edited [pull request #1972](https://github.com/YunoHost/apps/pull/1972): Support for GitLab autoupgrade.
[22:40:51]
<Aleks (he/him/il/lui)> my god i hate bash so much
[22:42:15]
<orhtej2> > <@yunohostinfra:matrix.org> [apps] @orhtej2 opened [pull request #1972](https://github.com/YunoHost/apps/pull/1972): Support for GitLab autoupgrade.
I'd like a thorough review on this one, especially given I moved existing Github-related stuff.
Certified Works On My Machine For A Handful Tests I Did but still
[22:42:15]
<Aleks (he/him/il/lui)> oooh you factorized the API stuff, neat
[22:43:23]
<orhtej2> > <@Alekswag:matrix.org> oooh you factorized the API stuff, neat
yeeeeah easier to repackage Gitlab response into Github-compatible one than copy-paste a ton of code
[22:43:25]
<Aleks (he/him/il/lui)> > I'd like a thorough review on this one, especially given I moved existing Github-related stuff.
> Certified Works On My Machine For A Handful Tests I Did but still
hmmmm yes, how familiar are you with the word "protoduction" ? 🤔
[22:45:22]
<Yunohost Git/Infra notifications> [apps] @orhtej2 [commented](https://github.com/YunoHost/apps/pull/1972#issuecomment-1907044500) on [issue #1972](https://github.com/YunoHost/apps/pull/1972) Support for GitLab autoupgrade.: Test URL for releases: https://code.castopod.org/api/v4//projects/2/releases Test URL for tags: https://code.castopod.or...
[22:45:22]
<Yunohost Git/Infra notifications> [apps] @orhtej2 [commented](https://github.com/YunoHost/apps/pull/1972#issuecomment-1907044500) on [issue #1972](https://github.com/YunoHost/apps/pull/1972) Support for GitLab autoupgrade.: Test URL for releases: https://code.castopod.org/api/v4//projects/2/releases Test URL for tags: https://code.castopod.or...
[23:08:44]
<Yunohost Git/Infra notifications> [apps] @tituspijean [commented](https://github.com/YunoHost/apps/pull/1799#issuecomment-1906796628) on [issue #1799](https://github.com/YunoHost/apps/pull/1799) Add a Yunohost App Generator (alias Yologen): - [ ] "Sources" field should be optional (Python app can only rely on pip to download sources) - [ ] "multi_instance" an...
[23:16:41]
<Yunohost Git/Infra notifications> [apps] @alexAubin approved [pull request #1972](https://github.com/YunoHost/apps/pull/1972#pullrequestreview-1840109210) Support for GitLab autoupgrade.: LGTM, id say lets run this in prod and see what happens, its "just" a dev tool anyway, *we* are the user base P
[23:24:02]
<Yunohost Git/Infra notifications> [apps] @orhtej2 [commented](https://github.com/YunoHost/apps/pull/1972#issuecomment-1907082621) on [issue #1972](https://github.com/YunoHost/apps/pull/1972) Support for GitLab autoupgrade.: Ill add support for source tarballs in releases and polish up some omissions soon (TM)
[23:25:20]
<Yunohost Git/Infra notifications> [apps] @orhtej2 just made [pull request #1972](https://github.com/YunoHost/apps/pull/1972) ready for review: Support for GitLab autoupgrade.
[23:26:27]
<Yunohost Git/Infra notifications> [apps] @orhtej2 [commented](https://github.com/YunoHost/apps/pull/1972#issuecomment-1907084094) on [issue #1972](https://github.com/YunoHost/apps/pull/1972) Support for GitLab autoupgrade.: On a second thought given nobody uses it for now lets give it a spin as-is and Ill add the missing part in separate re...
[23:26:44]
<Yunohost Git/Infra notifications> [apps] @alexAubin merged [pull request #1972](https://github.com/YunoHost/apps/pull/1972): Support for GitLab autoupgrade.
[23:26:44]
<orhtej2> you know what, let's merge it, I'll add source tarballs later as they're not used ATM anyway and if it blows up just revert it for take 2
[23:26:48]
<Yunohost Git/Infra notifications> [apps/master] Merge branch master into support_gitlab_autoupgrade - Alexandre Aubin
[23:26:49]
<Yunohost Git/Infra notifications> [apps] @alexAubin pushed 4 commits to master ([852ea432e74a...0fe7c0d743a6](https://github.com/YunoHost/apps/compare/852ea432e74a...0fe7c0d743a6))
[23:26:49]
<Aleks (he/him/il/lui)> i'll run it manually in prod to see if there's any obvious regression
[23:26:49]
<Yunohost Git/Infra notifications> [apps/master] Initial support for Gitlab - orhtej2
[23:36:34]
<Aleks (he/him/il/lui)> 193 apps with autoupdate enabled, really ? o.O
[23:36:45]
<Aleks (he/him/il/lui)> anyway seems to be running just fine