[PATCH app/xkbcomp] Add xkboutputdir to xkbcomp.pc

[Emil Velikov]

HI Adam,

On 7 March 2017 at 17:42, Adam Jackson <ajax at redhat.com> wrote:
> The X server (and everybody else, really) wants to know where to put
> xkbcomp's output, so xkbcomp.pc should say. This should be under
> localstatedir, since the output is architecture-dependent (as the note
> in xserver/xkb/README.compiled suggests).
>
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>  configure.ac        |  9 +++++++++
>  m4/ac_define_dir.m4 | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  xkbcomp.pc.in       |  1 +
>  xkbpath.c           |  4 +---
>  4 files changed, 63 insertions(+), 3 deletions(-)
>  create mode 100644 m4/ac_define_dir.m4
>
> diff --git a/configure.ac b/configure.ac
> index 0c8063e..3c151f3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -26,6 +26,7 @@ AC_INIT([xkbcomp], [1.3.1],
>          [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg], [xkbcomp])
>  AC_CONFIG_SRCDIR([Makefile.am])
>  AC_CONFIG_HEADERS([config.h])
> +AC_CONFIG_MACRO_DIR([m4])
>
>  # Initialize Automake
>  AM_INIT_AUTOMAKE([foreign dist-bzip2])
> @@ -59,7 +60,15 @@ AC_ARG_WITH([xkb_config_root],
>      [XKBCONFIGROOT="$withval"],
>      [XKBCONFIGROOT='${datadir}/X11/xkb'])
>  AC_SUBST([XKBCONFIGROOT])
> +AC_DEFINE_DIR(DFLT_XKB_CONFIG_ROOT, XKBCONFIGROOT, [XKB config root])
>
Not 100% sure but I think the current -DDFLT_XKB_CONFIG_ROOT... should
be sufficient. Alternatively can you point out what's the current vs
expected result ?

> +AC_ARG_WITH([xkb_output_path],
> +    [AS_HELP_STRING([--with-xkb-output-path=<path>],
> +        [Set default XKB output path (default: ${localstatedir}/xkb)])],
> +    [XKBOUTPUTPATH="$withval"],
> +    [XKBOUTPUTPATH='${localstatedir}/xkb'])
> +AC_SUBST([XKBOUTPUTPATH])
> +AC_DEFINE_DIR(DFLT_XKB_OUTPUT_PATH, XKBOUTPUTPATH, [XKB output path])
>
Slightly silly question: are these really the output path of xkbcomp ?
Afaict the tool outputs to any user specified location.
On my system the path /usr/share/X11/xkb is provided by
xkeyboard-config. The latter of which providing having xkb_base
variable in its .pc file for such purposes.

> --- a/xkbcomp.pc.in
> +++ b/xkbcomp.pc.in
> @@ -2,6 +2,7 @@ prefix=@prefix@
>  datarootdir=@datarootdir@
>  datadir=@datadir@
>  xkbconfigdir=@XKBCONFIGROOT@
> +xkboutputdir=@XKBOUTPUTPATH@
>
>  Name: xkbcomp
>  Description: XKB keymap compiler
> diff --git a/xkbpath.c b/xkbpath.c
> index 6f18b86..3345ba9 100644
> --- a/xkbpath.c
> +++ b/xkbpath.c
> @@ -33,9 +33,7 @@
>  #include <X11/extensions/XKM.h>
>  #include "xkbpath.h"
>
> -#ifndef DFLT_XKB_CONFIG_ROOT
> -#define DFLT_XKB_CONFIG_ROOT   "/usr/lib/X11/xkb"
Regardless if the current define is sufficient, we might want to
change this to something like below ?

#ifndef DFLT_XKB_CONFIG_ROOT
#error "DFLT_XKB_CONFIG_ROOT is not defined"
#endif

> -#endif
> +#include "config.h"
>
Hmm config.h management/amongst others looks very wrong on this
package. I'll give it some love (patches) after dinner.

-Emil