Skip to content

Commit

Permalink
Add colcon_ws_config to make it possible to run all static code analy…
Browse files Browse the repository at this point in the history
…sers (space-ros#57).
  • Loading branch information
xfiderek committed Mar 13, 2024
1 parent 7212267 commit 1bbcd25
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 0 deletions.
75 changes: 75 additions & 0 deletions colcon_ws_config/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Spaceros workspace config
This directory contains collection of scripts and configuration file used in spaceros colcon workspace.
The content of this directory allows to inject cmake code into downstream packages without any source code modification.
SpaceROS uses files from this directory to include set of common linters and static code analysers into each package and to provide common colcon configurations.

## Using the config
For now the only use case for the config is to patch dependecies of upstream spaceros packages, so that every package uses common set of linting tools and static code analysers when `colcon test` is invoked.

The config is applied during spaceros CI pipeline to include common set of linters.

You can also use it inside the spaceros docker image, to include common set of linters for all packages (spaceros + thirdparty) in your workspace. This can be achieved by running a `prepare_workspace.py` python script in root of the workspace:

```bash
python3 colcon_ws_config/prepare_workspace.py
```
The script above should produce a `spaceros-linters.meta` file, which can be added to the build command:

```bash
colcon build --metas spaceros-linters.meta (...rest of the command)
```

Now, `colcon test` should trigger common set of linters for each package.

Note that whenever any new package is added, you should re-run `prepare_workspace.py` script, to update the meta file.

## Package description

**Prerequisities**
- [Understand .meta files in colcon](https://colcon.readthedocs.io/en/released/user/configuration.html#meta-files)


The `prepare_workspace.py` creates a `spaceros-linters.meta` file that injects `spaceros_inject.cmake` script into packages in colcon workspace. The cmake script is injected using [CMAKE_PROJECT_INCLUDE](https://cmake.org/cmake/help/latest/variable/CMAKE_PROJECT_INCLUDE.html) CMAKE variable.
Additionally, `prepare_workspace.py` adds dependencies to `spaceros-linters.meta` file, so that packages used in injected cmake script are present to the package during build.

The `spaceros-linters.meta` file consist of the following entries for each spaceros package:
```json
"PACKAGE NAME": {
"dependencies": [
"ament_lint_auto",
"ament_cmake"
"ament_cmake_cobra",
...
], // from spaceros_linters_deps.yaml
"ament-cmake-args": [
"-DCMAKE_PROJECT_INCLUDE=ABSOLUTE_PATH_TO_WS/colcon_ws_config/spaceros_inject.cmake"
]
}
```

Packages declared in `spaceros_linters_deps.yaml` and their upstream dependencies are excluded from `spaceros-linters.meta` file, to avoid circular dependencies.


## Advanced guides

### Adding custom linters / arbitrary cmake code
You can inject arbitrary code into the packages, by modifying `spaceros_inject.cmake` file. Whenever you add any new package that the script depends on (i.e. you call `find_package()`), make sure you add the dependency into `spaceros_linters_deps.yaml` file.


### Understanding content of CMake script
By default [spaceros_inject.cmake](./spaceros_inject.cmake) script, contains set of `find_package` calls. We first call `find_package(ament_lint_auto REQUIRED)`. This creates [ament extension](https://docs.ros.org/en/foxy/How-To-Guides/Ament-CMake-Documentation.html#adding-to-extension-points) called `ament_lint_auto`, that will get executed as part of `ament_package()` macro in ros package.

After that, `find_package()` for every ament linter is called (e.g. `find_package(ament_cmake_copyright)`).`. This call adds a script to `ament_lint_auto` extension point, that will be executed as part of this extension.

In order to make sure that injected code does not interfere with tests already added to downstream packages, `AMENT_LINT_AUTO_SKIP_PREEXISTING_TESTS` flag is set in the file.

## Limitations
### Package coverage
Only packages using `ament_cmake` build system are modified to include all spaceros analysers. To list packages that are not impacted, run:
```bash
colcon list | grep -v "ros.ament_cmake"
```
As of the time of writing this README (March 2024), there are 143 `ament_cmake` packages, 10 `cmake` packages and 58 `ament_python` packages.

### Tests overwriting
If an upstream package directly invokes a test using a macro in its `CMakeLists.txt` file (e.g., `ament_xmlint()`), that takes precedence over `ament_lint_auto`. Consequently, the settings for that test defined in the `CMakeLists.txt` file of upstream package will be effective.
43 changes: 43 additions & 0 deletions colcon_ws_config/prepare_workspace.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import subprocess
import json
import os
import yaml
"""
Create `colcon.meta` file.
The file allows to inject dependencies and cmake code to downstream packages.
"""

# TODO Parametrize
CONFIG_PATH = "./colcon_ws_config"
CMAKE_INJECT_FILE_PATH = os.path.abspath(f"{CONFIG_PATH}/spaceros_inject.cmake")
INJECT_DEPENDENCIES_FILE_PATH = f"{CONFIG_PATH}/spaceros_inject.cmake"
OUTPUT_FILE_PATH = "spaceros-linters.meta"


def main():
with open(f"{CONFIG_PATH}/spaceros_linters_deps.yaml", "r") as file:
deps_file = yaml.safe_load(file)
deps = deps_file["dependencies"]
deps_str = " ".join(deps)

pkgs = subprocess.run(
[f"colcon list --packages-skip-up-to {deps_str} --names-only"],
shell=True,
capture_output=True,
text=True,
)

output_meta = {"names": {}}

for pkg in pkgs.stdout.split(sep="\n")[:-1]:
output_meta["names"][pkg] = {
"dependencies": deps,
"ament-cmake-args": [f"-DCMAKE_PROJECT_INCLUDE={CMAKE_INJECT_FILE_PATH}"],
}

with open(OUTPUT_FILE_PATH, "w") as f:
json.dump(output_meta, f, indent=4)


if __name__ == "__main__":
main()
31 changes: 31 additions & 0 deletions colcon_ws_config/spaceros_inject.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# TODO license

# The main use case for this file is to
# include dependencies and linting tools required by spaceros
# into every ROS package without modifying 'CMakeLists.txt' files

# The code can be injected into packages with `CMAKE_PROJECT_INCLUDE` variable
# Check: https://cmake.org/cmake/help/latest/variable/CMAKE_PROJECT_INCLUDE.html

cmake_minimum_required(VERSION 3.15)
find_package(ament_cmake REQUIRED)

# required to avoid test duplications in case tests are already defined in packages
set(AMENT_LINT_AUTO_SKIP_PREEXISTING_TESTS ON)

if (BUILD_TESTING)
find_package(ament_lint_auto REQUIRED)
# include common set of spaceros linters
find_package(ament_cmake_clang_tidy REQUIRED)
find_package(ament_cmake_cobra REQUIRED)
find_package(ament_cmake_copyright REQUIRED)
find_package(ament_cmake_cppcheck REQUIRED)
find_package(ament_cmake_cpplint REQUIRED)
find_package(ament_cmake_ikos REQUIRED)
find_package(ament_cmake_flake8 REQUIRED)
find_package(ament_cmake_lint_cmake REQUIRED)
find_package(ament_cmake_pep257 REQUIRED)
find_package(ament_cmake_uncrustify REQUIRED)
find_package(ament_cmake_xmllint REQUIRED)
endif()

14 changes: 14 additions & 0 deletions colcon_ws_config/spaceros_linters_deps.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
dependencies:
- ament_cmake
- ament_lint_auto
- ament_cmake_clang_tidy
- ament_cmake_cobra
- ament_cmake_copyright
- ament_cmake_cppcheck
- ament_cmake_cpplint
- ament_cmake_ikos
- ament_cmake_flake8
- ament_cmake_lint_cmake
- ament_cmake_pep257
- ament_cmake_uncrustify
- ament_cmake_xmllint

0 comments on commit 1bbcd25

Please sign in to comment.