[cmake-developers] RFC/Review Request: Topic GNUInstallDirs_debian-multiarch-fix

[Daniele E. Domenichelli]

On 15/01/14 16:27, Brad King wrote:
> On 01/15/2014 10:11 AM, Daniele E. Domenichelli wrote:
>> +    get_property(_libdir_set CACHE PROPERTY CMAKE_INSTALL_LIBDIR SET)
> 
> That should be
> 
>  get_property(_libdir_set CACHE CMAKE_INSTALL_LIBDIR PROPERTY TYPE)

Is there any issue in using
  get_property(_libdir_set CACHE CMAKE_INSTALL_LIBDIR PROPERTY TYPE SET)
instead?

When reading the code, the "SET" makes it easier for me to understand
that I want to know whether the variable is set or not, and I don't
care about the value... But I don't know if there is some performance
issue or any other reason not to use it.
Also perhaps it is possible that the user will set it to an empty
string? (Not sure if it really makes sense)


> Also I realized that __LAST_CMAKE_INSTALL_PREFIX should be called
> _GNUInstallDirs_LAST_CMAKE_INSTALL_PREFIX since it holds the value
> that this *module* last used as CMAKE_INSTALL_PREFIX.

Ok, renamed.


 
> Hmm...will this logic work correctly if GNUInstallDirs is included
> in more than one place in a project?  The test:
> 
> +if(NOT DEFINED CMAKE_INSTALL_LIBDIR
> +    OR NOT "${__LAST_CMAKE_INSTALL_PREFIX}" STREQUAL "${CMAKE_INSTALL_PREFIX}")
> 
> should not succeed the second time.  Please verify with manual
> testing.  Also this line should be tweaked to not consider the
> __LAST_CMAKE_INSTALL_PREFIX if it is not defined.

I moved the test for _libdir_set at the beginning, and added a check if
__LAST_CMAKE_INSTALL_PREFIX (now
_GNUInstallDirs_LAST_CMAKE_INSTALL_PREFIX) is defined. Now the test is
this:

  get_property(_libdir_set CACHE CMAKE_INSTALL_LIBDIR PROPERTY TYPE SET)
  if(NOT DEFINED CMAKE_INSTALL_LIBDIR OR (_libdir_set
      AND DEFINED _GNUInstallDirs_LAST_CMAKE_INSTALL_PREFIX
      AND NOT "${_GNUInstallDirs_LAST_CMAKE_INSTALL_PREFIX}" STREQUAL "${CMAKE_INSTALL_PREFIX}"))

If CMAKE_INSTALL_LIBDIR is not defined, it is always executed.
Otherwise
 * if _libdir_set is false it is not executed (meaning that it is not
   a cache variable)
 * if _GNUInstallDirs_LAST_CMAKE_INSTALL_PREFIX is not defined it is
   not executed
 * if _GNUInstallDirs_LAST_CMAKE_INSTALL_PREFIX and CMAKE_INSTALL_PREFIX
   are the same string it is not executed.
   _GNUInstallDirs_LAST_CMAKE_INSTALL_PREFIX is updated after the
    execution, of this part of code, therefore at the next inclusion
    of the file, CMAKE_INSTALL_LIBDIR is defined, and the 2 strings are
    equal, meaning that the if is not executed the code the second time.

It is a bit complicated, but I added some documentation...



I'm still not completely sure about what should happen if both the
cache and normal variable exist, i.e. if one runs cmake
-DCMAKE_INSTALL_LIBDIR=foo, and inside the code, before the inclusion
of GNUInstallDirs there is a set(CMAKE_INSTALL_LIBDIR bar).
Is this a possible use case? I think that at the moment the code is
still executed, but the comparison will be between the value computed
from the previous prefix and the _normal_ variable, but eventually the
replaced variable will be the _cache_ one. The user should still see
the normal one, until he goes out of scope, though.
I believe it should either skip this code (but I don't know if it is
possible to check if a normal variable exists, or instead compared with
the cache variable, and update it if required. What do you think?

Thanks,
 Daniele