TriBITSPub / TriBITS

TriBITS: Tribal Build, Integrate, and Test System,
http://tribits.org
Other
36 stars 46 forks source link

Document undocumented CMake -C *.cmake FORCE set() behavior #525

Closed bartlettroscoe closed 2 years ago

bartlettroscoe commented 2 years ago

See git commit msg and the updated documentation.

This was learned as documented in https://github.com/trilinos/Trilinos/pull/10930#issuecomment-1227637727.

bartlettroscoe commented 2 years ago

@bradking and @KyleFromKitware, can you please look over this and see if there is any CMake documentation that describes this behavior? Is this a defect in CMake or was this planned?

@e10harvey, this impacts how the GenConfig files work with Trilinos.

KyleFromKitware commented 2 years ago

Hmmm. I'm not seeing this scenario covered in any of the CMake tests, and I can't find it in the documentation either. I'm guessing it's a defect. @bradking do you know anything about this?

KyleFromKitware commented 2 years ago

Actually, this may even be a regression due to some restructuring of the command line argument handling. I'll test some older versions and see if this still holds true.

KyleFromKitware commented 2 years ago

So it turns out that the truth is even more nuanced than what this documentation suggests. If you put the -D after the -C:

cmake -C initial_cache.cmake -DMYOPT=cmdline .

then initial_cache.cmake will not override MYOPT. However, if you switch the order and put the -C after the -D:

cmake -DMYOPT=cmdline -C initial_cache.cmake .

then the cache file will override the -D option. This is true in both the latest master and as far back as v3.10.0. I don't know whether or not it was intentional.

bartlettroscoe commented 2 years ago

So it turns out that the truth is even more nuanced than what this documentation suggests.

@KyleFromKitware, that is not what I experienced in my experiments with CMake 3.18.0 documented in the details of https://github.com/trilinos/Trilinos/pull/10930#issuecomment-1227637727.

Are you suggesting the behavior is different based on the version of CMake?

bartlettroscoe commented 2 years ago

But log story story, if you use the TriBITS option -D <Project>_CONFIGURE_OPTIONS_FILE=<frag>.cmake, this results in an include(<frag>.cmake) call at the base project level CMakeLists.txt file and forced set() works as you would expect (it overrides the cache var any way it may be set).

KyleFromKitware commented 2 years ago

that is not what I experienced in my experiments with CMake 3.18.0 documented in the details of https://github.com/trilinos/Trilinos/pull/10930#issuecomment-1227637727.

Did you try switching the order of the -D and -C arguments?

KyleFromKitware commented 2 years ago

Are you suggesting the behavior is different based on the version of CMake?

No. I tried it in both 3.10.0 and the latest master and got the same behavior.

bartlettroscoe commented 2 years ago

Did you try switching the order of the -D and -C arguments?

@KyleFromKitware, okay, I verified above as documented in https://github.com/trilinos/Trilinos/pull/10930#issuecomment-1227808785.

I will update the documentation to note this order sensitivity.

bartlettroscoe commented 2 years ago

@KyleFromKitware, can you please review the updated documentation in the commit that I just pushed https://github.com/TriBITSPub/TriBITS/pull/525/commits/143b143645cecd79391644b529ccb9a895fa492d?

bartlettroscoe commented 2 years ago

CC: @e10harvey

@KyleFromKitware and @bradking, now that this behavior is documented for TriBITS users, does it make sense to document this for CMake proper? It might have saved me quite a bit of time scratching my head about what I was seeing and therefore I wonder if other people have struggled as well.

NOTE: After thinking about this some more, I think the current CMake implementation and behavior might make sense. If you view the set(<someVar> <val1> ... FORCE) statements in a -C <frag>.cmake file as equivalent as command-line sets -D<someVar>=<val1>, then the behavior based on the order should matter. For example, if you list:

$ cmake -DVar1=val1 -DVar1=val2 ...

the the value of the cache var Var1 will be val2 because it comes last.

Likewise, if the file frag.cmake contained:

set(Val1 val1 CACHE STRING "" FORCE)

and you called:

$ cmake -C frag.cmake -DVal1=val2 ...

then this is really equivalent putting all of the set() statements on the command-line as:

$ cmake -DVar1:STRING=val1 -DVar1=val2 ...

and you would expect the value to be val2.

But in the TriBITS case of using -D<Project>_CONFIGURE_OPTIONS_FILE=<frag>.cmake, that file does not get processed until it is included inside of the project's base CMakeLists.txt file and therefore would behave the same as a native set(<var>.... FORCE) statement (and therefore override all ways the cace var <var> may have been set, including using -D<var>=<val> on the command-line).

But boy this is going to confusing all kinds of people (as it confused me).

KyleFromKitware commented 2 years ago

does it make sense to document this for CMake proper?

Yeah, I think it makes sense. I'll work on updating the documentation.

KyleFromKitware commented 2 years ago

Please see https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7614.