[PATCH] Autodetect libdrm path (v2)

[Emil Velikov]

Hi Tom,

On 5 February 2017 at 22:24, Tom St Denis <tstdenis82 at gmail.com> wrote:
> (v2):  Use findLibDRM script instead of directly finding path
>
Since the project already depends on libdrm you might want to use the
drmDevice2 API and drop the iteration over all PCI devices
(libpciaccess dependency).
If the former is lacking something feel free to suggest/send patches ;-)

The libdrm requirement was missing last time I've skimmed through the README.

> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
> ---

> --- /dev/null
> +++ b/cmake_modules/FindLibDRM.cmake
> @@ -0,0 +1,35 @@
> +# Try to find libdrm
> +#
> +# Once done, this will define
> +#
> +# LIBDRM_FOUND
> +# LIBDRM_INCLUDE_DIR
> +# LIBDRM_LIBRARIES
> +
> +find_package(PkgConfig)
> +
> +pkg_check_modules(PC_LIBDRM QUIET libdrm)
You really want a version check.

> +
> +find_path(LIBDRM_INCLUDE_DIR NAMES amdgpu_drm.h
> +    HINTS
> +    ${PC_LIBDRM_INCLUDEDIR}
> +    ${PC_LIBDRM_INCLUDE_DIRS}
> +    /usr/include
> +)
> +
> +find_library(LIBDRM_LIBRARY NAMES libdrm_amdgpu.so.1
> +    HINTS
> +    ${PC_LIBDRM_LIBDIR}
> +    ${PC_LIBDRM_LIBRARY_DIRS}
> +    /usr/lib64
> +    /usr/lib
> +)
> +
Here is one of the things which makes me rage against cmake - no
picking /usr/{include,foo} is _not_ what you want.

During cross-compilation as one is missing the .pc file, the system
headers/libraries will be picked. This is horribly wrong, yet rather
common in cmake world.
What you really want is a lovely error message to warn the user -
s/OPTIONAL/REQUIRED/ will give you just that.

At which point, checking for the header/library in the paths provided
by the .pc file is redundant and thus nearly everything else in the
file ;-)


TL;Dr: All you need is pkg_check_modules(PC_LIBDRM REQUIRED libdrm>=XX).

Thanks
Emil