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

[Daniele E. Domenichelli]

On 15/01/14 15:29, Brad King wrote:
> On 01/14/2014 11:47 AM, Daniele E. Domenichelli wrote:
>> Updated the topic to behave in this way. Can you please review it again?
> 
> Please update the comment introducing __LAST_LIBDIR_DEFAULT to explain
> that it is the default we compute from __LAST_CMAKE_INSTALL_PREFIX,
> not a cache entry for the value that was last used as the default.

Fixed.

> 
> This line:
> 
> +          if(DEFINED ${__LAST_CMAKE_INSTALL_PREFIX})
> 
> should not have the ${}, right?

Yes, sorry... Fixed.


> In this line:
> 
> +set(__LAST_CMAKE_INSTALL_PREFIX "${CMAKE_INSTALL_PREFIX}" CACHE INTERNAL "CMAKE_INSTALL_PREFIX during last run" FORCE)
> 
> you don't need FORCE for INTERNAL cache entries.  Since the user cannot
> edit them anyway the set() command always forces the value.

I didn't know that, thanks for explaining. Fixed.


> There is one issue I neglected to raise last time around.  One of the
> design features of the current GNUInstallDirs implementation is that
> all the cache entries are created inside "if(NOT DEFINED ...)" guards.
> This allows projects to set the values as normal variables before
> including GNUInstallDirs to avoid having the entries cached or user-
> editable.
> 
> In order to preserve this feature you'll need to use get_property(CACHE)
> when CMAKE_INSTALL_LIBDIR is defined to make sure it is actually a cache
> entry before updating it.  If it is a plain variable then we should
> trust that the project intentionally set it as desired.


I changed it in this way:

-    set_property(CACHE CMAKE_INSTALL_LIBDIR PROPERTY VALUE "${_LIBDIR_DEFAULT}")
+    get_property(_libdir_set CACHE PROPERTY CMAKE_INSTALL_LIBDIR SET)
+    if(_libdir_set)
+      set_property(CACHE CMAKE_INSTALL_LIBDIR PROPERTY VALUE "${_LIBDIR_DEFAULT}")
+    endif()
+    unset(_libdir_set)

Will this work?


I updated the topic. Thanks!


Cheers,
 Daniele