YunoHost / package_linter

Linter for YunoHost applications packages
https://yunohost.org/#/packaging_apps
GNU Affero General Public License v3.0
17 stars 13 forks source link

bad_ynh_exec_syntax() false positive #123

Closed OniriCorpe closed 4 months ago

OniriCorpe commented 10 months ago

the CI warns me

! (Requires Yunohost 4.3) When using ynhexec*, please don't wrap your command between quotes (typically DONT write ynh_exec_warn_less 'foo --bar --baz')

but i don't have quoted code after a ynh_exec_*, my line starts with a quote and ends with a quote, but isn't quoted: ynh_exec_warn_less "$final_path"/gotosocial --config-path "$final_path/config.yaml" admin account promote --username "$admin" my code is clean, so I shouldn't be warned

the CI log: https://ci-apps-dev.yunohost.org/ci/job/9491 the warned code: https://github.com/YunoHost-Apps/gotosocial_ynh/commit/15fe5958c20a5de53e5ef7e81527f36612fdaca5

alexAubin commented 10 months ago

Rough guess but that's probably because you have :

ynh_exec_warn_less "$final_path"/gotosocial [...]

and I would instead write :

ynh_exec_warn_less $final_path/gotosocial [...]

... at least to make the linter happy ... Quoting variables should indeed be recommended, but we end up having to rely to boring greps to check for deprecated practices and those do have a few false-positive situations ... On another angle : I never really understood why we have this ynh_exec_warn_less helper instead of just using the classic 2>&1 ... but I guess the rationale when it was introduced was "2>&1 is super technical and non-tech people don't understand what this means" etc...

Psycojoker commented 10 months ago

Quoting variables should indeed be recommended

This makes shellcheck scream if you don't do it :sweat_smile:

But avoid launching shellcheck on yunohost helps, I tried, it didn't ended up well :<

Tagadda commented 10 months ago

my line starts with a quote and ends with a quote

boring greps

And you didn't even need to end it with a quote :v

https://github.com/YunoHost/package_linter/blob/cb892f1c4203f7a197369b37f6cbec68521a68ce/package_linter.py#L2438-L2441