Libguestfs - Dec 2018 - Re: [nbdkit PATCH] build: Allow 'make install' into non-root --prefix: bash-completion

In general, autotooled packages are supposed to allow
'./configure --prefix=$HOME/subdir' as a way to then get
'make install' to run as non-root.  In fact, 'make distcheck'
tests that this scenario works; alas, we fail due to:

 /usr/bin/install -c -m 644 ../../../bash/nbdkit
'/usr/share/bash-completion/completions'
/usr/bin/install: cannot remove
'/usr/share/bash-completion/completions/nbdkit': Permission denied

The culprit? We use pkg-config to ask bash-completion where it
would install user completion scripts.  bash-completion.pc states:

prefix=/usr
completionsdir=${prefix}/share/bash-completion/completions

but pkg-config --variable defaults to flattening the ${prefix}
in our use of the PKG_CHECK_VAR() macro, which in turn means
that './configure --prefix=$HOME/subdir' still uses an absolute
path pointing to a root-owned directory (rather than one relative
to our desired ${prefix}) in our definition of bashcompsdir.

The solution? Tell pkg-config to NOT flatten the prefix variable.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I'm posting this one for review, particularly since it means someone
running 'make install' with a non-root --prefix will now be stuck
with bash completion libraries that won't be loaded by default (the
user will have to probably edit their ~/.bashrc to load things), but
that still seems better than the alternative of failing to install
because the installation wants to override root-owned files in spite
of the non-root --prefix.

'make distcheck' still doesn't pass for me; the next failure is
due to attempts to overwrite root-owned ocaml files.  Similar symptoms
as what this patch addressed for bashcompsdir, but there we aren't
using pkg-config --variable.  Instaed, our definition of ocamllibdir
(based on ${OCAMLLIB} in m4/ocaml.m4) is coming from 'ocamlc -where',
and I have no idea how to coerce that to print a relative pathname,
or even if it is common to have ocaml libraries installed in a non-root
home directory but which can still be picked up by ocaml at runtime.

 configure.ac | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 33ed4d5..8c97ffb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -337,7 +337,15 @@ PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >=
2.0], [
     bash_completion=yes
     AC_MSG_CHECKING([for bash-completions directory])
     m4_ifdef([PKG_CHECK_VAR],[
-        PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir])
+        dnl PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir])
+        dnl Unfortunately, PKG_CHECK_VAR flattens references to ${prefix},
+        dnl which in turn means './configure --prefix=...' to a
location
+        dnl writable by non-root still fails when it comes to installing
+        dnl the bash completions.
+        dnl So here, we just run the same command as the macro would, but
+        dnl with an override for prefix to keep 'make distcheck' happy.
+        bashcompdir=`$PKG_CONFIG --define-variable=prefix='${prefix}' \
+          --variable=completionsdir bash-completion 2>/dev/null`
     ])
     AS_IF([test -z "$bashcompdir"], [
         bashcompdir="${sysconfdir}/bash_completion.d"
-- 
2.17.2

On 12/7/18 2:17 PM, Eric Blake wrote:
> In general, autotooled packages are supposed to allow
> './configure --prefix=$HOME/subdir' as a way to then get
> 'make install' to run as non-root.  In fact, 'make
distcheck'
> tests that this scenario works; alas, we fail due to:
> 
>   /usr/bin/install -c -m 644 ../../../bash/nbdkit
'/usr/share/bash-completion/completions'
> /usr/bin/install: cannot remove
'/usr/share/bash-completion/completions/nbdkit': Permission denied
> 
> The culprit? We use pkg-config to ask bash-completion where it
> would install user completion scripts.  bash-completion.pc states:
> 
> prefix=/usr
> completionsdir=${prefix}/share/bash-completion/completions
> 
> but pkg-config --variable defaults to flattening the ${prefix}
> in our use of the PKG_CHECK_VAR() macro, which in turn means
> that './configure --prefix=$HOME/subdir' still uses an absolute
> path pointing to a root-owned directory (rather than one relative
> to our desired ${prefix}) in our definition of bashcompsdir.
> 
> The solution? Tell pkg-config to NOT flatten the prefix variable.
Well, it's _a_ solution.  Another one would be to NOT ask pkg-config 
ANYTHING, and just _always_ default to installing into ${sysconfdir}, 
since the patch already shows that to be our default if pkg-config is 
not around:
>       AS_IF([test -z "$bashcompdir"], [
>           bashcompdir="${sysconfdir}/bash_completion.d"
> 
But that has its own drawbacks - that expands to 
/usr/local/etc/bash_completions.d by default (or to 
/etc/bash_completion.d if you use --prefix=/usr with Fedora's 
/usr/share/config.site), but that name does NOT match the pkg-config 
name of /usr/share/bash-completion/completions (which would be more like 
${datarootdir}/bash-completion/completions).  Ultimately, we could still 
punt the issue to the distro packagers, by stating:

"we will ALWAYS install relative to --prefix, and then leave it up to 
you in your post-install munging to then move it into the location that 
pkg-config recommends"

by merely doing this patch instead (compare it to my proposed patch for 
fixing ocaml install, by just always picking a location relative to 
--prefix):

diff --git i/configure.ac w/configure.ac
index 33ed4d5..4a3e5c9 100644
--- i/configure.ac
+++ w/configure.ac
@@ -335,15 +335,6 @@ AS_IF([test "x$enable_valgrind" =
"xyes"],[
  dnl Bash completion.
  PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >= 2.0], [
      bash_completion=yes
-    AC_MSG_CHECKING([for bash-completions directory])
-    m4_ifdef([PKG_CHECK_VAR],[
-        PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir])
-    ])
-    AS_IF([test -z "$bashcompdir"], [
-        bashcompdir="${sysconfdir}/bash_completion.d"
-    ])
-    AC_MSG_RESULT([$bashcompdir])
-    AC_SUBST([bashcompdir])
  ],[
      bash_completion=no
      AC_MSG_WARN([bash-completion not installed])
diff --git i/bash/Makefile.am w/bash/Makefile.am
index e46784c..3b20c1b 100644
--- i/bash/Makefile.am
+++ w/bash/Makefile.am
@@ -36,6 +36,8 @@ EXTRA_DIST = README

  if HAVE_BASH_COMPLETION

+bashcompdir = $(sysconfdir)/bash_completion.d
+
  dist_bashcomp_DATA = nbdkit

  endif


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

I don't really know enough on this topic to comment on the patch,
except for a general comment that this sounds like it could be a bug
in either pkg-config or bash-completion.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top