On 7/20/17 11:25 AM, Daniel Stone wrote:
Hi,

On 20 July 2017 at 10:14, Quentin Glidic
<[email protected]> wrote:
Using ${pc_sysrootdir} sounds like the correct thing to do. However, your
patch introduces a triple / when PKG_CONFIG_SYSROOT_DIR is not set.
Having two / is already trouble on some platform (though the one we
currently work on should be fine, I think), three / would break even more.
Please just use "${pc_sysrootdir}@datadir@".

Yeah, this all LGTM, and gets my:
Reviewed-by: Daniel Stone <[email protected]>

Also, using this variable should be in prefix=, IMO, since all the paths
should be fixed.

I'm wary of doing that, if only because it establishes a pattern. It's
fine for wayland-protocols itself, but when doing sysroot builds,
includedir/libdir/etc already get prefixed by the compiler/linker. So
I wouldn't want anyone to look at wayland-protocols and blindly copy
the prepend to prefix. Containing it to just variables we know will be
used and queried directly seems more safe.

pkg-config already prepends PKG_CONFIG_SYSROOT_DIR to --cflags and --libs (and properly handle it if you put it in prefix, i.e. no dublicate), so it is completely safe to have in all variables.

--

Quentin “Sardem FF7” Glidic
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to