From e3d14d4af48006cc586d4b84df90faeee9bd3279 Mon Sep 17 00:00:00 2001 From: Paul Bauer Date: Thu, 23 Apr 2020 13:14:50 +0000 Subject: [PATCH] Make clang-tidy script more robust Changed script to be more robust and in line with other source checking scripts. Modified clang-tidy:test job to check against the merge fork point for a commit instead of the previous git magic, as this did not work. Change-Id: Ic806439727c5317db0c18964afd6138eeeea0427 --- admin/clang-tidy.sh | 22 +++++++++++++--------- admin/git-pre-commit | 2 +- admin/gitlab-ci/lint.gitlab-ci.yml | 5 ++--- src/api/cpp/workflow.h | 4 ++-- src/gromacs/mdlib/forcerec.h | 2 +- src/testutils/refdata.cpp | 14 +++++++------- 6 files changed, 26 insertions(+), 23 deletions(-) diff --git a/admin/clang-tidy.sh b/admin/clang-tidy.sh index 2c5a6a1046..f5a3f22c26 100755 --- a/admin/clang-tidy.sh +++ b/admin/clang-tidy.sh @@ -145,9 +145,9 @@ git diff-files --name-only | grep -Ff $tmpdir/filelist_all >$tmpdir/localmods # Extract changed files to a temporary directory mkdir $tmpdir/org if [[ $action == *-index ]] ; then - git checkout-index --prefix=$tmpdir/org/ + git checkout-index --prefix=$tmpdir/org/ --stdin <$tmpdir/filelist_all else - rsync -a $srcdir/src/ $tmpdir/org/src/ + rsync --files-from=$tmpdir/filelist_all -a $srcdir/ $tmpdir/org/ fi # check for the existence of the compile_commands.json file and abort # if it is not present. If we don't have a build directory, try the @@ -162,8 +162,7 @@ else # Need to have compilation database file available somewhere above where we are using it rsync -a $builddir/compile_commands.json $tmpdir/org fi -# Duplicate the original files to a separate directory, where all changes will -# be made. +# Prepare directory to use for comparing changed and original files cp -r $tmpdir/org $tmpdir/new # Create output file for what was done (in case no messages get written) @@ -173,11 +172,11 @@ touch $tmpdir/messages # Can only perform clang-tidy on a non-empty list of files cd $tmpdir/new if [[ $tidy_mode != "off" && -s $tmpdir/filelist_clangtidy ]] ; then - $RUN_CLANG_TIDY `cat $tmpdir/filelist_clangtidy` -- -header-filter=.* -j $concurrency -fix -fix-errors --cuda-host-only -nocudainc -quiet >$tmpdir/clang-tidy.out 2>&1 - awk '/warning/,/clang-tidy|^$/' $tmpdir/clang-tidy.out | grep -v "warnings generated." | grep -v "Suppressed .* warnings" | grep -v "clang-analyzer" | grep -v "to display errors from all non" | sed '/^\s*$/d' > $tmpdir/clang-tidy-errors.out - cp $tmpdir/clang-tidy.out $tmpdir/clang-tidy-errors.out $srcdir + $RUN_CLANG_TIDY `cat $tmpdir/filelist_clangtidy` -header-filter=.* -j $concurrency -fix -quiet -extra-arg=--cuda-host-only -extra-arg=-nocudainc>$tmpdir/clang-tidy.out 2>&1 + awk '/warning/,/clang-tidy|^$/' $tmpdir/clang-tidy.out | grep -v "warnings generated." | grep -v "Suppressed .* warnings" | grep -v "clang-analyzer" | grep -v "to display errors from all non" | sed '/^\s*$/d' > $tmpdir/clang-tidy-warnings.out + awk '/.*error.*/' $tmpdir/clang-tidy.out > $tmpdir/clang-tidy-errors.out || true if [ -s $tmpdir/clang-tidy-errors.out ]; then - echo "Running code tidying failed. Check output below for errors:" + echo "Running of clang-tidy failed. Check output below for errors:" cat $tmpdir/clang-tidy-errors.out rm -rf $tmpdir exit 2 @@ -188,6 +187,8 @@ if [[ $tidy_mode != "off" && -s $tmpdir/filelist_clangtidy ]] ; then if [[ $action == update-* ]] ; then msg="clang-tidy performed" fi + rsync --files-from=$tmpdir/filelist_all -a $srcdir/ ./ + rsync -a $tmpdir/org/ $srcdir/ git diff --no-index --name-only ../org/ . | \ awk -v msg="$msg" '{sub(/.\//,""); print $0 ": " msg}' >> $tmpdir/messages fi @@ -255,7 +256,10 @@ fi popd >/dev/null # Report what was done -sort $tmpdir/messages | tee $warning_file +if [ -s $tmpdir/clang-tidy-warnings.out ] ; then + cat $tmpdir/clang-tidy-warnings.out | tee $warning_file +fi +sort $tmpdir/messages | tee -a $warning_file rm -rf $tmpdir exit $changes diff --git a/admin/git-pre-commit b/admin/git-pre-commit index be833e3312..6638b414fc 100755 --- a/admin/git-pre-commit +++ b/admin/git-pre-commit @@ -68,7 +68,7 @@ exec 1>&2 clangtidy_mode=`git config hooks.clangtidymode` clangformat_mode=`git config hooks.clangformatmode` copyright_mode=`git config hooks.copyrightmode` -if [ -z "$uncrustify_mode" ] +if [ -z "$clangtidy_mode" ] then clangtidy_mode=off fi diff --git a/admin/gitlab-ci/lint.gitlab-ci.yml b/admin/gitlab-ci/lint.gitlab-ci.yml index 21a7003e0a..d9ca4945cb 100644 --- a/admin/gitlab-ci/lint.gitlab-ci.yml +++ b/admin/gitlab-ci/lint.gitlab-ci.yml @@ -10,7 +10,7 @@ clang-tidy:configure-push: variables: COMPILER_MAJOR_VERSION: 9 BUILD_DIR: build-clang-tidy - CMAKE_EXTRA_OPTIONS: -DCLANG_TIDY=clang-tidy-$COMPILER_MAJOR_VERSION -DGMX_CLANG_TIDY=ON -DGMX_COMPILER_WARNINGS=ON + CMAKE_EXTRA_OPTIONS: -DCLANG_TIDY=clang-tidy-$COMPILER_MAJOR_VERSION -DGMX_CLANG_TIDY=ON -DGMX_COMPILER_WARNINGS=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON clang-tidy:configure-schedule: extends: @@ -40,7 +40,6 @@ clang-tidy:build: clang-tidy:test: extends: - .gromacs:base:build - - .use-ccache - .variables:default - .rules:basic-push stage: source-check @@ -58,7 +57,7 @@ clang-tidy:test: script: # TODO (issue #3272) `master` is not appropriate for use on release-xxxx branches, how should we handle that? # See discussion at https://gitlab.com/gromacs/gromacs/-/merge_requests/67 - - REV=$(git fetch -q https://gitlab.com/gromacs/gromacs.git master && git show -s --pretty=format:"%h" `git merge-base FETCH_HEAD HEAD`) + - REV=$(git fetch -q https://gitlab.com/gromacs/gromacs.git master && git show -s --pretty=format:"%h" `git merge-base --fork-point FETCH_HEAD HEAD`) - RUN_CLANG_TIDY=run-clang-tidy-$COMPILER_MAJOR_VERSION bash admin/clang-tidy.sh check --parallel=$KUBERNETES_CPU_LIMIT --warnings=clang-tidy.log --rev=$REV -B=$BUILD_DIR - grep -iq "found code issues" clang-tidy.log | tee code-lint.txt || true - if [ -s code-lint.txt ] ; then echo "clang-tidy.sh found issues"; exit 1; fi diff --git a/src/api/cpp/workflow.h b/src/api/cpp/workflow.h index ccba142620..0571395ec2 100644 --- a/src/api/cpp/workflow.h +++ b/src/api/cpp/workflow.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2018,2019, by the GROMACS development team, led by + * Copyright (c) 2018,2019,2020, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -149,7 +149,7 @@ public: * * \todo Not yet implemented. */ - NodeKey addNode(std::unique_ptr spec); + static NodeKey addNode(std::unique_ptr spec); /*! * \brief Get the node specification for a provided key. diff --git a/src/gromacs/mdlib/forcerec.h b/src/gromacs/mdlib/forcerec.h index a8343d62e6..a3b5f1f915 100644 --- a/src/gromacs/mdlib/forcerec.h +++ b/src/gromacs/mdlib/forcerec.h @@ -86,7 +86,7 @@ void forcerec_set_ranges(t_forcerec* fr, int natoms_force, int natoms_force_cons * \param[in] ic Structure holding the table constant * \param[in] tableExtensionLength Length by which to extend the tables. Taken from the input record. */ -void init_interaction_const_tables(FILE* fp, interaction_const_t* ic, const real tableExtensionLength); +void init_interaction_const_tables(FILE* fp, interaction_const_t* ic, real tableExtensionLength); /*! \brief Initialize forcerec structure. * diff --git a/src/testutils/refdata.cpp b/src/testutils/refdata.cpp index 1efcc375a9..7686090c7a 100644 --- a/src/testutils/refdata.cpp +++ b/src/testutils/refdata.cpp @@ -420,19 +420,19 @@ public: std::string appendPath(const char* id) const; //! Creates an entry with given parameters and fills it with \p checker. - ReferenceDataEntry::EntryPointer createEntry(const char* type, - const char* id, - const IReferenceDataEntryChecker& checker) const + static ReferenceDataEntry::EntryPointer createEntry(const char* type, + const char* id, + const IReferenceDataEntryChecker& checker) { ReferenceDataEntry::EntryPointer entry(new ReferenceDataEntry(type, id)); checker.fillEntry(entry.get()); return entry; } //! Checks an entry for correct type and using \p checker. - ::testing::AssertionResult checkEntry(const ReferenceDataEntry& entry, - const std::string& fullId, - const char* type, - const IReferenceDataEntryChecker& checker) const + static ::testing::AssertionResult checkEntry(const ReferenceDataEntry& entry, + const std::string& fullId, + const char* type, + const IReferenceDataEntryChecker& checker) { if (entry.type() != type) { -- 2.22.0