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

2016-10-06 Thread Stephen Kelly
Brad King wrote:

> On 10/05/2016 06:38 PM, Stephen Kelly wrote:
>> 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
> 
> That makes sense, but the codelite feature is just following a
> long-standing convention used for many settings.  If you want to
> make a sweeping effort to formalize this kind of scoping then that
> is fine with me.  Mostly I think it will be documenting that the
> value of the variable at the end of the top-level CMakeLists.txt
> file is the one that is used.  In some cases that may involve
> fixing generators to actually use that one.

Ok. I started by adjusting the new CodeLite feature. I based it on an early 
commit so that it can be merged to the release branch. The sweep belongs in 
master for the following release I think.

Thanks,

Steve.


-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers


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

2016-10-06 Thread Brad King
On 10/05/2016 06:38 PM, Stephen Kelly wrote:
> 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

That makes sense, but the codelite feature is just following a
long-standing convention used for many settings.  If you want to
make a sweeping effort to formalize this kind of scoping then that
is fine with me.  Mostly I think it will be documenting that the
value of the variable at the end of the top-level CMakeLists.txt
file is the one that is used.  In some cases that may involve
fixing generators to actually use that one.

-Brad
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers


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

2016-10-05 Thread Stephen Kelly
Craig Scott wrote:

> I'm coming in half way to this discussion, so apologies if my comments
> interspersed below are not so well related to the core topic of
> discussion.

Hi Craig,

Thanks for your input.

> Consider the following example which perhaps better shows that this
> problem may not be as uncommon as first thought:
> 
> set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -someOption")
> add_library(foo ...)
> set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -somethingElse")
> 
> I think most developers probably expect foo to not have the -somethingElse
> option when it is compiled, but I believe it would have it. 

The difference occurs depending on when the value is read. See my 
CMAKE_CXX_STANDARD example below.

> If I understand things correctly, directory *properties* don't typically
> have this unexpected behaviour as their value at the time of defining the
> targets is used, not at the end of that directory's processing. 

It all depends on when the value is read. Something that is read at 
configure-time appears to have immediate effect, regardless of whether it is 
a 'directory property' or a set() 'definition'. 

When something that is read at generate-time (such as CMAKE_CXX_FLAGS) it 
takes on the value at the end of the directory.

You might be referring to things like this, which are still 'definitions' 
not 'directory properties':

 set(CMAKE_CXX_STANDARD 11)

 # Reads CMAKE_CXX_STANDARD 'now' at configure time
 add_executable(foo ...)

 set(CMAKE_CXX_STANDARD 14)

 # Reads CMAKE_CXX_STANDARD 'now' again!
 add_executable(bar ...)


The important distinction is where in the CMake C++ code the 
cmMakefile::GetDefinition call occurs - generate-time code or configure-time 
code.

This is a sideline just for your information. What this thread is really 
about is whether things should be read 'only once' or 'once per directory', 
and whether the CMake C++ code chooses one or the other deliberately or 
accidentally, and what impact that has on maintainability and refactoring.

> this behaviour of using the
> variable's value at the end of the directory processing is likely a
> surprise to many and probably already causes some head-scratching until
> devs figure it out. Is the problem being discussed here relating to
> CMAKE_CODELITE_USE_TARGETS
> much different?

It seems like a related example to me. The CMAKE_CODELITE_USE_TARGETS 
feature is trying to be related to a project() command, but it is read at 
the end of the directory. Usually, I would think that is not a problem the 
way most people write project() commands and set these kinds of settings.

However, in the general sense of how 'global' settings should work, I think 
things could be better.

>> 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.
>>
> 
> Perhaps a bit more common than that, as the above example suggests.

Yes, those kinds of examples are the things that I would expect to arise 
during maintenance, perhaps several releases after a feature hits master.

Thanks,

Steve.


-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers


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

2016-10-05 Thread Craig Scott
I'm coming in half way to this discussion, so apologies if my comments
interspersed below are not so well related to the core topic of discussion.

On Thu, Oct 6, 2016 at 9:38 AM, Stephen Kelly  wrote:

> Brad King wrote:
>
> > 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.
>

Consider the following example which perhaps better shows that this problem
may not be as uncommon as first thought:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -someOption")
add_library(foo ...)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -somethingElse")

I think most developers probably expect foo to not have the -somethingElse
option when it is compiled, but I believe it would have it. Given that it
is not unusual (but not necessarily wise) for projects to fiddle with
variables like CMAKE_CXX_FLAGS in subdirectories which could be brought in
via include() rather than add_subdirectory(), this behaviour of using the
variable's value at the end of the directory processing is likely a
surprise to many and probably already causes some head-scratching until
devs figure it out. Is the problem being discussed here relating to
CMAKE_CODELITE_USE_TARGETS
much different?

If I understand things correctly, directory *properties* don't typically
have this unexpected behaviour as their value at the time of defining the
targets is used, not at the end of that directory's processing. They serve
as defaults for target-specific properties at the point of the target being
defined. Not sure if that helps with the original topic of discussion here
though.



>
> 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.
>

Perhaps a bit more common than that, as the above example suggests.


-- 
Craig Scott
Melbourne, Australia
https://crascit.com
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Generator options per-directory v. global (was: CMake 3.7.0-rc1 now ready for testing!)

2016-10-05 Thread Brad King
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.
Such cases could have documentation updated.

Some of them may be per-`project()`.

> 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.  The scoping doesn't
match the generator semantics exactly, but it is easy to use and
hasn't been a big problem.

-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers