Proposed FCL build/technical debt improvements

[sherm1]

Per face-to-face discussion at TRI with @jslee02, @SeanCurtis-TRI, @damrongguoy, and me:

We would like to upgrade FCL development towards modern best-practices. This would include:

• improvements to build system for speed, robustness, and transparency
• set up a build dashboard
• adoption of a uniform coding style guide (likely Google/Drake)
• gradual reduction of technical debt
• (edited) add test coverage to the CI (e.g., kcov)

In the latter category, we are proposing to focus future convex object handling on the built-in GJK/EPA algorithms with the intent of deprecating the libccd dependency eventually. That would allow us to extend Eigen use and differentiability through those algorithms.

We would continue to maintain the current API to the extent possible, with smooth deprecation where necessary.

We would like to hear ASAP from anyone who has further ideas for improvement or concern with this plan.

/cc @jamiesnape

[sherm1]

/cc @panjia1983

[jamiesnape]

It may be a little disruptive, but I would also propose dropping the artificial separation of headers and sources into separate directories include and src. It makes developing a modular, modern CMake build system rather more complicated.

(something like this: https://github.com/jamiesnape/fcl/tree/wip-modern-cmake)

[jamiesnape]

Note that CMake also now has better support for creating Debian and Ubuntu compliant packages, so we can automate the release process (relates #331).

[sherm1]

It may be a little disruptive, but I would also propose dropping the artificial separation of headers and sources into separate directories include and src.

I imagine that would be quite disruptive for FCL users (any users care to weigh in?). I'm guessing that many work with the source directory structure rather than an installed FCL (where headers could be relocated). How important is it to reorganize these, @jamiesnape -- I would think CMake could be made to deal with it as is.

[jamiesnape]

Installed or otherwise is completely separate to this, so not quite sure what you mean. I would have thought having the sources next to the relevant headers would be distinctly easier for developers.

[jamiesnape]

Note also that headers are not really relocated modulo the name of the top-level directory may change. If you pointed at fcl/include before, pointing to fcl would have the same effect. It is the source files that move.

[jamiesnape]

I would think CMake could be made to deal with it as is.

It can, but that does not mean it should. You will have more maintainable code and a better user experience with a one-off merge of directories, and it does not even change the API.

[SeanCurtis-TRI]

I would imagine there would be some minor difference. Putting .cpp files into a directory tree with "include" at its root seems a bit odd. If others feel that way, that would imply movement of the include files. This might impact developers workflow because they'd have to redirect their build system to look in the right place. I don't think this is particularly arduous, and I think it would be consistent with other technical debt issues we're looking at.

For the sake of completeness and to assuage the mind of anyone else who's reading this, I wouldn't advocate doing it until we release the current state into a new FCL public release and defer this reshuffle for the next phase.

[jamiesnape]

So your root directory would change, yes, but if you are using CMake correctly you are not hard-coding that anyway.

[scpeters]

@jamiesnape I believe there are some IDE's that make it very easy to open header files even if they are not adjacent to the src files, so the value of co-locating them is not clear to me. Also, I think it's also easier to know which header files will be installed if they are in a separate folder from the uninstalled cpp and header files. So I personally prefer the current folder structure.

[scpeters]

improvements to build system for speed, robustness, and transparency

Does this involve bazel?

[jamiesnape]

You are saying people have to use an IDE to work around a deficiency. I think here all the includes are installed in any case.

Does this involve bazel?

Not in my understanding. CMake can be fast, robust, and transparent, if you write good CMake that is not held back by legacy code and conventions.

[jamiesnape]

Plus this is a cross-platform library, Windows support with Bazel is pretty poor, and Bazel does not have the concept of an install, custom coding all the missing features that we had to do with Drake, would take a considerable amount of time, not least because we only support Ubuntu and Mac in Drake.

[SeanCurtis-TRI]

Correct. The intent is to leave CMake intact. But just to make other aspects of the library more robust and mature to facilitate continued development.

[jslee02]

I used to prefer having headers and sources in the same location for the same reasons @jamiesnape pointed out but now I'm inclined to have them separately because of the reasons @scpeters said (it's clearer which headers should be installed). I think we can handle either structure by CMake, but also personally prefer the current folder structure.

[jamiesnape]

I guess I am missing something here. I don't see a single header inside src. In any case, there are plenty of ways of distinguishing private headers that give you the best of both worlds.

[scpeters]

I guess I am missing something here. I don't see a single header inside src

I was speaking in general, as I maintain other packages with private headers in a src folder.

In any case, there are plenty of ways of distinguishing private headers that give you the best of both worlds.

Can you elaborate on this?

[jamiesnape]

I was speaking in general, as I maintain other packages with private headers in a src folder.

Every project is different, for this project I would say the structure offers little value; for other projects, especially other build systems, things may be different.

Can you elaborate on this?

Naming conventions are the most obvious, if you want a build system independent solution. This project has a convention -inl.h for inlined headers, for instance, which is a clear signal to the developer not to include it directly that does not need a separate directory.