YunoHost / issues

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

extend manifest.toml [resources.apt] section to allow _interactive_ installation of dependencies #2377

Open chri2 opened 7 months ago

chri2 commented 7 months ago

When using in manifest.toml something like …

    [resources.apt]
    extras.couchdb.repo = "deb https://apache.jfrog.io/artifactory/couchdb-deb/ __YNH_DEBIAN_VERSION__ main"
    extras.couchdb.key = "https://couchdb.apache.org/repo/keys.asc"
    extras.couchdb.packages = ["couchdb"]

everything will be setup as wished if couchdb doesn't need an interactive configuration during installation.

Formerly (v1 package format I guess) this would have been done like e.g. here in scripts/install:

echo "\
couchdb couchdb/mode select standalone
couchdb couchdb/mode seen true
couchdb couchdb/bindaddress string 127.0.0.1
couchdb couchdb/bindaddress seen true
couchdb couchdb/cookie string $password
couchdb couchdb/adminpass password $password
couchdb couchdb/adminpass seen true
couchdb couchdb/adminpass_again password $password
couchdb couchdb/adminpass_again seen true" | debconf-set-selections
DEBIAN_FRONTEND=noninteractive # apt-get install -y --force-yes couchdb

ynh_install_extra_app_dependencies \
    --repo="deb https://apache.jfrog.io/artifactory/couchdb-deb/ $(lsb_release -c -s) main" \
    --key="https://couchdb.apache.org/repo/keys.asc" \
    --package="couchdb"

The CI package_check comments this with ! Using helper ynh_install_extra_app_dependencies is deprecated when using packaging v2 ... It is replaced by: the apt source.

To get around this I'd like to see manifest.toml extended to support something like this:

[resources.apt]
extras.couchdb.packages = [ ["couchdb", "no-auto-install" ], "other_package" ]

Extending the configuration in this will will not alter the format of manifest.toml and keep things backward compatible.

It will offer the option to put instead of a packet name string like "other_package" an array into the array of packages.

If an array instead of a string is found while parsing manifest.toml a new piece of code can called that in this case would not call ynh_install_app_dependencies and let the maintainer of the app take care of that inside scripts/install.

Another example for an array being used in the list could be something like

[resources.apt]
extras.couchdb.packages = [ ["couchdb", "debconf-set-selections=couchdb couchdb/mode select standalone\ncouchdb couchdb/mode seen true\ncouchdb couchdb/bindaddress string 127.0.0.1\ncouchdb couchdb/bindaddress seen true\ncouchdb couchdb/cookie string $password\ncouchdb couchdb/adminpass password $password\ncouchdb couchdb/adminpass seen true
\ncouchdb couchdb/adminpass_again password $password\ncouchdb couchdb/adminpass_again seen true"], "other_package" ]

to actually include all the settings a new or extended ynh_install_app_dependencies helper should use to install the package using DEBIAN_FRONTEND=noninteractive .

I'd love to see some discussion, alternative approaches, improvements discussed to finally end up with a let's do it this way which I would be willing to work through in my own very slow way to provide a PR.

Update: I track my flohmarkt issue here

chri2 commented 7 months ago

Another question (that might need its own new issue):

If more than one app does pull in a dependency like couchdb in the example how do we decide on the global configuration of the database? On apps-developer someone already mentioned that the situation might be similar with mongodb: not included in core, but probably needed by more than one app in the future (or already).

chri2 commented 7 months ago

Looked through the yunohost-apps org to find the apps using couchdb and opened an issue on them.

tituspijean commented 6 months ago

Another example with OnlyOffice:

    echo onlyoffice-documentserver onlyoffice/ds-port select "$port" | debconf-set-selections
    echo onlyoffice-documentserver onlyoffice/db-host string 127.0.0.1 | debconf-set-selections
    echo onlyoffice-documentserver onlyoffice/db-user string "$db_user" | debconf-set-selections
    echo onlyoffice-documentserver onlyoffice/db-pwd password "$db_pwd" | debconf-set-selections
    echo onlyoffice-documentserver onlyoffice/db-name string "$db_name" | debconf-set-selections
    echo onlyoffice-documentserver onlyoffice/jwt-secret password "$jwt_secret" | debconf-set-selections

In this instance, we need to have the port, database, and JWT variables set before running the debconf. To be fully declarative, we need to have a variables resource in the manifest, to avoid relying on the install script to set the required variables. Example here with the JWT secret:

# from script/install
jwt_secret=$(ynh_string_random --length=32)

However, we need to address some kind of resource priority to establish the variables before running any scripts from the other resources, and to make sure we do not generate circular dependencies in the manifest.

chri2 commented 6 months ago

However, we need to address some kind of resource priority to establish the variables before running any scripts from the other resources, and to make sure we do not generate circular dependencies in the manifest.

Good point. You're right. For flohmarkt_ynh couchdb installation I'd need to generate a password and cookie first, also.

Maybe we just should accept it to be o.k. to define an additional repository in manifest.toml and a dependency for a package from that repository and allow some helper function to install the package in the scripts from an already defined repository (without criticising that from linter).

The 'install package from in manifest.toml defined repository' function should only offer to install packages that are also defined as a dependency in manifest.toml and error out if the dependency is missing in manifest.toml.

I didn't dive this deep to find whether the following problem is solved already:

eauchat commented 6 months ago

As @chri2 requested some comments, this is what I can say. (I didn't understand everything you mention in this issue, so hope my comments here will be relevant.)

Sound great to have yunohost handles installing dependencies like couchdb without needing to make a package for it. I don't have much to say about the installing process because I don't know well enough how yunohost handles dependencies, but I imagine there's a way to make it simple and reliable. The thing to take into consideration after install will be how couchdb settings are customized. At the moment couchdb_ynh settings customization is happening here and adds these settings. While for example a package like dato customize couchdb config here adding those settings.

So both of them add some files in /opt/couchdb/etc/local.d/, which I imagine wouldn't be so hard for yunohost to handle (I imagine it already does). I don't know how couchdb handles priorities between these files if they have conflicting settings. This is were issues may arise when multiple apps customize couchdb settings with conflicting needs.

As far as I know, those are the only issues to take into consideration for dato_ynh and couchdb_ynh.

chri2 commented 6 months ago

Had just this thought: Wouldn't it be possible to automatically install a dpkg without configuring (leaving it unconfigured or using any defaults the package proposes) and leave it to the scripts to configure it later?