[bug#47704] [PATCH] services: mysql: Add extra-environment as configuration option.

[david larsson]

> > 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 ...)

I don't think this is a nice solution because for every update to the 
mysql package would mean to verify that we are actually hitting a bunch 
of spread out invocations of external programs inside those shell 
scripts with the regex in the (substitute* .. procedure. I would suggest 
instead to define a variable like say: (define 
%default-mysqld-environment #~(list (string-append #$gawk "/bin/awk") 
..) which contains all the external commands invoked from the shell 
scripts in the mysql/bin folder, and append that to the 
extra-environment list.

> >  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
>
> This shouldn't be complicated, though possibly somewhat tedious.
> I took a look at
> /gnu/store/bjgz8jnfsbb4qvaa9csfy8i3x1i3ivp7-mariadb-10.5.8/bin/wsrep_*
> (your hash may vary).  The following should be ‘absolutised’:
>
> * In wsrep_sst_mariabackup:
>
>   OS=$(uname)
>   sfmt="tar"
>   if pv --help 2>/dev/null | grep -q FORMAT;then
>   mariabackup
>   wsrep_log_error
>   mbstream
>   xbcrypt (*)
>   [more things]
> * In wsrep_sst_*: other things (eg. rm)
>
> The following shouldn't be required, and could be commented out:
> # Setting the path for lsof on CentOS
> export PATH="/usr/sbin:/sbin:$PATH"
>
> It's a little tedious, but it should be worth it.  This could be done
> in the post-install phase of mariadb.

I find this to be too tedious, and since it introduces maintenance 
hassle I would prefer my suggestion above.

> For an (almost) good example on
> how to ‘absolutise’, see 'xvfb-run'.  Actually, it uses "which" which
> is incorrect when cross-compiling, but that can be worked-around by
> adding (setenv "PATH" (string-append BINDIR-OF-INPUTS-COREUTILS ":"
>                                      BINDIR-OF-INPUTS-AWK ...))
>
> About xbcrypt (*): I have no idea from which package this comes.
> Is it an optional dependency?  If it is optional, _not_ patching it
> could make sense, as to avoid increasing the closure.  This would
> extra-environment.

[..]

> That said, if that's too much work or too error-prone, I've an 
> alternative
> proposal below, which is a little more ‘high level’ than asking the 
> user to
> spell out the PATH:
>
> Take a look at 'nscd-shepherd-service' in gnu/serices/base.scm:
>
>   [snip]
>   #:environment-variables
>   (list (string-append "LD_LIBRARY_PATH="
>           (string-join
>             (map (lambda (dir)
>               (string-append dir "/lib"))
>             (list #$@name-services))
>             ":")))))
>
> Replace LD_LIBRARY_PATH with PATH.  Add a "extensions" field to
> <mysql-configuration>.  Replace 'name-services' with 'extensions'.

Not sure if you mean to keep /lib in the above procedure and use this 
for Galera's .so file, or to help set the PATH for the external programs 
invoked from the shell scripts?

AFAIK the galera service requires you to specify the full path to the 
.so either way, and if you meant to use /bin instead of /lib above; the 
binaries of the external programs that are needed are sometimes in 
out/sbin and sometimes in out/bin so this would unfortunately miss 
programs occasionally.

> Procedures you need to modify:
>   * mysql-cofiguration-file: add the field to $ <mysql-configuration>
>   * mysql-shepherd-service: add #:environment-variables as appropriate
>   * mysql-upgrade-shepherd-service: also, maybe, I don't know?
>
> Possible problems: maybe some scripts need some additional environment
> variables.  Mabe provide both an "extensions" field, and an
> "extra-environment" field, and combine the results?

Possibly. What would you say about opening a separate bug report and 
potentially fix those things in separate PATCH-set? The mysql-upgrade 
service in particular is something I don't know whether it could benefit 
from adding some things to the PATH.

> For the patch as you've submitted it, the lack of a system test 
> shouldn't
> be a problem, as Guix System doesn't support mariadb + galera.  The 
> system
> test is more for if ‘we’ add a 'galera' field to 'mysql-configuration' 
> as
> I suggested.

Great!

> > 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.
>
> Somewhat off-topic, which e-mails would these be?

This one for example: 
https://lists.gnu.org/archive/html/bug-guix/2020-02/msg00321.html
But I would also assume that some of the interest in using 
network-manager's vpn plugin is due to it being hard to cover all the 
options in openvpn-client-configuration. (There was also an issue with 
cert and key options being mandatory in openvpn-client-configuration)


> Agreed.  I believe the current plan is to:
>
> * add the 'extra-environment' escape hatch.
> * (possibly [dubious-discuss]) Add the extensions field (a list of
>   package objects).  This adds the listed packages to PATH.
>   Slightly neater than 'extra-environment', but is not as general
>   as 'extra-environment'.
> * The contents of 'extra-environment' and and 'extensions' are merged.

Can we do or discuss these things in a separately filed BUG report or 
PATCH?

Best regards,
David