[cmake-developers] Generator options per-directory v. global

[Stephen Kelly]

Brad King wrote:

> On 10/04/2016 05:46 PM, Stephen Kelly wrote:
>> This causes problems because now the code has to read the value for each
>> directory and can't assume that the value is always the same as the value
>> from the top-level CMakeLists file.
> 
> Many of these are honored only in the top-level directory anyway.

Some do...

 src/cmake$ git grep -c -e Makefiles --and -e "\[0\]" 
 Source/cmGlobalGenerator.cxx:6
 Source/cmGlobalUnixMakefileGenerator3.cxx:1
 Source/cmake.cxx:1

 src/cmake$ git grep -c -e LocalGenerators --and -e "\[0\]" 
 Source/cmExtraEclipseCDT4Generator.cxx:4
 Source/cmExtraKateGenerator.cxx:1
 Source/cmGlobalGenerator.cxx:1
 Source/cmGlobalKdevelopGenerator.cxx:1
 Source/cmGlobalNinjaGenerator.cxx:10
 Source/cmGlobalUnixMakefileGenerator3.cxx:6
 Source/cmGlobalVisualStudioGenerator.cxx:2
 Source/cmLocalNinjaGenerator.cxx:1
 Source/cmake.cxx:1



... but it is far more common for generate-time code to just get the closest 
cmMakefile and call GetDefinition on it.


> Such cases could have documentation updated.
> 
> Some of them may be per-`project()`.

CMake doesn't have an appropriately scoped data for that, AFAIK (touched on 
below).

>> Is the answer 'Use global properties or a cache variable instead'?
> 
> The options need to be something easy for the project to set itself
> or for a user to set.  

> A cache entry can work for that, but we don't
> really often read cache entries directly and instead read variables
> that fall back to cache entries if not defined.  

Yes, my suggestion is that this can be problematic.

> The scoping doesn't
> match the generator semantics exactly, but it is easy to use and
> hasn't been a big problem.

My mail is suggesting that it is a problem and is undesirable to maintain.

Big is subjective, and there are not many complaints, because generally 
people don't try to set things like this per-directory (and if they did it 
would probably mostly do what they expect). 

The problems are 

1) It is a behavior which is often not intended by the programmer.
2) It makes refactoring harder if such unintended behavior must be 
preserved.
3) It is unintuitive, because code such as 

 set(FOO ON)
 project(p)
 add_library(bar ...)
 set(FOO OFF)

looks like FOO is ON when defining the project and the target, but in 
reality it is only the value at the end of the directory that is consumed.
 
Those are not problems users or contributors adding features encounter, so 
that might affect a perception of 'big'ness. These problems only bubble up 
during refactoring or under longer-term maintenance when the true semantics 
of the code become known.

The suggestion to use the first cmMakefile for these kinds of definitions is 
a good one

1) It can be documented that the variable can only be set in the top level
2) It is what people already do probably
3) It is more convenient than the API for setting cache or global properties

In this CODELITE case, I can't intuitively understand where the value will 
come from in the case of a tree of directories, some of which contain 
project() and others which don't, others which contain multiple project() 
calls, and where the project() may appear in between add_subdirectory() 
calls.

I can't read the code and know that, because I know how subtle the 
projectMap stuff and the vector of cmLocalGenerator* is.

I think code such as 

 set(CMAKE_CODELITE_USE_TARGETS ON)

will be ignored if it occurs in a directory which does not contain a 
project(). I might be wrong though, and there may be other semantics of the 
code as written which are unexpected.

Should CMAKE_CODELITE_USE_TARGETS be changed to be 'global' and use 
something like the result of gg->GetMakefiles()[0]->GetDefinition() and 
should that be the model for how to access these kinds of settings in the 
future? 

cmGlobalGenerator could gain a GetGlobalSetting() for that purpose.

Thanks,

Steve.