CI fixes (#23)

* Add makefile, update pre-commit hooks

* Update

* Refactor CI, disable json formatter

* Set default shell for CI

* feat: Makefile extended with testing and linting

* feat: CMake linter and formatter added
fix: CMakeLists.txt codestyle fixed

* fix: added dependencies in Makefile

fix: added check for failure in CI steps

* fix: minor codestyle

* fix: added to log tests expicitly

* fix: .PHONY added to Makefile

* feat: added extended colcon test output to console

* feat: makefile simplified, sanitizer flags fixed
feat: CI pipeline uses Makefile

* Update dockerfile, bump action versions, minor fixes

* Fix bash shell, replace first step with make cmd

* fix: TODOS deleted

* feat: added yamllint config
fix: parallel clang-tidy tool added

* feat: replaced

* fix: root access for apt-get granted

* fix: root access granted for pip

* Minor fixes

* fix: fixed YAML codestyle

* fix: deleted redundant tags in docker-compose.yaml

---------

Co-authored-by: Bakin Denis <[email protected]>

.github/workflows/test.yml

@@ -2,92 +2,69 @@ name: Test
 
 on:
   pull_request:
-    branches: [ "master" ]
-    types: [ opened, reopened, ready_for_review, synchronize ]
-
+    branches: [master]
   push:
-    branches: [ "master" ]
+    branches: [master]
 
-  workflow_dispatch:
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
 
 jobs:
+  pre-commit:
+    name: Run pre-commit hooks
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with: { python-version: "3.10" }
+      - uses: pre-commit/[email protected]
+
   extract-image:
-    name: Extraction of configs
+    name: Get image version
     runs-on: ubuntu-latest
     outputs:
       image-url: ${{ steps.extractor.outputs.image_url }}
     steps:
-      - name: Checkout code
-        uses: actions/checkout@v3
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with: { python-version: "3.10" }
+
+      - name: Install dependencies
+        run: |
+          sudo pip install yq
+          sudo apt-get install -y jq
 
       - name: Extract image URL
         id: extractor
         run: |
-          IMAGE_URL=$(grep -o 'image:\s*[^ ]*' docker-compose.yaml | cut -d ' ' -f2)
-          echo "image_url=$IMAGE_URL" >> $GITHUB_OUTPUT
-          echo $IMAGE_URL
+          IMAGE_URL=$(cat docker-compose.yaml | yq ".services.${{ github.event.repository.name }}.image")
+          echo "image_url=$IMAGE_URL" | tee -a $GITHUB_OUTPUT
 
   build-and-check:
-    name: Building, testing, linting
+    name: Build, test, lint
     needs: extract-image
     runs-on: self-hosted
-    concurrency:
-      group: build-and-check-${{ github.ref }}
-      cancel-in-progress: true
-    env:
-      CMAKE_ARGS: "-DCMAKE_EXPORT_COMPILE_COMMANDS=1 -DCMAKE_TOOLS_ADDRESS_SANITIZER=1"
-
     container:
-      image: ${{ needs.extract-image.outputs.image-url}}
+      image: ${{ needs.extract-image.outputs.image-url }}
+    defaults:
+      run: { shell: bash }
 
     steps:
-      - name: Checkout code
-        uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Build ROS2 packages
-        shell: bash
         run: |
-          source $ROS_ROOT/setup.bash
-          cd $GITHUB_WORKSPACE/packages
-          colcon build --merge-install --cmake-args $CMAKE_ARGS
+          make build-all CMAKE_TOOLS_ADDRESS_SANITIZER="ON"
           source install/setup.bash
 
       - name: Test ROS2 packages
         continue-on-error: true
-        shell: bash
-        run: |
-          cd $GITHUB_WORKSPACE/packages
-          colcon test --ctest-args tests --merge-install --executor parallel --parallel-workers $(nproc) --return-code-on-test-failure
+        run: make test-all
 
       - name: Run Clang-Tidy
-        shell: bash
-        run: |
-          FILES=$(find . \( -name '*.h' -or -name '*.cpp' -or -name '*.cc' \) -not -path '*/build/*' -not -path '*/install/*' -not -path '*/log/*')
-          echo "Files to be linted:"
-          echo "$FILES"
-          for FILE in $FILES; do
-            echo $FILE
-            clang-tidy --fix -p=packages/build $FILE
-          done
+        run: make lint-all
 
-  code-style:
-    name: Formatting
-    needs: extract-image
-    runs-on: self-hosted
-    concurrency:
-      group: code-style-${{ github.ref }}
-      cancel-in-progress: true
-
-    container:
-      image: ${{ needs.extract-image.outputs.image-url}}
-
-    steps:
-      - name: Checkout repository
-        uses: actions/checkout@v3
-
-      # temporary solution until pre-commit is not installed in the container
-      - name: Launching pre-commit
-        shell: bash
-        run: |
-          pip3 install pre-commit
-          pre-commit run --color=always --all-files
+      - name: Check previous steps
+        if: ${{ failure() }}
+        run: exit 1

.pre-commit-config.yaml

@@ -4,28 +4,42 @@ repos:
     rev: v4.5.0
     hooks:
       - id: check-added-large-files
-        args: [--maxkb=512, --enforce-all]
       - id: check-merge-conflict
       - id: check-xml
+      - id: check-json
       - id: check-yaml
+      - id: check-toml
       - id: end-of-file-fixer
       - id: mixed-line-ending
       - id: trailing-whitespace
 
+  # YAML linter
+  - repo: https://github.com/adrienverge/yamllint.git
+    rev: v1.35.1
+    hooks:
+      - id: yamllint
+        exclude: ^.clang-tidy$
+        # TODO: Discuss yamllint settings (e.g. line-length)
+        args: [--strict, -d, ".yamllint-config.yaml"]
+
+  # CMake linter and formatter
+  - repo: https://github.com/cheshirekow/cmake-format-precommit
+    rev: v0.6.10
+    hooks:
+      - id: cmake-format
+      - id: cmake-lint
 
   # Python linters & formatters
   - repo: https://github.com/astral-sh/ruff-pre-commit
     rev: v0.1.3
     hooks:
       - id: ruff
-        args: [--show-fixes]
+        args: [--fix, --show-fixes]
       - id: ruff-format
-        args: ["--target-version=py38", "--respect-gitignore",
-               "--exclude=build", "--exclude=log", "--exclude=install"]
 
-  # C++ linters & formatters
-  - repo: https://github.com/pocc/pre-commit-hooks
-    rev: 336fdd7
+  # C++ formatter
+  - repo: https://github.com/pre-commit/mirrors-clang-format
+    rev: v17.0.6
     hooks:
       - id: clang-format
-        args: [-i, -style=file]
+        types: [c, c++]

.yamllint-config.yaml

@@ -0,0 +1,17 @@
+rules:
+  line-length:
+    max: 120
+
+  quoted-strings:
+    quote-type: double
+    required: false
+
+  braces:
+    min-spaces-inside: 1
+    max-spaces-inside: 1
+    min-spaces-inside-empty: 0
+    max-spaces-inside-empty: 0
+
+  brackets:
+    min-spaces-inside-empty: 0
+    max-spaces-inside-empty: 0

Dockerfile

@@ -293,7 +293,7 @@ RUN mkdir -p ${ROS_ROOT} \
         vision_opencv \
         visualization_msgs \
     > ${ROS_ROOT}/ros2.rosinstall \
-    && vcs import ${ROS_TMP} < ${ROS_ROOT}/ros2.rosinstall > /dev/null
+    && vcs import ${ROS_TMP} < ${ROS_ROOT}/ros2.rosinstall
 
 RUN apt-get update -q \
     && rosdep init \
@@ -353,3 +353,7 @@ RUN printf "export CC='${CC}'\n" >> ${HOME}/.bashrc \
     && printf "export RCUTILS_LOGGING_BUFFERED_STREAM=1\n" >> ${HOME}/.bashrc \
     && printf "export RCUTILS_CONSOLE_OUTPUT_FORMAT='[{severity}:{time}] {message}'\n" >> ${HOME}/.bashrc \
     && ln -sf /usr/bin/clang-format-${CLANG_VERSION} /usr/bin/clang-format
+
+WORKDIR /handy
+ENTRYPOINT ["/bin/bash", "-l", "-c"]
+CMD ["trap : TERM INT; sleep infinity & wait"]

Makefile

@@ -0,0 +1,69 @@
+.SILENT:
+.ONESHELL:
+# https://stackoverflow.com/questions/2131213#comment133331794_60363121
+.RECIPEPREFIX := $(.RECIPEPREFIX) $(.RECIPEPREFIX)
+
+SHELL = /bin/bash
+CMAKE_BUILD_TYPE ?= Release
+CMAKE_TOOLS_ADDRESS_SANITIZER ?= OFF
+CXXFLAGS := \
+    ${CXXFLAGS} \
+    $(shell if [ "$${CMAKE_TOOLS_ADDRESS_SANITIZER^^}" = "ON" ]; then echo "-fsanitize=address"; fi)
+CMAKE_ARGS ?= \
+    -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
+    -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} \
+    -DCMAKE_CXX_FLAGS="${CXXFLAGS}"
+
+FILES_TO_LINT := $(shell find . \( -name "*.h" -or -name "*.cpp" -or -name "*.cc" \) \
+                    -not -path "*/build/*" -not -path "*/install/*" -not -path "*/log/*")
+
+.PHONY: all
+all:
+    $(error Please use explicit targets)
+
+.PHONY: build-all
+build-all:
+    source ${ROS_ROOT}/setup.sh
+    colcon --log-base /dev/null build \
+        --base-paths packages \
+        --symlink-install \
+        --cmake-args ${CMAKE_ARGS}
+
+.PHONY: test-all
+test-all:
+    colcon --log-base /dev/null test \
+        --ctest-args tests --symlink-install \
+        --executor parallel --parallel-workers $$(nproc) \
+        --event-handlers console_cohesion+
+
+.PHONY: build
+# packages="first_pkg second_pkg third_pkg..."
+build:
+    source ${ROS_ROOT}/setup.sh
+    colcon --log-base /dev/null build \
+        --base-paths packages \
+        --symlink-install \
+        --cmake-args ${CMAKE_ARGS} \
+        --packages-up-to $(packages)
+
+.PHONY: test
+# packages="first_pkg second_pkg third_pkg..."
+test:
+    colcon --log-base /dev/null test --ctest-args tests --symlink-install \
+        --executor parallel --parallel-workers $$(nproc) \
+        --event-handlers console_cohesion+ --packages-select $(packages)
+
+.PHONY: lint-all
+# args="-fix ..."
+lint-all:
+    run-clang-tidy -p=build $(args) $(FILES_TO_LINT)
+
+.PHONY: lint
+# args="-fix ..."
+# files="first_file second_file third_file..."
+lint:
+    run-clang-tidy -p=build $(args) $(files)
+
+.PHONY: clean
+clean:
+    rm -rf build install

