Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cmake] Properly find correct libcec version #10960

Merged
merged 1 commit into from
Nov 22, 2016
Merged

Conversation

fetzerch
Copy link
Member

Fix compile error if libcec 3.1 is installed.

Description

Just setting a pkgconfig minimum version doesn't work and just causes pkgconfig to not find an older package. But since our CMake modules don't trust package config and validate that the include directory
really exists, it would still just find whatever version is installed.

The proper way is to either call find_package with VERSION or set
<Module Name>_FIND_VERSION inside the module.

Additionally: If we don't have a version from package config, the code now tries to find the version from the header file.

Motivation and Context

Fix.

How Has This Been Tested?

Compile tested on xenial (without build-deb ppa). Btw @wsnipex, will there gonna be a build-dep ppa for xenial?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@fetzerch fetzerch added CMake Type: Fix non-breaking change which fixes an issue labels Nov 19, 2016
@fetzerch fetzerch added this to the Krypton 17.0-beta6 milestone Nov 19, 2016
@fetzerch
Copy link
Member Author

jenkins build this please

PATHS ${PC_CEC_INCLUDEDIR})

set(CEC_VERSION ${PC_CEC_VERSION})
if(NOT PC_CEC_VERSION)

This comment was marked as spam.

This comment was marked as spam.

Just setting a pkgconfig minimum version doesn't work and just causes
pkgconfig to not find an older package. But since our CMake modules
don't trust package config and validate that the include directory
really exists, it would still just find whatever version is installed.

The proper way is to either call find_package with VERSION or set
<Module Name>_FIND_VERSION inside the module.

Additionally: If we don't have a version from package config, find it
in the header file.
@fetzerch
Copy link
Member Author

@wsnipex good to go then?

@wsnipex wsnipex merged commit 76d969b into xbmc:master Nov 22, 2016
@akva2
Copy link
Contributor

akva2 commented Nov 22, 2016

this won't be the only such error if you didn't consider this aspect of the do-not-trust-pkgconfig change. just fyi, i had used the pkgconfig version check in more find modules originally.

@fetzerch fetzerch deleted the cmake_cec branch December 10, 2016 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants