Message ID | 9ac2df37867548ae5e15e4c262458d8fc63ba9a1.1715989953.git.maxim.cournoyer@gmail.com |
---|---|
State | New |
Headers | show |
Series | [bug#71022] configure.ac: Set default value for the 'prefix' variable. | expand |
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > -When configuring Guix on a system that already has a Guix installation, > -be sure to specify the same state directory as the existing installation > -using the @option{--localstatedir} option of the @command{configure} > -script (@pxref{Directory Variables, @code{localstatedir},, standards, > -GNU Coding Standards}). Usually, this @var{localstatedir} option is set > -to the value @file{/var}. The @command{configure} script protects > -against unintended misconfiguration of @var{localstatedir} so you do not > -inadvertently corrupt your store (@pxref{The Store}). The configuration > -directory should also be configured by setting the @option{--sysconfdir} > -option to the @file{/etc} value, which is the location used by Guix to > -store for example the access control list of authorized machines and the > -definition of offload machines. This information is still useful, isn't it? It's important for people who intend to modify these locations, such as system administrators who might want to install things elsewhere. Perhaps this could be rewritten to explain why this would be a bad idea.
Hi Maxim, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > The Guix standard configuration uses a localstatedir of /var and a sysconfdir > of /etc. To ease things for everyone, make the default values match that > standard expected configuration. See > <https://lists.gnu.org/archive/html/guix-devel/2024-05/msg00003.html> for a > related discussion. > > * configure.ac: Default $prefix to '' unless already set. > * doc/contributing.texi (Building from Git): Streamline doc. > * doc/guix-cookbook.texi (Guix environment via direnv): Likewise. > > Change-Id: I23cd12b58a842d246fbc9fdc740311c573eb0212 [...] > +dnl Set some sane default directory variables for use with the Guix. This > +dnl also causes localstatedir to be /var and sysconfdir to be /etc. > +test "$prefix" = NONE && prefix= Shouldn’t it read “prefix=/”? Might be good to check whether this breaks “make distcheck”, so I suppose it should be fine. Apart from that it LGTM. Thanks for this patch! Ludo’.
Hi Ricardo, Ricardo Wurmus <rekado@elephly.net> writes: > Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > >> -When configuring Guix on a system that already has a Guix installation, >> -be sure to specify the same state directory as the existing installation >> -using the @option{--localstatedir} option of the @command{configure} >> -script (@pxref{Directory Variables, @code{localstatedir},, standards, >> -GNU Coding Standards}). Usually, this @var{localstatedir} option is set >> -to the value @file{/var}. The @command{configure} script protects >> -against unintended misconfiguration of @var{localstatedir} so you do not >> -inadvertently corrupt your store (@pxref{The Store}). The configuration >> -directory should also be configured by setting the @option{--sysconfdir} >> -option to the @file{/etc} value, which is the location used by Guix to >> -store for example the access control list of authorized machines and the >> -definition of offload machines. > > This information is still useful, isn't it? It's important for people > who intend to modify these locations, such as system administrators who > might want to install things elsewhere. Perhaps this could be rewritten > to explain why this would be a bad idea. It's useful, but it's duplicated with the warning you'd get from the GUIX_CHECK_LOCALSTATEDIR m4/guix.m4 macro: --8<---------------cut here---------------start------------->8--- dnl GUIX_CHECK_LOCALSTATEDIR dnl dnl Check that the LOCALSTATEDIR value is consistent with that of the existing dnl Guix installation, if any. Error out or warn if they do not match. AC_DEFUN([GUIX_CHECK_LOCALSTATEDIR], [ AC_REQUIRE([GUIX_CURRENT_LOCALSTATEDIR]) if test "x$guix_cv_current_localstatedir" != "xnone"; then if test "$guix_cv_current_localstatedir" != "$guix_localstatedir"; then case "$localstatedir" in NONE|\${prefix}*) # User kept the default value---i.e., did not pass '--localstatedir'. AC_MSG_ERROR([chosen localstatedir '$guix_localstatedir' does not match \ that of the existing installation '$guix_cv_current_localstatedir' Installing may corrupt $storedir! Use './configure --localstatedir=$guix_cv_current_localstatedir'.]) ;; *) # User passed an explicit '--localstatedir'. Assume they know what # they're doing. AC_MSG_WARN([chosen localstatedir '$guix_localstatedir' does not match \ that of the existing installation '$guix_cv_current_localstatedir']) AC_MSG_WARN([installing may corrupt $storedir!]) ;; esac fi fi]) --8<---------------cut here---------------end--------------->8--- So it seems judicious to remove it, especially since with this change the default `./configure` would no longer require the user to think about '--localstatedir' and friends (unless their system was an odd snowflake, in which case the macro would do its job to warn them). Does that clarify the intent?
Hi, Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > The Guix standard configuration uses a localstatedir of /var and a sysconfdir > of /etc. To ease things for everyone, make the default values match that > standard expected configuration. See > <https://lists.gnu.org/archive/html/guix-devel/2024-05/msg00003.html> for a > related discussion. > > * configure.ac: Default $prefix to '' unless already set. > * doc/contributing.texi (Building from Git): Streamline doc. > * doc/guix-cookbook.texi (Guix environment via direnv): Likewise. > > Change-Id: I23cd12b58a842d246fbc9fdc740311c573eb0212 While authoring the above, I got thinking about *why* we can't default to the default GNU /usr/local prefix, since Guix *should* handle this correctly, via paths registered in the (guix config). I guess the only reason is that /var is the hard-coded $localstatedir value we chose for any Guix package and can't meaningfully be changed (without rewritting the graft), so if it must exists anyway, we might as well use it for Guix itself by default. In other words, I believe this change does the right thing (the other idea I was musing with defaulting '--prefix=' or '--localstatedir' to the value of any installed guix, unless the user had provided one explicitly).
Hi Ludovic, Ludovic Courtès <ludo@gnu.org> writes: > Hi Maxim, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> The Guix standard configuration uses a localstatedir of /var and a sysconfdir >> of /etc. To ease things for everyone, make the default values match that >> standard expected configuration. See >> <https://lists.gnu.org/archive/html/guix-devel/2024-05/msg00003.html> for a >> related discussion. >> >> * configure.ac: Default $prefix to '' unless already set. >> * doc/contributing.texi (Building from Git): Streamline doc. >> * doc/guix-cookbook.texi (Guix environment via direnv): Likewise. >> >> Change-Id: I23cd12b58a842d246fbc9fdc740311c573eb0212 > > [...] > >> +dnl Set some sane default directory variables for use with the Guix. This >> +dnl also causes localstatedir to be /var and sysconfdir to be /etc. >> +test "$prefix" = NONE && prefix= > > Shouldn’t it read “prefix=/”? This is what I went for originally, but it would cause this error: --8<---------------cut here---------------start------------->8--- configure: error: chosen localstatedir '//var' does not match that of the existing installation '/var' Installing may corrupt /gnu/store! Use './configure --localstatedir=/var'. --8<---------------cut here---------------end--------------->8--- So it seems Autoconf's generated configure script expects --prefix to *not* contain a trailing '/', which is the case for --prefix=/. > Might be good to check whether this breaks “make distcheck”, so I > suppose it should be fine. Good suggestion; I just tested the target and the tarball was successfully generated. > Apart from that it LGTM. Thanks for this patch! Thanks for reviewing it! I'll send a v2, and wait for further comments, if any.
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: >> Shouldn’t it read “prefix=/”? > > This is what I went for originally, but it would cause this error: > > configure: error: chosen localstatedir '//var' does not match that of the existing installation '/var' > Installing may corrupt /gnu/store! > Use './configure --localstatedir=/var'. > > So it seems Autoconf's generated configure script expects --prefix to > *not* contain a trailing '/', which is the case for --prefix=/. Oh I see, makes sense. Thanks, Ludo’.
diff --git a/configure.ac b/configure.ac index 8c3a06da37..f831416650 100644 --- a/configure.ac +++ b/configure.ac @@ -73,6 +73,10 @@ AC_ARG_ENABLE([daemon], [guix_build_daemon="$enableval"], [guix_build_daemon="yes"]) +dnl Set some sane default directory variables for use with the Guix. This +dnl also causes localstatedir to be /var and sysconfdir to be /etc. +test "$prefix" = NONE && prefix= + # Prepare a version of $localstatedir & co. that does not contain references # to shell variables. guix_prefix="`eval echo $prefix | sed -e"s|NONE|/usr/local|g"`" diff --git a/doc/contributing.texi b/doc/contributing.texi index 66f4e86d0a..539b4dd0d3 100644 --- a/doc/contributing.texi +++ b/doc/contributing.texi @@ -235,7 +235,7 @@ Building from Git Then, run: @example -./configure --localstatedir=/var --sysconfdir=/etc +./configure @end example @noindent @@ -246,19 +246,6 @@ Building from Git important to pass the right @code{localstatedir} and @code{sysconfdir} values, which get recorded in the @code{(guix config)} Guile module. -When configuring Guix on a system that already has a Guix installation, -be sure to specify the same state directory as the existing installation -using the @option{--localstatedir} option of the @command{configure} -script (@pxref{Directory Variables, @code{localstatedir},, standards, -GNU Coding Standards}). Usually, this @var{localstatedir} option is set -to the value @file{/var}. The @command{configure} script protects -against unintended misconfiguration of @var{localstatedir} so you do not -inadvertently corrupt your store (@pxref{The Store}). The configuration -directory should also be configured by setting the @option{--sysconfdir} -option to the @file{/etc} value, which is the location used by Guix to -store for example the access control list of authorized machines and the -definition of offload machines. - Finally, you can build Guix and, if you feel so inclined, run the tests (@pxref{Running the Test Suite}): diff --git a/doc/guix-cookbook.texi b/doc/guix-cookbook.texi index 3bc63cba7a..cbd088632a 100644 --- a/doc/guix-cookbook.texi +++ b/doc/guix-cookbook.texi @@ -5076,7 +5076,7 @@ Guix environment via direnv # Predefine configure flags. configure() @{ - ./configure --localstatedir=/var --prefix= + ./configure @} export_function configure