docker-compose.yaml

@@ -1,30 +1,30 @@
-version: "3.9"
 services:
   handy:
+    image: "registry.robotics-lab.ru/handy:0.7.2"
     container_name: "${CONTAINER_NAME:-handy-${USER}}"
-    image: registry.robotics-lab.ru/handy:0.7.0
+    networks: [default]
     privileged: true
-    stdin_open: true
-    runtime: ${DOCKER_RUNTIME:-runc}
-    entrypoint: "/bin/bash -c 'trap : TERM INT; sleep infinity & wait'"
-    working_dir: "/handy"
-    build:
-      dockerfile: Dockerfile
-      context: .
-      tags:
-          - "registry.robotics-lab.ru/handy:latest"
-          - "registry.robotics-lab.ru/handy:0.7.0"
-      x-bake:
-        platforms: [linux/arm64, linux/amd64]
-        cache-to: "type=registry,ref=registry.robotics-lab.ru/handy:cache,mode=max"
-        cache-from: "type=registry,ref=registry.robotics-lab.ru/handy:cache"
-    networks:
-      - default
     ports:
       - "${FOXGLOVE_PORT:-8765}:8765"
     volumes:
       - "${PWD}:/handy"
       - "/dev:/dev"
+
+    build:
+      dockerfile: Dockerfile
+      context: .
+      x-bake:
+        platforms:
+          - linux/arm64
+          - linux/amd64
+        cache-from:
+          - type=registry,ref=registry.robotics-lab.ru/handy:cache-arm64
+          - type=registry,ref=registry.robotics-lab.ru/handy:cache-amd64
+        cache-to:
+          # [!!!] Warning: https://github.com/docker/buildx/discussions/1382
+          # - type=registry,mode=max,ref=registry.robotics-lab.ru/handy:cache-arm64
+          # - type=registry,mode=max,ref=registry.robotics-lab.ru/handy:cache-amd64
+
 networks:
   default:
     name: "${CONTAINER_NAME:-handy-${USER}}"