Sankel feedback on BCM
Overall, I have two big concerns about this approach:
- It is too tied to how Boost builds things and/or has incompatible Boost-specific workarounds.
- BCM provides an alternative to the typical CMake conventions. I'd much rather see standard CMake commands being used when possible, even if this means there'll be more code and repetition. I've seen similar shorthands made in several different projects and it has always resulted in harder-to-approach code and leaky abstractions. For maintainability, conventional CMake combined with a good coding standard seems to work the best.
It is too tied to how Boost builds things and/or has incompatible Boost-specific workarounds.
The bcm_boost_package is very boost-specific, however, the rest of the functions can work with any project. I should provide an overview on how to use these modules in non-boost projects.
I've seen similar shorthands made in several different projects and it has always resulted in harder-to-approach code and leaky abstractions.
I've seen similar things as well(for example trilinos), but do you think these modules suffer the same issues?
I would like for boost libraries to be written in high-level way as it allows for us to generate a lot of things directly from that. For other libraries that are not boost-specific, they can use bcm_package or bcm_install_targets or bcm_auto_export where it makes sense, and use built-in cmake for everything else.
Of course, any abstraction can have the problems you mentioned, I want to make sure this is not the case here. Thanks for the feedback.
On Mon, 2017-07-24 at 05:22 +0000, Michael H. Cox wrote:
@mikehcox commented on this pull request. In doc/src/Intro.rst:
@@ -76,6 +90,12 @@ For dependencies on other boost libraries, they can be listed with the ``DEPENDS config ) +.. This is interesting. I like the reduction in boilerplate, but I question the + choice to go in such a different direction from the standard CMake way of + doing things. It strikes me as a possibly leaky abstraction where folks will + have a mix of code which uses both the CMake way and the BCM way; maintainers + will have to be familiar with both. Definitely agree with camio on this. See this Youtube video by Daniel Pfeiffer https://www.youtube.com/watch?v=bsXLMQ6WgIk Having a set of guidelines, templates, and conventions on how to use modern CMake (3.0+) is a better way to go than reinventing CMake's conventions.
I have actually removed this function. I totally agree, I don't want to write functions that wraps cmake's current functions. My original goal was to make something easier for developers who do not want to learn cmake, but this does lead to maintenance problems.
I am trying to follow the model the Daniel has outlined in his Effective CMake talk. However, this does lead to repeating the dependencies 3 times.
Now I have refactored this, so the user creates the library using standard
cmake, and then call bcm_deloy which will do the install and package
configuration. This does reduce down the repetition without reinventing cmake
conventions. You can see a comparison here:
http://bcm.readthedocs.io/en/latest/src/Building.html
Please let me know if you still think this is too much. I really want to avoid creating cmake functions like what is used in trillinos or llvm that seems to make the build more complicated. The ultimate goal is to build something that can be merged into cmake eventually. So that the function we provide are standard cmake functionality.