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

[Maxime Devos]

On Mon, 2021-04-12 at 20:06 +0200, david larsson wrote:
> > > 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.

What about inserting a (string-append "export PATH=" #$(file-apend gawk 
"/bin/awk") ...)
line in the scripts?  Some of the scripts even already have a PATH=... line
(PATH=/bin:/sbin:/usr/bin or something like that).

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

Fair enough (-:.

> > [...]
> > 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?

I meant for help setting 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.

I guess both out/bin and out/sbin could be added to the PATH.

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

mysql-configuration-file deconstruct the <mysql-configuration> struct.  I'm not
100% sure, but I think when you have a (match obj (($ <record> field ...) ...))
expression, and you add a field to <record>, you need to add the new field to
$ <record> field ..., in the same order.

mysql-shepherd-service: no need to explain, you have modified it in your patch
to pass #:enviroment-variables #$extra-env.

mysql-upgrade-service: I don't know either.

I'm not going to write a patch.  I don't have a mysql service on my system;
it was only a suggestion.

> [...]
> 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)
That's an informtive example, thanks!

> 
> > 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?

These things = the 'extensions' field, and merging 'extra-environment' and
'extensions'?  As I said, it was only a suggestion and I won't be working on it.
I think your original patch is good to go into the git repo.  I'll open a
separate bug report about ‘absolutising’ the binaries referred to from the 
scripts.

Note: I do not have commit access.

Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part