[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[Maxim Cournoyer]

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

 configure.ac           |  4 ++++
 doc/contributing.texi  | 15 +--------------
 doc/guix-cookbook.texi |  2 +-
 3 files changed, 6 insertions(+), 15 deletions(-)

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
 

base-commit: 5a624adfd7b14c3717237d137bd0766c77f0f570
-- 
2.41.0

[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[Ricardo Wurmus]

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.

-- 
Ricardo

[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[Maxim Cournoyer]

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?

-- 
Thanks,
Maxim

[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[Ludovic Courtès]

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

[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[Maxim Cournoyer]

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

[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[Ludovic Courtès]

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

[bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.

[Maxim Cournoyer]

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

-- 
Thanks,
Maxim

[bug#71022] [PATCH v2] configure.ac: Set default value for the 'prefix' variable.

[Maxim Cournoyer]

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

Changes in v2:
 - combine comments and default directory variables manipulation

 configure.ac           |  8 +++++---
 doc/contributing.texi  | 15 +--------------
 doc/guix-cookbook.texi |  2 +-
 3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8c3a06da37..c7b61d56a7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -73,9 +73,11 @@ AC_ARG_ENABLE([daemon],
   [guix_build_daemon="$enableval"],
   [guix_build_daemon="yes"])
 
-# 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"`"
+dnl Prepare a version of $localstatedir & co. that does not contain references
+dnl to shell variables.  Also set some sane default directory variables for
+dnl use with the Guix.  This also causes localstatedir to be /var and
+dnl sysconfdir to be /etc.
+test "$prefix" = NONE && prefix=
 guix_localstatedir="`eval echo $localstatedir | sed -e "s|NONE|$guix_prefix|g"`"
 guix_sysconfdir="`eval echo $sysconfdir | sed -e "s|NONE|$guix_prefix|g"`"
 guix_sbindir="`eval echo $sbindir | sed -e "s|NONE|$guix_prefix|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
 

base-commit: 5a624adfd7b14c3717237d137bd0766c77f0f570
-- 
2.41.0

[bug#71022] [PATCH v2] configure.ac: Set default value for the 'prefix' variable.

[Ludovic Courtès]

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

LGTM! :-)

bug#71022: [PATCH v2] configure.ac: Set default value for the 'prefix' variable.

[Maxim Cournoyer]

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> 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
>
> LGTM! :-)

I ran 'make distcheck', which initially failed with a

--8<---------------cut here---------------start------------->8---
'dot' error: Format: "pdf" not recognized. Use one of: canon cmap cmapx cmapx_np
dot dot_json eps fig gd gd2 gif gv imap imap_np ismap jpe
jpeg jpg json json0 mp pic plain plain-ext png pov ps ps2
svg svgz tk vdx vml vmlz vrml wbmp xdot xdot1.2 xdot1.4 xdot_json
--8<---------------cut here---------------end--------------->8---

error.  I needed to use the full graphviz, not 'graphviz-minimal' as
used by the Guix package.

I also had to add imagemagick for the 'convert' command.  After that it
proceeded normally; it failed finding many missing substitutes but
otherwise 'make distcheck' went fine.

Pushed, at last.  Some related commits to the above will appear to
guix-patches soon.

-- 
Maxim