Make clang-tidy script more robust
authorPaul Bauer <paul.bauer.q@gmail.com>
Thu, 23 Apr 2020 13:14:50 +0000 (13:14 +0000)
committerPaul Bauer <paul.bauer.q@gmail.com>
Thu, 23 Apr 2020 13:14:50 +0000 (13:14 +0000)
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
admin/git-pre-commit
admin/gitlab-ci/lint.gitlab-ci.yml
src/api/cpp/workflow.h
src/gromacs/mdlib/forcerec.h
src/testutils/refdata.cpp

index 2c5a6a1046601a069985c171afebc4bdc4d71355..f5a3f22c260e046ecd79b2bd206f43cb7f883dab 100755 (executable)
@@ -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
index be833e3312429ae111b8596aa19f904a9b2ad7c1..6638b414fcbabc9869fbeb34bd0edb5fcd8a8071 100755 (executable)
@@ -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
index 21a7003e0a6e3d5ec30dd827c12d0980914d3058..d9ca4945cbfb2616f489b07ed2d99818a11b604c 100644 (file)
@@ -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
index ccba142620957e31c142d9cad7f8ba2fbface9dd..0571395ec263cd050cf113cb4ec0d52421bb7546 100644 (file)
@@ -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<NodeSpecification> spec);
+    static NodeKey addNode(std::unique_ptr<NodeSpecification> spec);
 
     /*!
      * \brief Get the node specification for a provided key.
index a8343d62e67e0a3b29f9b37fda3b2b7e9c3950d7..a3b5f1f9159455544604db1beb0feb239511cab0 100644 (file)
@@ -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.
  *
index 1efcc375a9030c2cc3918a3124d34eacd8423728..7686090c7aab8f2af0ae0c1378b66f0aef6de2ac 100644 (file)
@@ -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)
         {