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

  • Done
  • quality assurance status badge
Details
4 participants
  • david larsson
  • Julien Lepiller
  • Leo Prikler
  • Maxime Devos
Owner
unassigned
Submitted by
david larsson
Severity
normal
D
D
david larsson wrote on 11 Apr 2021 10:44
(address . guix-patches@gnu.org)
91c1726e12b938f7656d7d7862920f69@selfhosted.xyz
Hi!
This patch is needed for the Galera add-on to MariaDB, which runs some
scripts like for example wsrep_sst_rsync that needs access to additional
binaries in PATH.

I tested the patch with (and without) below snippets to the
mysql-service in my config.scm and successfully connected to a
MariaDB/Galera cluster.

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 (string-append "PATH=/usr/bin:/bin:" #$rsync
"/bin:" #$coreutils "/bin:" #$gawk "/bin:" #$grep "/bin:" #$mariadb
"/bin:" #$iproute "/sbin:"
"/run/setuid-programs:/run/current-system/profile/bin:/run/current-system/profile/sbin"
) (string-append "SHELL=" #$bash) "USER=mysql"
"SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt"
"SSL_CERT_DIR=/run/current-system/profile/etc/ssl/certs"))


(extra-content #~(string-append "log_error=/var/lib/mysql/log_error.log
#
binlog_format=ROW
default-storage-engine=innodb
innodb_autoinc_lock_mode=2

# Galera Provider Configuration
wsrep_on=ON
wsrep_provider=" #$galera "/lib/libgalera_smm.so

# Galera Cluster Configuration
wsrep_cluster_name=\"test_cluster\"
wsrep_cluster_address=\"gcomm://redacted,redacted\"
# according to
# leaving it empty starts a new cluster, so you should immediately
reconfigure again after doing this.
#wsrep_cluster_address=\"gcomm://\"

# Galera Synchronization Configuration
wsrep_sst_method=rsync

# Galera Node Configuration
wsrep_node_address=\"redacted\"
wsrep_node_name=\"librem13v3guixsd\""))
))

------------------------------------------------------------------

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

which adds support for tcp loadbalancing that can be used to scale a
MariaDB/Galera cluster.

Best regards,
David
From 927444652f4b774928de78c1558b4e942abff203 Mon Sep 17 00:00:00 2001
From: methuselah-0 <david.larsson@selfhosted.xyz>
Date: Sun, 11 Apr 2021 10:30:01 +0200
Subject: [PATCH] services: mysql: Add extra-environment as configuration
option.

* gnu/services/databases.scm (mysql-configuration): Add extra-environment
(mysql-service): Use #:log-file and #:environment-variables

* doc/guix.texi: Document it.
---
doc/guix.texi | 3 +++
gnu/services/databases.scm | 7 ++++++-
2 files changed, 9 insertions(+), 1 deletion(-)

