Better place for workspace configuration

[stonier]

The parallel build directories are a better place for workspace configuration. e.g. you might have three parallel builds:

• debug
• release
• arm cross compile

and you'd probably use three different workspace.cmake files to modify variables like CMAKE_BUILD_TYPE and CMAKE_CXX_FLAGS et. al. You don't want to have to keep copying in and out of the source's root folder.

[stonier]

If going this way, workspace.cmake could possibly use a name that reflects its role more clearly.

[dirk-thomas]

Yes, having the workspace file in the source folder is not a good idea. Anyway the "magic" to look for that file in a specific location is totally opaque to the user. It would be cleaner if we would add an option like CATKIN_WORKSPACE_FILE(S) which is set get included in the beginning.

But this feature would not be available for catkin_make_isolated or plain cmake invocation. Can you achieve your goals also with a preload script which populate the cache (cmake -C)? This would work for all three build cases (plain, catkin_make and catkin_make_isolated). If yes I would propose to remove the workspace.cmake file handling completely from catkin.

[stonier]

I spent quite a bit of time fighting cmake to use an initial cache. It is possible, but more awkward and less transparent than just using the initial cache or a post-config workspace.cmake/rosconfig.cmake.

Case in example - CMAKE_CXX_FLAGS. On windows it is for finding include directories and for our arm cross compiles, it is used to set cpu specific compiler flags.

It is not possible to set CMAKE_CXX_FLAGS via the initial cache. The compiler detection will simply empty the variable and initialise it from CMAKE_CXX_FLAGS_INIT, which incidentally in many cases is also emptied on a whim and so not configurable from the cache (would be much better if they checked the cache first).

CMAKE_USER_MAKE_RULES_OVERRIDE is the current mechanism which does make it possible by letting you append to CMAKE_CXX_FLAGS_INIT.

So final conclusion - on windows we need a cache.cmake and an override.cmake, cross-compiling a toolchain.cmake, cache.cmake and override.cmake.

I'm ok with this I think - I can bury the details behind some scripts I write, which I'll try now and update back here.

Bit shirty at cmake for making compiler flags so difficult to manipulate.

[dirk-thomas]

So you would like to have the argument I mentioned to specify cmake files which than get included at the beginning?

[stonier]

I just tested my winros scripts (they work a bit like catkin_make), and I can simplify the -C <initial cache> and -DCMAKE_USER_MAKE_RULES_OVERRIDE=... under the hood easily enough. I'll be able to do the same for my cross compiling scripts as well.

As you said, important to have a method that works for isolated or plain cmake invocation of a single package, so I'm happy to continue that way and have the workspace.cmake option deleted.

[dirk-thomas]

Thanks for clarifying. I have removed the functionality.

[roehling]

@dirk-thomas Your change silently breaks all configurations that depend on settings in workspace.cmake. I just spent several hours to find out why our compiler flags were no longer applied.

Was it really necessary to remove the option without warning? It worked perfectly fine for our workflow, and now I have to remember some arcane -DCMAKE_USER_MAKE_RULES_OVERRIDE option each time I invoke catkin_make or start writing my own custom build scripts.

👎

[dirk-thomas]

This option was never really documented or recommended to be used anywhere so it was not really expected that it was used at all. The "solution" has several conceptional issues and was therefore removed (non-obvious side effect of a file being somewhere; since it was looked up in the source folder it was also not working well for different build types; etc.).
I am sorry for you inconvenience and will pay extra attention in the future to make such transitions easier. If you can imagine a clean way to provide the same level of convenience for your process please open a feature request for it.

[stonier]

I think dirk's earlier comment was important roehling...But this feature would not be available for catkin_make_isolated or plain cmake invocation. In other words, there were limitations. Having the cache and user override gets around those limitations and yeah, doing that by hand is a nuisance. But so is setting all your options by the command line.

As it is now, it's easy enough to build a convenience tool on top that works without limitations and provides convenience. We're prototyping something with yujin_tools in house that hides the override file and provides an easy to use default cache (amongst other things). Feel free to try that.

[roehling]

For now, I went with the simplest solution and wrote my own CMakeLists.txt that includes both workspace.cmake and the (symlinked) toplevel.cmake. I understand your concern about the non-obvious side effect of a file being somewhere.

However, I disagree about the problems with different build types. In our configuration, we use something like

set(CMAKE_CXX_FLAGS_FOO "....")
set(CMAKE_CXX_FLAGS_BAR "....")
if(NOT CMAKE_BUILD_TYPE)
    message(STATUS "No CMAKE_BUILD_TYPE selected. Using Foo as default.")
    set(CMAKE_BUILD_TYPE Foo)
endif(NOT CMAKE_BUILD_TYPE)

This works for arbitrary configuration names (you could use Foo and Bar literally here) and does not suffer from the aforementioned cache initialization problems.

[dirk-thomas]

A build configuration might consist of more than just "Debug" or "Release". I was referring to the fact that you can not have two different workspace.cmake files for different build configurations. It would required using additional arguments to conditionally enable certain commands or switch the workspace file which is both not really a great way to do it.

[roehling]

Just to wrap this up: From my point of view, you two killed a useful feature out of the blue, although it did not even break anything for you. From your point of view, you removed an undocumented kludge from the build system that was not meant to be used in production code and did not get the job done in the first place. Dirk apologized for the inconvenience, and I don't intend to hold any grudges. I can trivially undo your change for our local workspace setup, and if I can come up with a better solution, I will get back to you.

[stonier]

@roehling I certainly wouldn't have posted an issue if something wasn't broken for me. One of the design goals of catkin (see the catkin reviews on the wiki) was to support multiple builds. We needed this for mixing native and cross-compiling builds in our lab. As it was, this design goal was broken.