[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#47704] [PATCH] services: mysql: Add extra-environment as configurat

From: david larsson
Subject: [bug#47704] [PATCH] services: mysql: Add extra-environment as configuration option.
Date: Sun, 11 Apr 2021 20:07:15 +0200

Hi Maxime!

On 2021-04-11 17:33, Maxime Devos wrote:
Please corect the galera package to refer to the coreutils (ls, stat, ...)
by absolute file name instead, using something like

(add-after 'install
  (substitute* "INSTALL-LOCATION/wsrep_sst_rsync"
(("\\bls\\b") (string-append (assoc-ref inputs "coreutils") "/bin/ls"))

(Likewise for rsync, gawk, iproute ...)

Don't use (which "ls") instead of string-append + assoc-ref! (which "ls") is
incorrect when cross-compiling;

That way, people don't have to fiddle with PATH in their configuration file.

I think you misundestood here - these rsync, gawk, iproute etc are executed as part of scripts in the mysqld/bin package output folder (see `ls -la $(dirname $(readlink -f $(which mysqld)))/` ) because the Galera add-on was (at least partially) merged into the regular mariadb sources, so the scripts where the binaries are needed are not from the galera package outputs but the mysql one. The galera package in guix master today only provides the #$galera "/lib/ file. I agree it would be nice to patch all the shell scripts in the $#mysql/bin folder but this would 1. require maintaining all the additional patching of those scripts as part of the mysql package and 2. since custom wsrep_sst_<X> shell scripts are possible and are usually added to /usr/bin/wsrep_sst_<X> those binaries will also require setting additional environment variables or the same type of patching (which would therefore "require" this patch to make it easier).

If possible, consider writing a "system test" automatically testing
some very basic functionality of mariadb + galera (gnu/tests/databases.scm).

I would have liked to, but Im sorry - I do not have the time/skill for this for the time being. I hope the current patch is simple enough that an additional test is not currently needed.

I ran these commands to test:
guix pull --url=/home/user1/src/guix --profile=/tmp/guix.master
--disable-authentication --allow-downgrades ;
GUIX_PROFILE="/tmp/guix.master" ; . "$GUIX_PROFILE/etc/profile" ; guix
system reconfigure config.scm --fallback --allow-downgrades


(extra-environment #~(list ...


It seems extra-environment is still useful. "USER=mysql" should probably be
added automatically, though (see my proposal below).

Yes, in particular for custom wsrep scripts. The "USER=mysql" may not be needed per se for the example snippet config I sent earlier as I did not make sure that the list of environment variables there were the minimal amount needed; I mainly hoped to provide a repeatable example of something to test with.

Perhaps you could extend "mysql-configuration" with a "galera" field
(with #f as default)?  Theoretical example:

  ;; [...] other fields
    (package my-version-of-galera) ; optional
    (cluster-name "test_cluster")
    (cluster-address "gcom://...")
    (synchronization-method 'rsync)
    (node-adress "redacted")
    (node-name "librem13v3guixsd")))

.. and modify mysql-service-type to insert appropriate configuration entries and perhaps add things to the PATH of the shepherd service as appropriate.

Escape hatches like "extra-content" are useful, but this seems a bit

This looks nice! I agree that something like what you show here is the better option, at the same time I don't have the time/skill to provide a patch with a well-featured interface to the galera options for the time being. The "escape hatch" would therefore be very useful for now, and less hassle to maintain - compare with for example the vpn service and the amount of emails in the lists regarding lack of options that could have easily been added via some g-expression strings. In general I don't see the harm in providing both "the escape hatch" way to add options to a configuration file and the guile interface which is otherwise nicer (IMO).


Please someone also review [bug#47517] [PATCH] gnu: nginx: Enable stream

I'll take a look at it.


Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]