espresso icon indicating copy to clipboard operation
espresso copied to clipboard

Reorganize core library

Open fweik opened this issue 6 years ago • 6 comments

The code in the core library should be split into src, include and tests folders. This allows for public (those in include) and private (those in src) header files and makes clear what is exported. In cmake one would then only expose the include folder to other targets.

fweik avatar Feb 25 '19 16:02 fweik

https://api.csswg.org/bikeshed/?force=1&url=https://raw.githubusercontent.com/vector-of-bool/pitchfork/develop/data/spec.bs#tld.build

KaiSzuttor avatar Mar 31 '19 21:03 KaiSzuttor

https://github.com/vector-of-bool/pitchfork

KaiSzuttor avatar Mar 31 '19 21:03 KaiSzuttor

https://github.com/tomtom-international/cpp-dependencies

KaiSzuttor avatar Mar 31 '19 21:03 KaiSzuttor

Also #2628.

fweik avatar Apr 01 '19 09:04 fweik

Would we have to update .codecov.yml? E.g. in the current state, changing codecov/project/tests to

        paths:
          - src/core/unit_tests/
          - src/pdbparser/unit_tests/
          - src/utils/tests/

and codecov/project/core to

        paths:
          - src/core/
          - !src/core/unit_tests/

(I'm no expert in the codecov YAML syntax, and their docs aren't very helpful, so take these code blocks with a grain of salt.)

jngrad avatar May 10 '19 12:05 jngrad

PR #4541 re-organized src/core and src/script_interface by grouping similar features into submodules:

  • src/core/*.{h,c}pp files are now for the most part declaring or using global variables (snake case filenames), while src/core/*/*.{h,c}pp mostly contain class definitions (Pascal case filenames)
  • src/script_interface/*.{h,c}pp files now all contain the ScriptInterface classes and helper functions, while src/script_interface/*/*.{h,c}pp contains the consumer code that bridges the core classes to the Python classes

I think we should move forward with the Pitchfork layout. In the last 12 months, while refactoring the core and script interface, I found myself in situations where I had to create multiple header files for one source file to comply with separation of concerns or reduce compilation times. The Pitchfork layout automates this by having an include folder for header files of the public interface that is passed to target_include_directories(), and a source folder that contains private header files and source files.

Another benefit is that name collisions becomes impossible. This is important for the script interface, since interface classes sometimes have the same name as the corresponding core classes. This is probably why we have this crude workaround: https://github.com/espressomd/espresso/blob/18d49bfc4b6ca5be264f16d262338fda1f9141a7/src/script_interface/CMakeLists.txt#L54-L55

The script interface includes the entire project structure, such that one can write #include "core/cells.hpp" instead of #include "cells.hpp", which would be ambiguous if a Cells.hpp class existed in the script interface; filenames in C++ can be case-insensitive and some compilers will throw -Wnonportable-include-path if a name collision occurs between two filenames that only differ in case. It took me a while to figure out why AppleClang was rejecting the Version.hpp filename, and I couldn't replace #include "version.hpp" by #include "config/version.hpp" since that file is auto-generated and lives in the build directory. I ended up renaming the class to CodeVersion.hpp in 1fba163e86a49fe9e0bd46b132925eac91b160de as a workaround.

jngrad avatar Aug 05 '22 15:08 jngrad