From: Maxime Devos <maximedevos@telenet.be>
To: david larsson <david.larsson@selfhosted.xyz>
Cc: 47704@debbugs.gnu.org
Subject: [bug#47704] [PATCH] services: mysql: Add extra-environment as configuration option.
Date: Mon, 12 Apr 2021 22:09:14 +0200 [thread overview]
Message-ID: <60efaa4ac56f3dda7da44f0e60df921ccdd7bc89.camel@telenet.be> (raw)
In-Reply-To: <701e03466beb09a26cc25e444382d14e@selfhosted.xyz>
[-- Attachment #1: Type: text/plain, Size: 5273 bytes --]
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.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
next prev parent reply other threads:[~2021-04-12 20:10 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-11 8:44 [bug#47704] [PATCH] services: mysql: Add extra-environment as configuration option david larsson
2021-04-11 15:33 ` Maxime Devos
2021-04-11 18:07 ` david larsson
2021-04-11 20:44 ` Maxime Devos
2021-04-12 18:06 ` david larsson
2021-04-12 20:09 ` Maxime Devos [this message]
2021-04-13 16:58 ` bug#47704: " Leo Prikler
2021-04-13 22:29 ` [bug#47704] " Julien Lepiller
2021-04-13 22:38 ` Leo Prikler
2021-04-13 22:56 ` Julien Lepiller
2021-04-19 9:59 ` Leo Prikler
2021-04-27 18:15 ` david larsson
2021-04-27 18:47 ` Leo Prikler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=60efaa4ac56f3dda7da44f0e60df921ccdd7bc89.camel@telenet.be \
--to=maximedevos@telenet.be \
--cc=47704@debbugs.gnu.org \
--cc=david.larsson@selfhosted.xyz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).