Toggle diff (52 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index c23d044ff5..298585a695 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19809,6 +19809,9 @@ Socket file to use for local (non-network) connections.
@item @code{extra-content} (default: @code{""})
Additional settings for the @file{my.cnf} configuration file.
+@item @code{extra-environment} (default: @code{#~'()})
+List of environment variables passed to the @command{mysqld} process.
+
@item @code{auto-upgrade?} (default: @code{#t})
Whether to automatically run @command{mysql_upgrade} after starting the
service. This is necessary to upgrade the @dfn{system schema} after
diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index a841e7a50e..8d266c1cba 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -7,6 +7,7 @@
;;; Copyright © 2018 Julien Lepiller <julien@lepiller.eu>
;;; Copyright © 2019 Robert Vollmert <rob@vllmrt.net>
;;; Copyright © 2020 Marius Bakke <marius@gnu.org>
+;;; Copyright © 2021 David Larsson <david.larsson@selfhosted.xyz>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -527,6 +528,7 @@ created after the PostgreSQL database is started.")))
(port mysql-configuration-port (default 3306))
(socket mysql-configuration-socket (default "/run/mysqld/mysqld.sock"))
(extra-content mysql-configuration-extra-content (default ""))
+ (extra-environment mysql-configuration-extra-environment (default #~'()))
(auto-upgrade? mysql-configuration-auto-upgrade? (default #t)))
(define %mysql-accounts
@@ -611,11 +613,14 @@ FLUSH PRIVILEGES;
(provision '(mysql))
(documentation "Run the MySQL server.")
(start (let ((mysql (mysql-configuration-mysql config))
+ (extra-env (mysql-configuration-extra-environment config))
(my.cnf (mysql-configuration-file config)))
#~(make-forkexec-constructor
(list (string-append #$mysql "/bin/mysqld")
(string-append "--defaults-file=" #$my.cnf))
- #:user "mysql" #:group "mysql")))
+ #:user "mysql" #:group "mysql"
+ #:log-file "/var/log/mysqld.log"
+ #:environment-variables #$extra-env)))
(stop #~(make-kill-destructor)))))
(define (mysql-upgrade-wrapper mysql socket-file)
--
2.30.2
M
M
Maxime Devos wrote on 11 Apr 2021 17:33
dbec1cbdca95fa8997f37ae8455b80b2bca25546.camel@telenet.be
On Sun, 2021-04-11 at 10:44 +0200, david larsson wrote:
Toggle quote (10 lines)
> Hi!
> This patch is needed for the Galera add-on to MariaDB, which runs some
> scripts like for example wsrep_sst_rsync that needs access to additional
> binaries in PATH.
> [... reordered ...]
> (extra-environment #~(list (string-append "PATH=/usr/bin:/bin:" #$rsync
> "/bin:" #$coreutils "/bin:" #$gawk "/bin:" #$grep "/bin:" #$mariadb
> "/bin:" #$iproute "/sbin:"
> "/

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.

Toggle quote (4 lines)
> I tested the patch with (and without) below snippets to the
> mysql-service in my config.scm and successfully connected to a
> MariaDB/Galera cluster.

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

Toggle quote (14 lines)
> 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 ...
>
> "USER=mysql"
> "SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt"
> "SSL_CERT_DIR=/run/current-system/profile/etc/ssl/certs"))

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

Toggle quote (29 lines)
>
> (extra-content #~(string-append "log_error=/var/lib/mysql/log_error.log
> #
> https://www.percona.com/blog/2017/07/26/what-is-innodb_autoinc_lock_mode-and-why-should-i-care/
> binlog_format=ROW
> default-storage-engine=innodb
> innodb_autoinc_lock_mode=2
>
> # Galera Provider Configuration
> wsrep_on=ON
> wsrep_provider=" #$galera "/lib/libgalera_smm.so
>
> # Galera Cluster Configuration
> wsrep_cluster_name=\"test_cluster\"
> wsrep_cluster_address=\"gcomm://redacted,redacted\"
> # according to
> https://galeracluster.com/library/documentation/mysql-wsrep-options.html
> # leaving it empty starts a new cluster, so you should immediately
> reconfigure again after doing this.
> #wsrep_cluster_address=\"gcomm://\"
>
> # Galera Synchronization Configuration
> wsrep_sst_method=rsync
>
> # Galera Node Configuration
> wsrep_node_address=\"redacted\"
> wsrep_node_name=\"librem13v3guixsd\""))
> ))

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

(mysql-configuration
(port A-DIFFERENT-PORT)
;; [...] other fields
(galera
(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
neater.

Toggle quote (5 lines)
> ------------------------------------------------------------------
>
> Please someone also review [bug#47517] [PATCH] gnu: nginx: Enable stream
> module

I'll take a look at it.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYHMWrRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7ktzAP9Kkf6RoBHbkGmFNTbwbC940xhH
V8t0U4rW8DjiwuH0WAD/UgcUarIcTtNxZS86B+sl+P0I/h7gfZxdWPqE8tcnJww=
=JcEP
-----END PGP SIGNATURE-----


D
D
david larsson wrote on 11 Apr 2021 20:07
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 47704@debbugs.gnu.org)
7d2d1250a3e87ac67c80897bffe0b82c@selfhosted.xyz
Hi Maxime!

On 2021-04-11 17:33, Maxime Devos wrote:
Toggle quote (19 lines)
> 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/libgalera_smm.so 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).

Toggle quote (4 lines)
> 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.

Toggle quote (18 lines)
>> 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 ...
>>
>> "USER=mysql"
>> "SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt"
>> "SSL_CERT_DIR=/run/current-system/profile/etc/ssl/certs"))
>
> 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.

Toggle quote (22 lines)
> Perhaps you could extend "mysql-configuration" with a "galera" field
> (with #f as default)? Theoretical example:
>
> (mysql-configuration
> (port A-DIFFERENT-PORT)
> ;; [...] other fields
> (galera
> (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
> neater.

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

Toggle quote (8 lines)
>> ------------------------------------------------------------------
>>
>> Please someone also review [bug#47517] [PATCH] gnu: nginx: Enable
>> stream
>> module
>
> I'll take a look at it.

Thanks!

Best regards,
David
M
M
Maxime Devos wrote on 11 Apr 2021 22:44
(name . david larsson)(address . david.larsson@selfhosted.xyz)(address . 47704@debbugs.gnu.org)
7cfafd05c98540590905ceb5f3cd554fc9e2b79b.camel@telenet.be
On Sun, 2021-04-11 at 20:07 +0200, david larsson wrote:
Toggle quote (25 lines)
> 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

Ok, I should have asked to modify the mariadb package instead.

Toggle quote (7 lines)
> (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/libgalera_smm.so file.

So the mariadb package has the scripts, and the galera package has a library.

Toggle quote (5 lines)
> 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. 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.
Toggle quote (6 lines)
> 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).

I'd recommend that these custom shell scripts are patched as well,
but idk how they would be used with mariadb, perhaps there are
some complications here.

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

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?

Toggle quote (8 lines)
> > 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.

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.

Toggle quote (11 lines)
> > 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.

Warning: I didn't actually test your patch. I don't have a mysql service.

Toggle quote (21 lines)
> > Perhaps you could extend "mysql-configuration" with a "galera" field
> > (with #f as default)? Theoretical example:
> >
> > (mysql-configuration [...] (galera [...]))
>
> > .. 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
> > neater.
>
> 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.

Somewhat off-topic, which e-mails would these be?

Toggle quote (5 lines)
> 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).

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.

Toggle quote (2 lines)
> Best regards,
> David
^W^W^W^W^W Maxime
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYHNfmRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7t3HAP93dk3g0LqnHS4M2eSHlTNxIvUI
SDLmLDY9ANYNLptPUAEA1W1G/6CJ+Mk7/whluJgTq9SHJnZ0LLCR2/3C3RQZ+w4=
=1nDU
-----END PGP SIGNATURE-----


D
D
david larsson wrote on 12 Apr 2021 20:06
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 47704@debbugs.gnu.org)
701e03466beb09a26cc25e444382d14e@selfhosted.xyz
Toggle quote (13 lines)
>> 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.

Toggle quote (29 lines)
>> 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.

Toggle quote (11 lines)
> 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.

[..]

Toggle quote (20 lines)
> 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.

Toggle quote (9 lines)
> 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.

Toggle quote (8 lines)
> 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!

Toggle quote (7 lines)
>> 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:
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)


Toggle quote (9 lines)
> 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
M
M
Maxime Devos wrote on 12 Apr 2021 22:09
(name . david larsson)(address . david.larsson@selfhosted.xyz)(address . 47704@debbugs.gnu.org)
60efaa4ac56f3dda7da44f0e60df921ccdd7bc89.camel@telenet.be
On Mon, 2021-04-12 at 20:06 +0200, david larsson wrote:
Toggle quote (23 lines)
> > > 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).

Toggle quote (3 lines)
> I find this to be too tedious, and since it introduces maintenance
> hassle I would prefer my suggestion above.

Fair enough (-:.

Toggle quote (19 lines)
> > [...]
> > 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.

Toggle quote (6 lines)
> 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.

Toggle quote (14 lines)
> > 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.

Toggle quote (7 lines)
> [...]
> 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!

Toggle quote (13 lines)
>
> > 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.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYHSo6hccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7q13AP9rfOcbfRSxrJ/IrUD5m0AknBXT
11QFKQ/ZZL9txDfhrQD+M/G0DWsn/FKpGkmaLuG/pYOTq+hDCJPSH6AQyE2RBQg=
=PgDB
-----END PGP SIGNATURE-----


L
L
Leo Prikler wrote on 13 Apr 2021 18:58
(address . 47704-done@debbugs.gnu.org)
7a570acae3d60b16e34a89097d801876d5951f4e.camel@student.tugraz.at
Am Montag, den 12.04.2021, 22:09 +0200 schrieb Maxime Devos:
Toggle quote (4 lines)
> 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.
I've pushed this patch now, but let us still look for a smaller
solution if applicable. (That said, I'm not a mysql user and I'm happy
to leave security stuff to lle_bout.)

@david: Note, that I did not change the author, meaning it is committed
as "methuselah-0 <david.larsson@selfhosted.xyz>" rather than
"david larsson <david.larsson@selfhosted.xyz>". Since this patch is
hopefully small enough to not require attribution, that is fine, but if
you plan on making bigger changes, please consider setting your git up
appropriately.

Regards,
Leo
Closed
J
J
Julien Lepiller wrote on 14 Apr 2021 00:29
Re: bug#47704: [PATCH] services: mysql: Add extra-environment as configuration option.
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
20210414002958.2c5a6e19@tachikoma.lepiller.eu
Le Tue, 13 Apr 2021 18:58:57 +0200,
Leo Prikler <leo.prikler@student.tugraz.at> a écrit :

Toggle quote (23 lines)
> Am Montag, den 12.04.2021, 22:09 +0200 schrieb Maxime Devos:
> > 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.
> I've pushed this patch now, but let us still look for a smaller
> solution if applicable. (That said, I'm not a mysql user and I'm
> happy to leave security stuff to lle_bout.)
>
> @david: Note, that I did not change the author, meaning it is
> committed as "methuselah-0 <david.larsson@selfhosted.xyz>" rather
> than "david larsson <david.larsson@selfhosted.xyz>". Since this
> patch is hopefully small enough to not require attribution, that is
> fine, but if you plan on making bigger changes, please consider
> setting your git up appropriately.
>
> Regards,
> Leo
>
>
>
>

Hi!

I saw this patch went through, but it updates the Guix Manual while we
are in string freeze. The change is very small and close to the start
of string freeze, so I'm willing to let it go. However, please refrain
from pushing such changes to the manual. Ideally, this patch should
have been pushed yesterday or postponed until after the release.

As a reminder, the string freeze applies to guix and to the manual. It
does not apply to the website, package synopsis and descriptions, nor
to the cookbook.

Thank you!
L
L
Leo Prikler wrote on 14 Apr 2021 00:38
(name . Julien Lepiller)(address . julien@lepiller.eu)(address . 47704@debbugs.gnu.org)
8202cabb845f308220a32c28c88bc29f75fd1364.camel@student.tugraz.at
Am Mittwoch, den 14.04.2021, 00:29 +0200 schrieb Julien Lepiller:
Toggle quote (43 lines)
> Le Tue, 13 Apr 2021 18:58:57 +0200,
> Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
>
> > Am Montag, den 12.04.2021, 22:09 +0200 schrieb Maxime Devos:
> > > 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.
> > I've pushed this patch now, but let us still look for a smaller
> > solution if applicable. (That said, I'm not a mysql user and I'm
> > happy to leave security stuff to lle_bout.)
> >
> > @david: Note, that I did not change the author, meaning it is
> > committed as "methuselah-0 <david.larsson@selfhosted.xyz>" rather
> > than "david larsson <david.larsson@selfhosted.xyz>". Since this
> > patch is hopefully small enough to not require attribution, that is
> > fine, but if you plan on making bigger changes, please consider
> > setting your git up appropriately.
> >
> > Regards,
> > Leo
> >
> >
> >
> >
>
> Hi!
>
> I saw this patch went through, but it updates the Guix Manual while
> we
> are in string freeze. The change is very small and close to the start
> of string freeze, so I'm willing to let it go. However, please
> refrain
> from pushing such changes to the manual. Ideally, this patch should
> have been pushed yesterday or postponed until after the release.
>
> As a reminder, the string freeze applies to guix and to the manual.
> It
> does not apply to the website, package synopsis and descriptions, nor
> to the cookbook.
>
> Thank you!
Ahh, my bad, I totally missed that. Would it be better to revert this
change until April 18th and then apply it again?
J
J
Julien Lepiller wrote on 14 Apr 2021 00:56
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)(address . 47704@debbugs.gnu.org)
20210414005627.6b900925@tachikoma.lepiller.eu
Le Wed, 14 Apr 2021 00:38:50 +0200,
Leo Prikler <leo.prikler@student.tugraz.at> a écrit :

Toggle quote (48 lines)
> Am Mittwoch, den 14.04.2021, 00:29 +0200 schrieb Julien Lepiller:
> > Le Tue, 13 Apr 2021 18:58:57 +0200,
> > Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
> >
> > > Am Montag, den 12.04.2021, 22:09 +0200 schrieb Maxime Devos:
> > > > 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.
> > > I've pushed this patch now, but let us still look for a smaller
> > > solution if applicable. (That said, I'm not a mysql user and I'm
> > > happy to leave security stuff to lle_bout.)
> > >
> > > @david: Note, that I did not change the author, meaning it is
> > > committed as "methuselah-0 <david.larsson@selfhosted.xyz>" rather
> > > than "david larsson <david.larsson@selfhosted.xyz>". Since this
> > > patch is hopefully small enough to not require attribution, that
> > > is fine, but if you plan on making bigger changes, please consider
> > > setting your git up appropriately.
> > >
> > > Regards,
> > > Leo
> > >
> > >
> > >
> > >
> >
> > Hi!
> >
> > I saw this patch went through, but it updates the Guix Manual while
> > we
> > are in string freeze. The change is very small and close to the
> > start of string freeze, so I'm willing to let it go. However, please
> > refrain
> > from pushing such changes to the manual. Ideally, this patch should
> > have been pushed yesterday or postponed until after the release.
> >
> > As a reminder, the string freeze applies to guix and to the manual.
> > It
> > does not apply to the website, package synopsis and descriptions,
> > nor to the cookbook.
> >
> > Thank you!
> Ahh, my bad, I totally missed that. Would it be better to revert this
> change until April 18th and then apply it again?
>

Yeah, it would be nicer for our translators if you could revert the
change for now. Can you do it?
L
L
Leo Prikler wrote on 19 Apr 2021 11:59
(name . Julien Lepiller)(address . julien@lepiller.eu)(address . 47704@debbugs.gnu.org)
39c4166f7f0e7a74aa900f92856d7b9a95b5fe40.camel@student.tugraz.at
Am Mittwoch, den 14.04.2021, 00:56 +0200 schrieb Julien Lepiller:
Toggle quote (35 lines)
> Le Wed, 14 Apr 2021 00:38:50 +0200,
> Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
>
> > Am Mittwoch, den 14.04.2021, 00:29 +0200 schrieb Julien Lepiller:
> > > Le Tue, 13 Apr 2021 18:58:57 +0200,
> > > Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
> > >
> > > [...]
> > > Hi!
> > >
> > > I saw this patch went through, but it updates the Guix Manual
> > > while
> > > we
> > > are in string freeze. The change is very small and close to the
> > > start of string freeze, so I'm willing to let it go. However,
> > > please
> > > refrain
> > > from pushing such changes to the manual. Ideally, this patch
> > > should
> > > have been pushed yesterday or postponed until after the release.
> > >
> > > As a reminder, the string freeze applies to guix and to the
> > > manual.
> > > It
> > > does not apply to the website, package synopsis and descriptions,
> > > nor to the cookbook.
> > >
> > > Thank you!
> > Ahh, my bad, I totally missed that. Would it be better to revert
> > this
> > change until April 18th and then apply it again?
> >
>
> Yeah, it would be nicer for our translators if you could revert the
> change for now. Can you do it?
What's the current status w.r.t. string freeze and translations? Is
this good to go?
D
D
david larsson wrote on 27 Apr 2021 20:15
Re: [bug#47704] [PATCH] services: mysql: Add extra-environment as configuration option.
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
01e11520eb02c2c3fedf639a778e24e2@selfhosted.xyz
On 2021-04-19 11:59, Leo Prikler wrote:
Toggle quote (39 lines)
> Am Mittwoch, den 14.04.2021, 00:56 +0200 schrieb Julien Lepiller:
>> Le Wed, 14 Apr 2021 00:38:50 +0200,
>> Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
>>
>> > Am Mittwoch, den 14.04.2021, 00:29 +0200 schrieb Julien Lepiller:
>> > > Le Tue, 13 Apr 2021 18:58:57 +0200,
>> > > Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
>> > >
>> > > [...]
>> > > Hi!
>> > >
>> > > I saw this patch went through, but it updates the Guix Manual
>> > > while
>> > > we
>> > > are in string freeze. The change is very small and close to the
>> > > start of string freeze, so I'm willing to let it go. However,
>> > > please
>> > > refrain
>> > > from pushing such changes to the manual. Ideally, this patch
>> > > should
>> > > have been pushed yesterday or postponed until after the release.
>> > >
>> > > As a reminder, the string freeze applies to guix and to the
>> > > manual.
>> > > It
>> > > does not apply to the website, package synopsis and descriptions,
>> > > nor to the cookbook.
>> > >
>> > > Thank you!
>> > Ahh, my bad, I totally missed that. Would it be better to revert
>> > this
>> > change until April 18th and then apply it again?
>> >
>>
>> Yeah, it would be nicer for our translators if you could revert the
>> change for now. Can you do it?
> What's the current status w.r.t. string freeze and translations? Is
> this good to go?

Ping! :-)
L
L
Leo Prikler wrote on 27 Apr 2021 20:47
(name . david larsson)(address . david.larsson@selfhosted.xyz)
e08e36579a210d584b9f1211473fff4df6d58c96.camel@student.tugraz.at
Am Dienstag, den 27.04.2021, 20:15 +0200 schrieb david larsson:
Toggle quote (48 lines)
> On 2021-04-19 11:59, Leo Prikler wrote:
> > Am Mittwoch, den 14.04.2021, 00:56 +0200 schrieb Julien Lepiller:
> > > Le Wed, 14 Apr 2021 00:38:50 +0200,
> > > Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
> > >
> > > > Am Mittwoch, den 14.04.2021, 00:29 +0200 schrieb Julien
> > > > Lepiller:
> > > > > Le Tue, 13 Apr 2021 18:58:57 +0200,
> > > > > Leo Prikler <leo.prikler@student.tugraz.at> a écrit :
> > > > >
> > > > > [...]
> > > > > Hi!
> > > > >
> > > > > I saw this patch went through, but it updates the Guix Manual
> > > > > while
> > > > > we
> > > > > are in string freeze. The change is very small and close to
> > > > > the
> > > > > start of string freeze, so I'm willing to let it go. However,
> > > > > please
> > > > > refrain
> > > > > from pushing such changes to the manual. Ideally, this patch
> > > > > should
> > > > > have been pushed yesterday or postponed until after the
> > > > > release.
> > > > >
> > > > > As a reminder, the string freeze applies to guix and to the
> > > > > manual.
> > > > > It
> > > > > does not apply to the website, package synopsis and
> > > > > descriptions,
> > > > > nor to the cookbook.
> > > > >
> > > > > Thank you!
> > > > Ahh, my bad, I totally missed that. Would it be better to
> > > > revert
> > > > this
> > > > change until April 18th and then apply it again?
> > > >
> > >
> > > Yeah, it would be nicer for our translators if you could revert
> > > the
> > > change for now. Can you do it?
> > What's the current status w.r.t. string freeze and
> > translations? Is
> > this good to go?
>
> Ping! :-)
Pushed once again, thanks for the reminder!
?
Your comment

This issue is archived.

To comment on this conversation send an email to 47704@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 47704
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch