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

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Maxim Cournoyer
  • Ricardo Wurmus
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 18 May 01:52 +0200
(address . guix-patches@gnu.org)
9ac2df37867548ae5e15e4c262458d8fc63ba9a1.1715989953.git.maxim.cournoyer@gmail.com
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
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(-)

Toggle diff (65 lines)
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
R
R
Ricardo Wurmus wrote on 19 May 12:42 +0200
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87h6eunkw3.fsf@elephly.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (13 lines)
> -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
L
L
Ludovic Courtès wrote on 19 May 16:16 +0200
Re: [bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87ttit3n11.fsf@gnu.org
Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

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

[...]

Toggle quote (4 lines)
> +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’.
M
M
Maxim Cournoyer wrote on 22 May 02:16 +0200
Re: [PATCH] configure.ac: Set default value for the 'prefix' variable.
(name . Ricardo Wurmus)(address . rekado@elephly.net)
87r0du4s6b.fsf@gmail.com
Hi Ricardo,

Ricardo Wurmus <rekado@elephly.net> writes:

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

Toggle snippet (28 lines)
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])

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
M
M
Maxim Cournoyer wrote on 22 May 02:24 +0200
(address . guix-patches@gnu.org)
87jzjm4rtr.fsf@gmail.com
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (12 lines)
> 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
M
M
Maxim Cournoyer wrote on 22 May 14:30 +0200
Re: [bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87frua3u7q.fsf@gmail.com
Hi Ludovic,

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

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

Toggle snippet (5 lines)
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=/.

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

Toggle quote (2 lines)
> 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
M
M
Maxim Cournoyer wrote on 22 May 14:51 +0200
[PATCH v2] configure.ac: Set default value for the 'prefix' variable.
(address . 71022@debbugs.gnu.org)
8e5c69aa6d108ce35c3124a969252227ce9ed178.1716382317.git.maxim.cournoyer@gmail.com
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
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(-)

Toggle diff (69 lines)
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
L
L
Ludovic Courtès wrote on 23 May 11:17 +0200
Re: [bug#71022] [PATCH] configure.ac: Set default value for the 'prefix' variable.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87cypcc2g2.fsf@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (11 lines)
>> 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’.
L
L
Ludovic Courtès wrote on 23 May 11:17 +0200
Re: [PATCH v2] configure.ac: Set default value for the 'prefix' variable.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
878r00c2f7.fsf@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (12 lines)
> 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! :-)
M
M
Maxim Cournoyer wrote on 26 May 05:38 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)
877cfhi6or.fsf@gmail.com
Hi,

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

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

Toggle snippet (6 lines)
'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

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
Closed
?
Your comment

Commenting via the web interface is currently disabled.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 71022
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