search

BlueBrain / hpc-coding-conventions

Apache License 2.0
8 stars 5 forks source link

Move CMake options to configure the tools in a YAML file #128

Closed tristan0x closed 2 years ago

tristan0x commented 2 years ago

Status

Many variables are defined at CMake level. It makes them a bit complicated to overwrite them (#117), and it is required to run the configure phase to use the associated tools (ClangFormat, ...). Furthermore, the CLI utilities are unusable right away since most of their inputs are defined at CMake level.

Target

Proposal

Remove some CMake variables and put them in a new configuration file. hpc-coding-conventions project would provide a default one, but a parent project could provide its own.

The config file

in YAML format. Filename: .cppproject.yml Default file used by hpc-coding-conventions in `cpp/.cppproject.yml

List of CMake cache variables to remove

Content of the default configuration file

tools:

  # a tool is implicitly enabled when specified here

  # not all sections are mandatory, not mentioning a tool means it is not enabled.

  ClangFormat:

    # Any Python version specifiers as described in PEP 440
    # https://peps.python.org/pep-0440/#compatible-release
    # For instance:
    #
    # Exact match required: == 13.0.0
    # At least 13.0.1, but less than 13.1: >= 13.0.1, == 13.0.*
    # Same as above: ~= 13.0.1
    version: ==13.0.0

    # passed to clang-format command line. Can be:
    # - None
    # - a string
    # - a list of string
    option:

    include:
      match:
      - .*\.[it]?cc?$
      - .*\.hh?$
      - .*\.[chit]((pp)|(xx))$

    # targets that needs to be built before formatting the code
    cmake_deps:
    - build_cpp_headers

  CMakeFormat:
    version: ">=0.6, <0.7"
    include:
      match:
      - .*\.cmake$
      - .*CMakeLists.txt$

  ClangTidy:
    # static analysis with ClangTidy is explicitely disabled
    enable: False
    version: ">=7"
    option: -extra-arg=-Wno-unknown-warning-option
    include:
      match: .*\.c((c)|(pp)|(xx))$

  PreCommit:
    enable: False
    hooks:
      commit:
      - check-clang-format
      - check-cmake-format
      push:
      - clang-tidy
      - courtesy-msg

  # Global settings to every tool, can be locally overriden within the
  # section config of a tool

  global:
    exclude:
      match:
      - third[-_]party/.*
      - third[-_]parties/.*

Implementation details

Finding utilities like ClangTidy or CMakeFormat

Using Python version specifiers in the YAML file to describe the versions required is a move toward the idea to create a virtualenv to automagically install the required tools. Right now, the utilities like ClangTidy (an executable) or CMakeFormat (a Python package) are searched by CMake. It is probable that introducing Python version specifiers means the end of this and rely on Python to find (and maybe later install) the required tools.

1uc commented 2 years ago

Wow! Very nice. Can we open it up to do the equivalent for python? My personal interest is automatic formatting, so that's the context.

I would like to write in documentation something like: "Please use bbp-format.py format to format the entire code-base according to the BBP standards." It would be awesome if it would format all code (to be formatted), including C++ (done), CMake (done) and Python and anything else that needs formatting at BBP. (We can add this later, I'm happy to help.) Therefore, I'd slightly change the name of the file to something less C++ centric: .bbp_project.yml, .bbp_settings.yml, .bbp_config.yml, .hpc-coding-conventions.rc.

It would be super nice if we can easily enable pre-write hooks in editors (vim). Can we implement:

bbp-format.py format --file to_be_formatted.{cpp,py}

which will format the specified file only? (Changing files while they're loaded in an IDE/editor causes a little bit of disruption, hence the request.) Also it would select the write formatter automatically.

1uc commented 2 years ago

Does it need a way of agreeing on where to put compile_commands.json?

tristan0x commented 2 years ago

Wow! Very nice. Can we open it up to do the equivalent for python? My personal interest is automatic formatting, so that's the context.

Absolutely, we can extend the YAML to support other tools like Python. We could use Black, flake8 + flake8-import-order. I would first implement this issue and then add support for Python code formatting / analysis in another PR though.

I would like to write in documentation something like: "Please use bbp-format.py format to format the entire code-base according to the BBP standards." It would be awesome if it would format all code (to be formatted), including C++ (done), CMake (done) and Python and anything else that needs formatting at BBP. (We can add this later, I'm happy to help.)

👍

Therefore, I'd slightly change the name of the file to something less C++ centric: .bbp_project.yml, .bbp_settings.yml, .bbp_config.yml, .hpc-coding-conventions.rc.

Ok for .bbp_project.yaml! (.yaml is the official extension)

It would be super nice if we can easily enable pre-write hooks in editors (vim). Can we implement:

bbp-format.py format --file to_be_formatted.{cpp,py}

which will format the specified file only? (Changing files while they're loaded in an IDE/editor causes a little bit of disruption, hence the request.) Also it would select the write formatter automatically.

I like the idea to have a script per task, not per tool. I would make an even simpler synopsis for the formatting utility:

USAGE: format [options] [<file> or <dir> ...]

Format the given files or directories, the entire project otherwise.

OPTIONS:

  -n --dry-run : do not actually update the files, simply report formatting issues.
  --lang : only format the specified languages, default is: c++,cmake,python
tristan0x commented 2 years ago

Does it need a way of agreeing on where to put compile_commands.json?

The file compile_commands.json is written by CMake in the top build directory and AFAIK there is no way to modify the path or its name, so it is IMO complicated to mention a proper location of this file in .bbp_project.yaml, but I am really open to ideas. Today, the bbp-clang-tidy script expects the compile command database to be passed through option -p, see here

Besides, I would not worry too much about that since ClangTidy integration in CMake is pretty good: