Incremental build bug? Changing project option default isn't registering on incremental builds.

Usage
,
1

Here is a link to a public repository to show the problem:

The instructions are in the readme.md but I’ll repeat them here:

cmake_default_option_issue

By default cmake options do not support incremental builds. If a component A imports component B and B defines an option (say FOO) that A does not explicitly set, any change (in B) of the default value of FOO will be silently ignored by cmake (when doing incremental builds). With the config helper the incremental build will fail, which is still not good but better than silently producing bad binaries.

How to reproduce:

Run cmake and build normally. Run the executable and see this message.

.\A.exe FOO = ON

In B/CMakeLists.txt change the default for FOO from ON to OFF (o vice versa)

Do an incremental build. The change from step 2. is ignored. Cmake re-runs but nothing is rebuilt

.\A.exe FOO = ON
2

I’m just going by memory but doesn’t the “option” command add the variable into the cache?

Then subsequent uses of “option” are ignored as it’s already present in the cache.
You would then have to explicitly set the option value you want as the new default is ignored.

Refer to CMP0077 and the option command.

3

You are correct

option(foo “Documentation” ON)

is the same as

set(foo ON CACHE BOOL “Documentation”)

But I was hoping to point out that this behavior might not be desirable for projects.

And or is non-obvious.

Because if I absorb a project that is for example defaulting an option to be off since it is experimental code. Then after being fully tested they change the default to be on. Then I don’t get the new default.

4

If you want your project to follow the default options, then you need to delete those options from the cache (and any other cache variables that might depend on them) and re-run CMake. This will result in the new defaults being picked up. Look into the -U option of the cmake command line tool, or you can select which variables to remove from the cache interactively if using ccmake or the CMake GUI application.

5

This issue is related to what you want: https://gitlab.kitware.com/cmake/cmake/issues/14756

There’s another to have an explicit default feature for cache variables. This is emulated in VTK and ParaView in a few places. https://gitlab.kitware.com/vtk/vtk/blob/76d9b339869fef2241bcf7d2310cddda12916799/CMakeLists.txt#L298

6

Thanks Ben the VTK link hit the point exactly!

7

Would it make sense to create a generic cmake module that encapsulates what VTK is doing?

include (option_default)

option_default(foo …)

8

I wouldn’t mind seeing a module offer such functionality.

9

I’ve created and tested an implementation of option_default() this fixes the issue described in this post.

Feel free to try it and look at the implementation (which is basically an abstraction of the VTK implementation):

10

You might want to also take in another TYPE parameter. Having option end up with a STRING variable seems…odd as well. And mark_as_advanced should also be left to the caller to decide. I’m also a chronic quoter in CMake :slight_smile: .

function(set_with_default variable help_text type value)
  set("${variable}" "<DEFAULT>" CACHE "${type}" "${help_text}")
  if (variable STREQUAL "<DEFAULT>")
    set("${variable}" "${value}" PARENT_SCOPE)
  endif ()
endfunction ()
11

100% agree I was mindlessly copying the VTK thank you very much for the feedback :slight_smile:

One note I just tested the function and variable needs to be called with this syntax in order to work ${variable}

function(set_with_default variable help_text type value)
  set("${variable}" "<DEFAULT>" CACHE "${type}" "${help_text}")
  if (${variable} STREQUAL "<DEFAULT>")
    set("${variable}" "${value}" PARENT_SCOPE)
  endif ()
endfunction()
12

Do you think there is enough merit to try and make this a standard module?

13

I personally think it has enough utility. Even though it is short, it’s another useful tool in the toolbox and a decent convention to have (though bikeshedding over the spelling of <DEFAULT> is possible).

Note that another approach (though it involves a lot more work), is to add a DEFAULT property for CACHE variables and have CMake do this internally. This approach would also assist with this issue.

1 Like
14

Either of those options sounds great.

Just to clarify is this what you mean?

set(<variable> <value>... CACHE <type> <docstring> [FORCE] [DEFAULT])
option(<variable> "<help_text>" [value] [DEFAULT])

set(foo "Hello" CACHE STRING "good documentation" DEFAULT)
option(bar "..." ON DEFAULT)
15

Or what about this?

set(<variable> <value>... CACHE <type> <docstring> [FORCE] [NO_DEFAULT])
option(<variable> "<help_text>" [value] [NO_DEFAULT])

set(foo "Hello" CACHE STRING "good documentation")
option(bar "..." ON)
16

Something like this may work (I expect there to be design discussions, so I recommend opening an issue and Cc’ing at least me, @craig.scott and @brad.king). This could work as-if:

set(<variable> "<DEFAULT>" ... CACHE <type> <docstring> [FORCE])
set_property(CACHE <variable> PROPERTY DEFAULT <value>)

I suppose that CMake will have to do the <DEFAULT> translation at its cmCacheManager level to make this work. I don’t know if FORCE DEFAULT makes sense either.

Eh, booleans are harder to do here, so any option() support should be considered after we have the groundwork laid out. This is mostly a GUI (and curses) thing though, so maybe not that bad.

17

Please don’t let this topic die.

A lot of projects I’ve worked on have a terrible understanding of cache variables and they think when they update their defaults it will propagate correctly. And of course it doesn’t (unless the user deletes their cache, which people who are new to cmake don’t know to do). This has literally been a roadblock to cmake adoption at my current company.

Resulting in annoyed developers complaining to the build person. Resulting in resistance to using cmake as a build system (from my companies experience).

While setting a property like was mentioned by @ben.boeckel works.

It’s way too advanced for new cmake writers. I think it should be a policy change.

That way we can keep the syntax for set/option. Without adding complexity for people new to cmake.

I don’t think anyone really wants the existing behavior anyway (at least the people I’ve worked with). Also granted everyone I work with is just getting used to the idea of cmake. (Which I think is a perfect argument in my favor, cmake should be intuitive to new users).

And if people really want to use the old behavior then they can set the policy to use the old behavior.

In summary the policy would effectively make option/set do exactly what @YMba9g8j9CJp0wLoQf5y’s set_with_default does and I think everyone would be happier. I don’t see how this would even hurt existing projects.

18

Plus making this a policy change would make it so you wouldn’t have to write a CMAKE_DEPENDENT_OPTION_DEFAULT.

19

The problem with any other naive setting is that it becomes indeterminate whether a cache variable that happens to be set to the default has that value because it was left as the default or was explicitly chosen to be that value (in which case, it should not be updated). Whatever new behavior is available, this distinction must be possible. As such, the existing set() behavior cannot be sufficient.

That’s not how policies work. Policies are there for backwards compatibility only, not feature behavior selection. A policy says “this is how CMake prefers to work now, but we’re supporting the old behavior until you upgrade”. Setting policies to OLD should go away as projects bump minimum CMake versions.

1 Like
20

That makes sense. What about the suggested syntax above? I feel like it’s much more clear than setting a property.

set(<variable> <value>... CACHE <type> <docstring> [FORCE | DEFAULT])
option(<variable> "<help_text>" [value] [DEFAULT])