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

[Emil Velikov]

On 8 March 2017 at 05:08, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> On Tue, Mar 07, 2017 at 09:31:54PM +0000, Emil Velikov wrote:
>> 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.
>
> sort-of. the server defaults to ${datadir}/X11/xkb/compiled but on fedora we
> use --with-xkb-output=%{_localstatedir}/lib/xkb which I guess is where ajax'
> motivation comes from (and it makes more sense anyway). But for the repos to
> update this reliably we need it in the pc file, etc. which is what the rest
> of the patch does.
>
Right, so I was confused. The "compiled" xkb path and README.compiled
are provided by Xorg, but what's not obvious is - who is the user of
said path/files ?
This patch on its own does not make much sense, I'm afraid - we add a
"default" location and we have nobody of change and/or read it :-\

>>
>> > --- 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
>
> can't we just drop it? there's no realistic path in configure that ends up
> in DFLT_XKB_CONFIG_ROOT. And we should drop it from the Makefile.am as well.
>
Sure dropping the #error might be better, although why do we need
AC_DEFINE_DIR in the first place ?
Can anyone kindly point to instructions how to reproduce the issue, or
more info about the issue itself.

Thanks
Emil