rockowitz / ddcutil

Control monitor settings using DDC/CI and USB
http://www.ddcutil.com
GNU General Public License v2.0
978 stars 40 forks source link

FindDDCUtil.cmake does not provide ddcutil version #129

Open ypnos opened 4 years ago

ypnos commented 4 years ago

In your API you have breaking changes based on version. However, the supplied Find script for CMake does not determine nor expose the installed version in a variable.

I would like to patch KDE's powerdevil to support version 0.9.9 without breaking compatibility to older versions. The only change needed to be based on version number would be a call to ddca_feature_list_string. A CMake variable DDCUTIL_VERSION would be helpful.

rockowitz commented 4 years ago

Header file ddcutil_macros.h has #defines for the version number, and ddca_ddcutil_version()/ddca_ddcutil_version_string() report the version at execution time.

Can you describe in more detail what you want to do?   If FindDDCUtil.cmake could report the version, how would you use it?

I have to admit that FindDDCUtil.cmake was supplied by a user some years ago, and I haven't given it much thought since.  ddcui's CMakeList.txt uses pkg_check_modules() to check for libddcutil.

Sanford

On 6/28/20 11:29 AM, ypnos wrote:

In your API you have breaking changes based on version. However, the supplied Find script for CMake does not determine nor expose the installed version in a variable.

I would like to patch KDE's powerdevil to support version 0.9.9 without breaking compatibility to older versions. The only change needed to be based on version number would be a call to |ddca_feature_list_string|.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rockowitz/ddcutil/issues/129, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADMGY3XYP54LEOENS5WK6ODRY5OWLANCNFSM4OKSP5LA.

ypnos commented 4 years ago

Thank you. I was not aware of the macro so that would solve the problem for my case.

Exposing the version through CMake has the advantage that it can be used within CMake, e.g. to ensure that a compatible version is installed/selected for compilation. After reading through https://cmake.org/cmake/help/v3.0/module/FindPkgConfig.html, I assume that actually DDCUTIL_VERSION variable is present! Just not mentioned in the comments of FindDDCUtil.cmake.

rockowitz commented 4 years ago

FindDDCUtil.cmake is installed as part of the development package, as are the include files.  So I would think that all that's required is for FindDDCUtil.cmake to read ddcutil_macros.h.  It already looks for ddcutil_c_api.h to identify the include directory.  Or is there something more subtle going on here?

On 6/29/20 3:40 PM, ypnos wrote:

Thank you. I was not aware of the macro so that would solve the problem for my case.

Exposing the version through CMake has the advantage that it can be used within CMake, e.g. to ensure that a compatible version is installed/selected for compilation. However it is rather involved, you would need to put a version file somewhere that is used by the find script. So I'm not sure it's worth the work.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rockowitz/ddcutil/issues/129#issuecomment-651319633, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADMGY3THYUTRA64TU4B7HXDRZDU3JANCNFSM4OKSP5LA.

ypnos commented 4 years ago

I edited my comment to reflect that pkg_check_modules already does provide version information.

To answer your question:

  1. Yes, the macros header file could be parsed to extract the version.
  2. Mind that the presence of FindDDCUtil.cmake does not imply the presence of the development package. Software using it would typically ship its own version of it. It's how the CMake crowd does it. Not a big deal though, just in case you weren't aware.