Bug 75858

Created attachment 95264 [details] [review]
Hack pkgconfig files to be more MinGW-friendly

Created attachment 95265 [details] [review]
Process and install pkg-config files for dbus

This is tied to the other patch, the one that makes pkg-config files
MinGW-friendly.

Comment on attachment 95264 [details] [review]
Hack pkgconfig files to be more MinGW-friendly

Review of attachment 95264 [details] [review]:
-----------------------------------------------------------------

::: configure.ac
@@ +1451,3 @@
>  #### find the actual value for $prefix that we'll end up with
>  ##   (I know this is broken and should be done in the Makefile, but
>  ##    that's a major pain and almost nobody actually seems to care)

You are running into precisely the brokenness described by this comment. Your use-case is why it is considered broken.

@@ +1453,5 @@
>  ##    that's a major pain and almost nobody actually seems to care)
> +UNEXPANDED_PREFIX="$prefix"
> +UNEXPANDED_LOCALSTATEDIR="$localstatedir"
> +UNEXPANDED_SYSCONFDIR="$sysconfdir"
> +UNEXPANDED_BINDIR="$bindir"

That's a sh(1) variable reference. What you've done here is not "unexpanded", it's "expanded, but not recursively" - for instance, in a typical Unix build with default settings, that's equivalent to

    UNEXPANDED_BINDIR='${exec_prefix}/bin'

At best that's misleading naming.

prefix, localstatedir, sysconfdir, bindir etc. are all AC_SUBST'd anyway, so you should just be able to use

prefix=@prefix@
exec_prefix=@exec_prefix@
bindir=@bindir@

and so on.

::: dbus-1.pc.in
@@ +3,4 @@
>  libdir=@libdir@
>  includedir=@includedir@
>  system_bus_default_address=@DBUS_SYSTEM_BUS_DEFAULT_ADDRESS@
> +sysconfdir=@sysconfdir@

s/@EXPANDED_SYSCONFDIR@/@sysconfdir@/ is an improvement, I'd take that patch (assuming the CMake stuff either substitutes @sysconfdir@, or can be made to do so).

@@ +3,5 @@
>  libdir=@libdir@
>  includedir=@includedir@
>  system_bus_default_address=@DBUS_SYSTEM_BUS_DEFAULT_ADDRESS@
> +sysconfdir=@sysconfdir@
> +session_bus_services_dir=@UNEXPANDED_DATADIR@/dbus-1/services

The right way to do this is something like:

datarootdir=@datarootdir@
datadir=@datadir@
session_bus_services_dir=${datadir}/dbus-1-services

or whatever the hierarchy of expansions is for the default ${datadir}.

@@ +6,5 @@
> +sysconfdir=@sysconfdir@
> +session_bus_services_dir=@UNEXPANDED_DATADIR@/dbus-1/services
> +system_bus_services_dir=@UNEXPANDED_DATADIR@/dbus-1/system-services
> +interfaces_dir=@UNEXPANDED_DATADIR@/dbus-1/interfaces
> +daemondir=${exec_prefix}/bin

No, daemondir really does need to be its own variable - otherwise, this will break the functionality of --with-dbus-daemondir in configure.ac.

You could have a variable that is unexpanded and defaults to '${bindir}' - I would suggest calling it dbus_daemondir - and a separate expanded version from

    AS_AC_EXPAND([DBUS_DAEMONDIR], ["$dbus_daemondir"])

for hard-coding into binaries and bus/org.freedesktop.dbus-session.plist (only used on Unix and Mac OS, respectively).

The fact that we manually expand *anything* is arguably a bug, but it's one we're stuck with for backwards compatibility in D-Bus-as-an-OS-feature, which is its most important use case.

Comment on attachment 95265 [details] [review]
Process and install pkg-config files for dbus

Review of attachment 95265 [details] [review]:
-----------------------------------------------------------------

::: cmake/CMakeLists.txt
@@ +75,5 @@
>  set(DBUS_DAEMONDIR           ${EXPANDED_BINDIR})
> +set(libdir \${prefix}/lib)
> +set(includedir \${prefix}/include)
> +set(sysconfdir \${prefix}/etc)
> +set(UNEXPANDED_DATADIR \${prefix}/share)

set(datadir ...), set(datarootdir ...)

(In reply to comment #0)
> DBus pkg-config files explicitly use expanded versions of various paths in
> .pc
> files. I assume this is necessary on *nix.

It isn't, if you substitute all the layers of variables like

    prefix=@prefix@
    datarootdir=@datarootdir@
    datadir=@datadir@
    session_bus_services_dir=${datadir}/dbus-1-services

which typically expands to

    prefix=/usr
    datarootdir=${prefix}/share
    datadir=${datarootdir}
    session_bus_services_dir=${datadir}/dbus-1-services

which pkg-config evaluates at runtime to

    session_bus_services_dir=/usr/share/dbus-1-services

as desired (but if you used pkg-config --define-variable=prefix=opt it would expand it to /opt/share/dbus-1-services, and so on).

Created attachment 95341 [details] [review]
Hack pkgconfig files to be more MinGW-friendly (v2, no autotools)

Doesn't include configure.ac changes yet

Created attachment 95342 [details] [review]
Process and install pkg-config files for dbus (v2)

Created attachment 95349 [details] [review]
Hack pkgconfig files to be more MinGW-friendly (v2, autotools part)

Works as intended, if i configure with:
--prefix=/mingw --with-dbus-daemondir=\${prefix}/bin --datarootdir=\${prefix}/share --docdir=\${prefix}/\${docdir} --sbindir=\${prefix}/sbin --libexecdir=\${prefix}/libexec --sysconfdir=\${prefix}/etc --localstatedir=\${prefix}/var

CMake version works as intended, if configured with:
    -DCMAKE_INSTALL_PREFIX=/${prefix} \
    -DCMAKE_SYSTEM_PREFIX_PATH=${prefix} \
    -Ddbus_daemondir=\${prefix}/bin

Totally forgot to add datarootdir=@datarootdir@ to .pc files

Created attachment 95944 [details] [review]
Hack pkgconfig files to be more MinGW-friendly (v3, no autotools)

Fixed version of the .pc changes, with datarootdir added. Other two patches are unaffected.

Comment on attachment 95342 [details] [review]
Process and install pkg-config files for dbus (v2)

Review of attachment 95342 [details] [review]:
-----------------------------------------------------------------

::: cmake/CMakeLists.txt
@@ +82,5 @@
> +endif()
> +set(libdir \${prefix}/lib)
> +set(includedir \${prefix}/include)
> +set(sysconfdir \${prefix}/etc)
> +set(datadir \${prefix}/share)

There should probably be a datarootdir here too, for completeness?

Comment on attachment 95349 [details] [review]
Hack pkgconfig files to be more MinGW-friendly (v2, autotools part)

Review of attachment 95349 [details] [review]:
-----------------------------------------------------------------

::: configure.ac
@@ +1588,3 @@
>  else
>      DBUS_DAEMONDIR=$with_dbus_daemondir
> +    dbus_daemondir=$with_dbus_daemondir

This still performs one level of expansion. Is that really what you want?

I would have expected that you'd want

if test -z "$with_dbus_daemondir"; then
    DBUS_DAEMONDIR=$EXPANDED_BINDIR
    dbus_daemondir='$bindir'
else
    DBUS_DAEMONDIR=$with_dbus_daemondir
    dbus_daemondir=$with_dbus_daemondir
fi

so that in a normal Unix installation, dbus-1.pc would say

prefix=/usr
exec_prefix=${prefix}
dbus_daemondir=${exec_prefix}/bin

and on msys it would be something like

prefix=/c/mingw
exec_prefix=${prefix}
dbus_daemondir=${exec_prefix}/bin

(With your patch it would be ${prefix}/bin - one level of expansion applied to the default value of ${bindir} - but the most correct thing would be to have zero levels of expansion.)

(In reply to comment #12)
> I would have expected that you'd want
> 
> if test -z "$with_dbus_daemondir"; then
>     DBUS_DAEMONDIR=$EXPANDED_BINDIR
>     dbus_daemondir='$bindir'
> else
>     DBUS_DAEMONDIR=$with_dbus_daemondir
>     dbus_daemondir=$with_dbus_daemondir
> fi

Er, that should have said '${bindir}' not '$bindir'.

To be consistent with the name of the variable in the .pc file, maybe daemondir would be a better name for the unexpanded version.

I think this would be fine as one big commit - I don't see any reason why it would make sense to apply some, but not all, of it.


> so that in a normal Unix installation, dbus-1.pc would say
> 
> prefix=/usr
> exec_prefix=${prefix}
> dbus_daemondir=${exec_prefix}/bin

That should have said:

prefix=/usr
exec_prefix=${prefix}
bindir=${exec_prefix}/bin
daemondir=${bindir}

Getting that result might mean you need to add bindir=@bindir@ to the .pc files.

(In reply to comment #8)
> Works as intended, if i configure with:
> --prefix=/mingw --with-dbus-daemondir=\${prefix}/bin
> --datarootdir=\${prefix}/share --docdir=\${prefix}/\${docdir}
> --sbindir=\${prefix}/sbin --libexecdir=\${prefix}/libexec
> --sysconfdir=\${prefix}/etc --localstatedir=\${prefix}/var

You shouldn't have to do all that. If you've got it right, "--prefix=/mingw" should be enough.

> --docdir=\${prefix}/\${docdir}

So docdir='${prefix}/${docdir}', with infinite recursion when evaluated? That's not going to work very well.

(In reply to comment #11)
> ::: cmake/CMakeLists.txt
> @@ +82,5 @@
> > +endif()
> > +set(libdir \${prefix}/lib)
> > +set(includedir \${prefix}/include)
> > +set(sysconfdir \${prefix}/etc)
> > +set(datadir \${prefix}/share)
> 
> There should probably be a datarootdir here too, for completeness?

Yes, but it's kind of optional. Datarootdir is needed there for autotools first and foremost, because datadir=${datarootdir} in that case. In cmake datadir is explicitly set as ${prefix}/share, so datarootdir is not really needed. But it's OK to have it too.(In reply to comment #12)

> Review of attachment 95349 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: configure.ac
> @@ +1588,3 @@
> >  else
> >      DBUS_DAEMONDIR=$with_dbus_daemondir
> > +    dbus_daemondir=$with_dbus_daemondir
> 
> This still performs one level of expansion. Is that really what you want?
> 
> I would have expected that you'd want
> 
> if test -z "$with_dbus_daemondir"; then
>     DBUS_DAEMONDIR=$EXPANDED_BINDIR
>     dbus_daemondir='$bindir'
> else
>     DBUS_DAEMONDIR=$with_dbus_daemondir
>     dbus_daemondir=$with_dbus_daemondir
> fi
> 
> so that in a normal Unix installation, dbus-1.pc would say
> 
> prefix=/usr
> exec_prefix=${prefix}
> dbus_daemondir=${exec_prefix}/bin
> 
> and on msys it would be something like
> 
> prefix=/c/mingw
> exec_prefix=${prefix}
> dbus_daemondir=${exec_prefix}/bin
> 
> (With your patch it would be ${prefix}/bin - one level of expansion applied
> to the default value of ${bindir} - but the most correct thing would be to
> have zero levels of expansion.)

I get dbus_daemondir=${exec_prefix}/bin with my patch, when building with cmake. What am i doing wrong?
Shell expansion always baffled me, to be honest.

(In reply to comment #14)
> (In reply to comment #8)
> > Works as intended, if i configure with:
> > --prefix=/mingw --with-dbus-daemondir=\${prefix}/bin
> > --datarootdir=\${prefix}/share --docdir=\${prefix}/\${docdir}
> > --sbindir=\${prefix}/sbin --libexecdir=\${prefix}/libexec
> > --sysconfdir=\${prefix}/etc --localstatedir=\${prefix}/var
> 
> You shouldn't have to do all that. If you've got it right, "--prefix=/mingw"
> should be enough.

I like to not to rely on autotools' defaults, they are wrong sometimes (i build lots of things, this (minus the --with-dbus-daemondir) is basically a template i use everywhere).
> 
> > --docdir=\${prefix}/\${docdir}
> 
> So docdir='${prefix}/${docdir}', with infinite recursion when evaluated?
> That's not going to work very well.

Ah, that. No, i guess i wasn't making things clear enough.
This is what i do:

prefix=/mingw
docdir=share/doc/${name}/${ver}

...

if test "x${install_dirs}" == "x"; then install_dirs="--datarootdir=\\\${prefix}/share --docdir=\\\${prefix}/\${docdir} --sbindir=\\\${prefix}/sbin --libexecdir=\\\${prefix}/libexec --sysconfdir=\\\${prefix}/etc --localstatedir=\\\${prefix}/var"; fi

...

eval "install_dirs=\"$install_dirs\""

...

  ./configure \
    --prefix=${prefix} \
    --enable-silent-rules \
    --enable-verbose-mode \
    --enable-checks \
    --enable-asserts \
    --enable-xml-docs \
    --enable-embedded-tests \
    --enable-modular-tests \
    --enable-installed-tests \
    --with-dbus-daemondir=\${prefix}/bin \
    ${install_dirs} \
...

(In reply to comment #16)
> This is what i do:
> 
> prefix=/mingw
> docdir=share/doc/${name}/${ver}

I recommend using a less confusing name for your intermediate variable :-)

> if test "x${install_dirs}" == "x"; then
> install_dirs="--datarootdir=\\\${prefix}/share
> --docdir=\\\${prefix}/\${docdir} --sbindir=\\\${prefix}/sbin
> --libexecdir=\\\${prefix}/libexec --sysconfdir=\\\${prefix}/etc
> --localstatedir=\\\${prefix}/var"; fi
> ...
> eval "install_dirs=\"$install_dirs\""

If you find yourself using eval, it's usually a sign that something has gone wrong.

(In reply to comment #15)
> > (With your patch it would be ${prefix}/bin - one level of expansion applied
> > to the default value of ${bindir} - but the most correct thing would be to
> > have zero levels of expansion.)
> 
> I get dbus_daemondir=${exec_prefix}/bin with my patch, when building with
> cmake. What am i doing wrong?
> Shell expansion always baffled me, to be honest.

More specifically, with no special configure command-line options, the right thing would be for the resulting .pc file to contain a line «dbus_daemondir=${exec_prefix}/bin» and a line «exec_prefix=${prefix}» and a line «prefix=/usr/local», so that pkg-config can do all the levels of expansion.

(Deliberately using non-standard quotes to avoid confusion with sh(1) syntax :-)

If that's the case already with your patchset, then I'll need to look at what's going on in configure and why it works correctly when I expected that it wouldn't.

(In reply to comment #16)
> (In reply to comment #14)
> > You shouldn't have to do all that. If you've got it right, "--prefix=/mingw"
> > should be enough.
> 
> I like to not to rely on autotools' defaults, they are wrong sometimes (i
> build lots of things, this (minus the --with-dbus-daemondir) is basically a
> template i use everywhere).

That's fine for your personal meta-build-system, but by making all these command-line options explicit, you're hiding bugs in configure scripts. When you're testing patches to be submitted, please also test them with that simpler situation (no command-line options, or --prefix=/mingw and nothing else) and check that they are working as intended, and you aren't ignoring (or creating!) configure-script bugs.

dbus needs to work in multiple configurations, not just your personal configuration - indeed, the most important configuration for it to work in is «--prefix=/usr --sysconfdir=/etc --localstatedir=/var», because that's what Linux distributions will be using.

(In reply to comment #17)
> More specifically, with no special configure command-line options, the right
> thing would be for the resulting .pc file to contain a line
> «dbus_daemondir=${exec_prefix}/bin» and a line «exec_prefix=${prefix}» and a
> line «prefix=/usr/local», so that pkg-config can do all the levels of
> expansion.

Ugh, now I'm confusing myself. It should ideally have «dbus_daemondir=${bindir}» and «bindir=${exec_prefix}/bin» and «exec_prefix=${prefix}» and «prefix=/usr/local» - in other words, pkg-config should be doing *all* of the levels of expansion.

Created attachment 98131 [details] [review]
Make sure bindir is in .pc file and daemondir there is unexpanded

With attachment 95349 [details] [review] and attachment 95944 [details] [review] and with invoking configure like this:
../dbus-1.8.0/configure --prefix=/mingw --enable-silent-rules --enable-verbose-mode --enable-checks --enable-asserts --enable-xml-docs --enable-embedded-tests --enable-modular-tests --enable-installed-tests --build=i686-w64-mingw32 STRIP=true CFLAGS="-g -O2 -D__USE_MINGW_ANSI_STDIO=1"

i get this in the .pc file:

> prefix=/mingw
> exec_prefix=${prefix}
> libdir=${exec_prefix}/lib
> includedir=${prefix}/include
> system_bus_default_address=unix:path=/mingw/var/run/dbus/system_bus_socket
> datarootdir=${prefix}/share
> datadir=${datarootdir}
> sysconfdir=${prefix}/etc
> session_bus_services_dir=${datadir}/dbus-1/services
> system_bus_services_dir=${datadir}/dbus-1/system-services
> interfaces_dir=${datadir}/dbus-1/interfaces
> daemondir=/mingw/bin

and documentation goes into /mingw/share/doc/dbus.

If i add --sysconfdir=/etc --localstatedir=/var, i get this in the .pc file:

> prefix=/mingw
> exec_prefix=${prefix}
> libdir=${exec_prefix}/lib
> includedir=${prefix}/include
> system_bus_default_address=unix:path=/var/run/dbus/system_bus_socket
> datarootdir=${prefix}/share
> datadir=${datarootdir}
> sysconfdir=/etc
> session_bus_services_dir=${datadir}/dbus-1/services
> system_bus_services_dir=${datadir}/dbus-1/system-services
> interfaces_dir=${datadir}/dbus-1/interfaces
> daemondir=/mingw/bin


with obvious consequences (conf files in <instdir>/etc instead of <instdir><prefix>/etc , same with /var).

Without --prefix (but with --sysconfdir=/etc --localstatedir=/var) i get this:

> prefix=/usr/local
> exec_prefix=${prefix}
> libdir=${exec_prefix}/lib
> includedir=${prefix}/include
> system_bus_default_address=unix:path=/var/run/dbus/system_bus_socket
> datarootdir=${prefix}/share
> datadir=${datarootdir}
> sysconfdir=/etc
> session_bus_services_dir=${datadir}/dbus-1/services
> system_bus_services_dir=${datadir}/dbus-1/system-services
> interfaces_dir=${datadir}/dbus-1/interfaces
> daemondir=/usr/local/bin

and some things get installed into <instdir>/usr/local

> More specifically, with no special configure command-line options, the right thing would be for the resulting .pc file to contain a line «dbus_daemondir=${exec_prefix}/bin» and a line «exec_prefix=${prefix}» and a line «prefix=/usr/local», so that pkg-config can do all the levels of expansion.
> Ugh, now I'm confusing myself. It should ideally have «dbus_daemondir=${bindir}» and «bindir=${exec_prefix}/bin» and «exec_prefix=${prefix}» and «prefix=/usr/local» - in other words, pkg-config should be doing *all* of the levels of expansion.

[ ] daemondir (by the way, is it "daemondir" or "dbus_daemondir"?)
[ ] bindir
[x] exec_prefix
[x] prefix

So, we need to:
add bindir=${exec_prefix}/bin (config.status says «S["bindir"]="${exec_prefix}/bin"», so just setting it to $bindir in configure will probably work)
fix daemondir to not to be expanded (i.e. use $bindir instead of $EXPANDED_BINDIR)

With the extra patch that i'm attaching, i get (configured with --prefix=/mingw and --enable..., just as the first config presented in this comment):

> prefix=/mingw
> exec_prefix=${prefix}
> bindir=${exec_prefix}/bin
> libdir=${exec_prefix}/lib
> includedir=${prefix}/include
> system_bus_default_address=unix:path=/mingw/var/run/dbus/system_bus_socket
> datarootdir=${prefix}/share
> datadir=${datarootdir}
> sysconfdir=${prefix}/etc
> session_bus_services_dir=${datadir}/dbus-1/services
> system_bus_services_dir=${datadir}/dbus-1/system-services
> interfaces_dir=${datadir}/dbus-1/interfaces
> daemondir=${exec_prefix}/bin

Which should satisfy your conditions.

Comment on attachment 98131 [details] [review]
Make sure bindir is in .pc file and daemondir there is unexpanded

Review of attachment 98131 [details] [review]:
-----------------------------------------------------------------

> With the extra patch that i'm attaching, i get (configured with --prefix=/mingw and --enable..., just as the first config presented in this comment):
> 
> > prefix=/mingw
> > exec_prefix=${prefix}
> > bindir=${exec_prefix}/bin
> > libdir=${exec_prefix}/lib
> > includedir=${prefix}/include
> > system_bus_default_address=unix:path=/mingw/var/run/dbus/system_bus_socket
> > datarootdir=${prefix}/share
> > datadir=${datarootdir}
> > sysconfdir=${prefix}/etc
> > session_bus_services_dir=${datadir}/dbus-1/services
> > system_bus_services_dir=${datadir}/dbus-1/system-services
> > interfaces_dir=${datadir}/dbus-1/interfaces
> > daemondir=${exec_prefix}/bin
>
> Which should satisfy your conditions.

This all looks good except that daemondir has been expanded once: its value is a copy of the value of ${bindir}, not the unexpanded string «${bindir}».

Would you mind combining all the patches you want applied for this bug into one, or perhaps two if you want to keep Attachment #95342 [details] separate? I don't think the intermediate states with some, but not all, of your three most recent patches make sense; it'll be clearer to just get it right in one go.

::: configure.ac
@@ +1584,4 @@
>  #### Directory to install dbus-daemon
>  if test -z "$with_dbus_daemondir" ; then
>      DBUS_DAEMONDIR=$EXPANDED_BINDIR
> +    dbus_daemondir=$bindir

I think this should probably be

    dbus_daemondir='${bindir}'

with the quotes.

The fact that configure is a shell script means that you're applying one level of shell expansion by leaving it unquoted: after executing this line, the value of the dbus_daemondir variable will be something like «${exec_prefix}/bin», whereas for the maximum amount of scope for redefining things when invoking pkg-config, it would be better for it to be «${bindir}».

Created attachment 98256 [details] [review]
Hack pkgconfig files to be more MinGW-friendly (v4, combined)

I haven't touched cmake in a while, and i'm currently not very interested in doing so. To that end i'd rather see this to be committed right now, as-is, without checking that cmake (with or without the attachment 95342 [details] [review]) counterpart works, and *then* someone can see to making cmake compatible with the new .pc.in files.

Comment on attachment 98256 [details] [review]
Hack pkgconfig files to be more MinGW-friendly (v4, combined)

Review of attachment 98256 [details] [review]:
-----------------------------------------------------------------

::: dbus-1-uninstalled.pc.in
@@ +6,2 @@
>  system_bus_default_address=@DBUS_SYSTEM_BUS_DEFAULT_ADDRESS@
> +datadir=@datadir@

The default is datadir='${datarootdir}', so as far as I can see, you will need

datarootdir=@datarootdir@

before this line to make it work for anyone not setting an explicit datadir.

::: dbus-1.pc.in
@@ +4,4 @@
>  libdir=@libdir@
>  includedir=@includedir@
>  system_bus_default_address=@DBUS_SYSTEM_BUS_DEFAULT_ADDRESS@
> +datadir=@datadir@

Likewise

(In reply to LRN from comment #21)
> To that end i'd rather see this to be committed right now, as-is,
> without checking that cmake (with or without the attachment 95342 [details] [review])
> counterpart works, and *then* someone can see to making cmake
> compatible with the new .pc.in files.

Sorry, no, I am not willing to commit this with known regressions. The cmake side of things can be pretty simple - only the things that now work need to still work, and in particular it doesn't necessarily need to leave strings unexpanded like you're doing with the Autotools side - but it does need to exist.

As far as I can see, that means, at a minimum,

set(bindir ${EXPANDED_BINDIR})
set(datadir ${EXPANDED_DATADIR})
set(sysconfdir ${EXPANDED_SYSCONFDIR})
set(dbus_daemondir ${DBUS_DAEMONDIR})

(In reply to Simon McVittie from comment #22)
> The default is datadir='${datarootdir}', so as far as I can see, you will
> need
> 
> datarootdir=@datarootdir@
> 
> before this line to make it work for anyone not setting an explicit datadir.

Actually, this would work, for now, but only because configure contains explicit compatibility code:

# If the template does not know about datarootdir, expand it.
# FIXME: This hack should be removed a few years after 2.60.

This doesn't seem like something that will continue to work forever, and provokes warnings:

config.status: creating dbus-1.pc
config.status: WARNING:  '/home/smcv/src/fdo/dbus/dbus-1.pc.in' seems to ignore the --datarootdir setting
config.status: creating dbus-1-uninstalled.pc
config.status: WARNING:  '/home/smcv/src/fdo/dbus/dbus-1-uninstalled.pc.in' seems to ignore the --datarootdir setting

(In reply to Simon McVittie from comment #23)
> The cmake
> side of things can be pretty simple - only the things that now work need to
> still work, and in particular it doesn't necessarily need to leave strings
> unexpanded like you're doing with the Autotools side - but it does need to
> exist.

Actually, no, I take that back. The CMake build system doesn't currently install .pc files at all, which is a bug, but means it isn't a regression if the CMake build system doesn't substitute everything that needs to be substituted.

(In reply to Simon McVittie from comment #25)
> The CMake build system doesn't currently
> install .pc files at all, which is a bug

Bug #84966

(In reply to Simon McVittie from comment #22)
> you will need
> 
> datarootdir=@datarootdir@

Fixed that and committed the result. Fixed in git for 1.9.2, thanks!