From 4fcabdfdf169fb9745fac07ca2c675c0c35cb286 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Thu, 20 Oct 2022 02:11:47 +0200 Subject: [PATCH 1/7] Simplify running clang-tidy - Use run-clang-tidy to run parallel jobs - Drop -export-fixes argument to allow users to specify a location --- industrial_ci/src/tests/source_tests.sh | 28 +++++-------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/industrial_ci/src/tests/source_tests.sh b/industrial_ci/src/tests/source_tests.sh index a00b278dc..e1b7d1976 100644 --- a/industrial_ci/src/tests/source_tests.sh +++ b/industrial_ci/src/tests/source_tests.sh @@ -63,32 +63,14 @@ function run_clang_tidy { ici_error "CLANG_TIDY_JOBS=$CLANG_TIDY_JOBS is invalid." fi - rm -rf "$db".{command,warn,error} - cat > "$db.command" << EOF -#!/bin/bash -num_non_file_args=\$1; shift -args=("\${@:1:\$num_non_file_args}") -files=("\${@:\$((num_non_file_args+1))}") -fixes=\$(mktemp) -rm -f /tmp/clang_tidy_output.\$\$ -for f in "\${files[@]}" ; do - ( cd \$(dirname \$f); clang-tidy "-export-fixes=\$fixes" "-header-filter=$regex" "-p=$build" "\${args[@]}" \$f &>> /tmp/clang_tidy_output.\$\$ 2>&1 || { touch "$db.error"; } ) - if [ -s "\$fixes" ]; then touch "$db.warn"; fi -done -rm -rf "\$fixes" -EOF - chmod +x "$db.command" - + local err=0 ici_log "run clang-tidy for ${#files[@]}/$num_all_files file(s) in $max_jobs process(es)." + printf "%s\0" "${files[@]}" | xargs --null run-clang-tidy "-j$max_jobs" "-header-filter=$regex" "-p=$build" "$@" | tee "$db.tidy.log" || err=$? - printf "%s\0" "${files[@]}" | xargs --null -P "$max_jobs" -n "$(( (${#files[@]} + max_jobs-1) / max_jobs))" "$db.command" "$#" "$@" - cat /tmp/clang_tidy_output.* | grep -vP "^([0-9]+ warnings generated|Use .* to display errors from system headers as well)\.$" || true - rm -rf /tmp/clang_tidy_output.* - - if [ -f "$db.error" ]; then + if [ "$err" -ne "0" ]; then _run_clang_tidy_errors+=("$name") - ici_time_end "${ANSI_RED}" - elif [ -f "$db.warn" ]; then + ici_time_end "${ANSI_RED}" "$err" + elif grep -q ": warning: " "$db.tidy.log"; then _run_clang_tidy_warnings+=("$name") ici_time_end "${ANSI_YELLOW}" else From f69e5fa14c616776432b955986a83d6d05131678 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Wed, 2 Nov 2022 20:06:40 +0100 Subject: [PATCH 2/7] Merge fixes files of clang-tidy --- industrial_ci/src/tests/merge_fixes.py | 48 +++++++++++++++++++++++++ industrial_ci/src/tests/source_tests.sh | 16 +++++++++ 2 files changed, 64 insertions(+) create mode 100755 industrial_ci/src/tests/merge_fixes.py diff --git a/industrial_ci/src/tests/merge_fixes.py b/industrial_ci/src/tests/merge_fixes.py new file mode 100755 index 000000000..73ad4ad94 --- /dev/null +++ b/industrial_ci/src/tests/merge_fixes.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +# Copyright (c) 2022, Robert Haschke +# All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import yaml +import sys + + +def merge_fixes(files): + """Merge all fixes files into mergefile""" + # The fixes suggested by clang-tidy >= 4.0.0 are given under + # the top level key 'Diagnostics' in the output yaml files + mergefile = files[0] + mergekey = "Diagnostics" + merged = [] + for file in files: + try: + with open(file, 'r') as inp: + content = yaml.safe_load(inp) + if not content: + continue # Skip empty files. + merged.extend(content.get(mergekey, [])) + except FileNotFoundError: + pass + + with open(mergefile, 'w') as out: + if merged: + # Assemble output dict with MainSourceFile=''. + output = {'MainSourceFile': '', mergekey: merged} + yaml.safe_dump(output, out) + + +if __name__ == "__main__": + merge_fixes(sys.argv[1:]) diff --git a/industrial_ci/src/tests/source_tests.sh b/industrial_ci/src/tests/source_tests.sh index e1b7d1976..465779865 100644 --- a/industrial_ci/src/tests/source_tests.sh +++ b/industrial_ci/src/tests/source_tests.sh @@ -92,8 +92,24 @@ function run_clang_tidy_check { ici_hook "before_clang_tidy_checks" + # replace -export-fixes with temporary file + local fixes_final="" + local fixes_tmp + local num_args=${#clang_tidy_args[@]} + fixes_tmp=$(mktemp) + for (( i=0; i Date: Thu, 3 Nov 2022 13:47:22 +0100 Subject: [PATCH 3/7] Translate file names from target workspace to target repo --- industrial_ci/src/tests/source_tests.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/industrial_ci/src/tests/source_tests.sh b/industrial_ci/src/tests/source_tests.sh index 465779865..af3b72c61 100644 --- a/industrial_ci/src/tests/source_tests.sh +++ b/industrial_ci/src/tests/source_tests.sh @@ -112,6 +112,10 @@ function run_clang_tidy_check { fi done < <(find "$target_ws/build" -mindepth 2 -name compile_commands.json) # -mindepth 2, because colcon puts a compile_commands.json into the build folder + if [ -n "${fixes_final}" ]; then + # translate file names in fixes file + sed -i "s#$target_ws/src/$TARGET_REPO_NAME/#$TARGET_REPO_PATH/#g" "${fixes_final}" + fi ici_hook "after_clang_tidy_checks" if [ "${#warnings[@]}" -gt "0" ]; then From 0db98d3fd352021a28d6f9ae3c67d79a038a6099 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Thu, 3 Nov 2022 09:09:03 +0100 Subject: [PATCH 4/7] Remove duplicates while merging fixes --- industrial_ci/src/tests/merge_fixes.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/industrial_ci/src/tests/merge_fixes.py b/industrial_ci/src/tests/merge_fixes.py index 73ad4ad94..afba1cda6 100755 --- a/industrial_ci/src/tests/merge_fixes.py +++ b/industrial_ci/src/tests/merge_fixes.py @@ -20,6 +20,13 @@ import sys +def key(item): + name = item.get("DiagnosticName") + msg = item.get("DiagnosticMessage") + file = msg.get("FilePath") + offset = msg.get("FileOffset") + return name, file, offset + def merge_fixes(files): """Merge all fixes files into mergefile""" # The fixes suggested by clang-tidy >= 4.0.0 are given under @@ -27,13 +34,22 @@ def merge_fixes(files): mergefile = files[0] mergekey = "Diagnostics" merged = [] + seen = set() # efficiently remember fixes already inserted + + def have(x): + k = key(x) + return k in seen or seen.add(k) + + def unique(seq): + return [x for x in seq if not have(x)] + for file in files: try: with open(file, 'r') as inp: content = yaml.safe_load(inp) if not content: continue # Skip empty files. - merged.extend(content.get(mergekey, [])) + merged.extend(unique(content.get(mergekey, []))) except FileNotFoundError: pass From 243a518076e201a8da9ed211db060c06eb60200e Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Thu, 5 Jan 2023 12:22:03 +0100 Subject: [PATCH 5/7] Fix warnings detection --- industrial_ci/src/tests/source_tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/industrial_ci/src/tests/source_tests.sh b/industrial_ci/src/tests/source_tests.sh index af3b72c61..d6087fcf4 100644 --- a/industrial_ci/src/tests/source_tests.sh +++ b/industrial_ci/src/tests/source_tests.sh @@ -65,12 +65,12 @@ function run_clang_tidy { local err=0 ici_log "run clang-tidy for ${#files[@]}/$num_all_files file(s) in $max_jobs process(es)." - printf "%s\0" "${files[@]}" | xargs --null run-clang-tidy "-j$max_jobs" "-header-filter=$regex" "-p=$build" "$@" | tee "$db.tidy.log" || err=$? + printf "%s\0" "${files[@]}" | xargs --null run-clang-tidy "-j$max_jobs" "-header-filter=$regex" "-p=$build" "$@" 2>&1 | tee "$db.tidy.log" || err=$? if [ "$err" -ne "0" ]; then _run_clang_tidy_errors+=("$name") ici_time_end "${ANSI_RED}" "$err" - elif grep -q ": warning: " "$db.tidy.log"; then + elif grep -q "warning: " "$db.tidy.log"; then _run_clang_tidy_warnings+=("$name") ici_time_end "${ANSI_YELLOW}" else From 6377a74c61daad0ab8c581adee8f605c200cb4a3 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Thu, 5 Jan 2023 15:22:53 +0100 Subject: [PATCH 6/7] Enable pipefail --- industrial_ci/src/tests/source_tests.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/industrial_ci/src/tests/source_tests.sh b/industrial_ci/src/tests/source_tests.sh index d6087fcf4..303702a0b 100644 --- a/industrial_ci/src/tests/source_tests.sh +++ b/industrial_ci/src/tests/source_tests.sh @@ -65,7 +65,9 @@ function run_clang_tidy { local err=0 ici_log "run clang-tidy for ${#files[@]}/$num_all_files file(s) in $max_jobs process(es)." + set -o pipefail printf "%s\0" "${files[@]}" | xargs --null run-clang-tidy "-j$max_jobs" "-header-filter=$regex" "-p=$build" "$@" 2>&1 | tee "$db.tidy.log" || err=$? + set +o pipefail if [ "$err" -ne "0" ]; then _run_clang_tidy_errors+=("$name") From 0122d46b885f011687c1b4113f133bbffeb94f25 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Thu, 5 Jan 2023 15:23:52 +0100 Subject: [PATCH 7/7] Quote -header-filter regex --- industrial_ci/src/tests/source_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/industrial_ci/src/tests/source_tests.sh b/industrial_ci/src/tests/source_tests.sh index 303702a0b..85496098d 100644 --- a/industrial_ci/src/tests/source_tests.sh +++ b/industrial_ci/src/tests/source_tests.sh @@ -66,7 +66,7 @@ function run_clang_tidy { local err=0 ici_log "run clang-tidy for ${#files[@]}/$num_all_files file(s) in $max_jobs process(es)." set -o pipefail - printf "%s\0" "${files[@]}" | xargs --null run-clang-tidy "-j$max_jobs" "-header-filter=$regex" "-p=$build" "$@" 2>&1 | tee "$db.tidy.log" || err=$? + printf "%s\0" "${files[@]}" | xargs --null run-clang-tidy "-j$max_jobs" "-header-filter=\"$regex\"" "-p=$build" "$@" 2>&1 | tee "$db.tidy.log" || err=$? set +o pipefail if [ "$err" -ne "0" ]; then