diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9f8bc5adfc..e7b3ca2e7f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -44,9 +44,9 @@ repos: src/base/base_uses.f90| src/common/util.F| )$ - - id: analyze_src - name: Run analyze_src - entry: ./tools/conventions/analyze_src.py --fail --suppressions ./tools/conventions/conventions.supp + - id: check_file_properties + name: Run check_file_properties.py + entry: ./tools/precommit/check_file_properties.py language: script files: '^src/.*\.(F|fypp|c|cu|cpp|h|hpp)$' - id: regen_data diff --git a/tools/conventions/analyze_src.py b/tools/conventions/analyze_src.py deleted file mode 100755 index 75d57e6f68..0000000000 --- a/tools/conventions/analyze_src.py +++ /dev/null @@ -1,233 +0,0 @@ -#!/usr/bin/env python3 - -# author: Ole Schuett - -import argparse -import re -import sys -import os -from os import path -from datetime import datetime - -flag_exceptions_re = re.compile( - r"__COMPILE_REVISION|__COMPILE_DATE|__COMPILE_ARCH|__COMPILE_HOST|CUDA_VERSION|" - r"__INTEL_COMPILER|__cplusplus|_OPENMP|\$\{.*\}\$|__.*__" -) - -portable_filename_re = re.compile(r"^[a-zA-Z0-9._/#~=+-]*$") - -BANNER_F = """\ -!--------------------------------------------------------------------------------------------------! -! CP2K: A general program to perform molecular dynamics simulations ! -! Copyright 2000-{:d} CP2K developers group ! -! ! -! SPDX-License-Identifier: GPL-2.0-or-later ! -!--------------------------------------------------------------------------------------------------! -""" - -BANNER_Fypp = """\ -#!-------------------------------------------------------------------------------------------------! -#! CP2K: A general program to perform molecular dynamics simulations ! -#! Copyright 2000-{:d} CP2K developers group ! -#! ! -#! SPDX-License-Identifier: GPL-2.0-or-later ! -#!-------------------------------------------------------------------------------------------------! -""" - -BANNER_C = """\ -/*----------------------------------------------------------------------------*/ -/* CP2K: A general program to perform molecular dynamics simulations */ -/* Copyright 2000-{:d} CP2K developers group */ -/* */ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/*----------------------------------------------------------------------------*/ -""" - -DEFAULT_EXCLUDED_DIRS = ( - ".git", - "obj", - "lib", - "exe", - "regtesting", - "tools/toolchain/build", - "tools/toolchain/install", -) - - -def validate(cp2k_dir, filelist=None, excluded_dirs=DEFAULT_EXCLUDED_DIRS): - """ - Check the source files in the given directory/filelist for convention violations, like: - - - correct copyright headers - - undocumented preprocessor flags - - stray unicode characters - - :param cp2k_dir: base directory to look for files - :param filelist: specific list of files to check in the given directory - :param excluded_dirs: List of directories to exclude in the checks (usually relative to `cp2k_dir`) - """ - # check flags and banners - flags = set() - year = datetime.utcnow().year - warnings = [] - - # directories to exclude, if given as relative dirs, they're relative to cp2k_dir - excluded_dirs = [path.join(cp2k_dir, p) for p in DEFAULT_EXCLUDED_DIRS] - - # also exclude any Git submodules - if path.exists(path.join(cp2k_dir, ".gitmodules")): - with open(path.join(cp2k_dir, ".gitmodules"), encoding="utf8") as fhandle: - excluded_dirs += [ - path.join(cp2k_dir, l.split()[-1]) for l in fhandle if "path =" in l - ] - - if filelist: - # limited emulation of the os.walk generator: - # triples of (dir, list-of-subdirs, list-of-files) - fileiter = [ - (path.join(cp2k_dir, path.dirname(f)), [], [path.basename(f)]) - for f in filelist - ] - else: - fileiter = os.walk(path.join(cp2k_dir, "src")) - - for root, dirs, files in fileiter: - dirs[:] = [d for d in dirs if path.join(root, d) not in excluded_dirs] - - for fn in files: - fn_ext = fn.rsplit(".", 1)[-1] - - with open(path.join(root, fn), encoding="utf8") as fhandle: - content = fhandle.read() - - # check banner - if ( - (fn_ext in ("F",) and not content.startswith(BANNER_F.format(year))) - or ( - fn_ext in ("fypp",) - and not content.startswith(BANNER_Fypp.format(year)) - ) - or ( - fn_ext in ("c", "cu", "cpp", "h", "hpp") - and not content.startswith(BANNER_C.format(year)) - ) - ): - warnings += ["%s: Copyright banner malformed" % fn] - - # find all flags - line_continuation = False - for line in content.split("\n"): - if not line_continuation: - if len(line) == 0 or line[0] != "#": - continue - if line.split()[0] not in ("#if", "#ifdef", "#ifndef", "#elif"): - continue - line = line.split("//", 1)[0] - line_continuation = line.strip().endswith("\\") - line = re.sub(r"[\\|()!&><=*/+-]", " ", line) - line = line.replace("defined", " ") - for m in line.split()[1:]: - if re.match("[0-9]+[ulUL]*", m): - continue # skip numbers - if fn_ext in ("h", "hpp") and fn.upper().replace(".", "_") == m: - continue # ignore aptly named inclusion guards - flags.add(m) - - flags = [f for f in flags if not flag_exceptions_re.match(f)] - - with open(path.join(cp2k_dir, "INSTALL.md"), encoding="utf8") as fhandle: - install_txt = fhandle.read() - with open(path.join(cp2k_dir, "src/cp2k_info.F"), encoding="utf8") as fhandle: - cp2k_info = fhandle.read() - - flags_src = re.search( - r"FUNCTION cp2k_flags\(\)(.*)END FUNCTION cp2k_flags", cp2k_info, re.DOTALL - ).group(1) - - for f in sorted(flags): - if f not in install_txt: - warnings += ["Flag %s not mentioned in INSTALL.md" % f] - if f not in flags_src: - warnings += ["Flag %s not mentioned in cp2k_flags()" % f] - - if not filelist: - # check for copies of data files - data_files = set() - for _, _, files in os.walk(path.join(cp2k_dir, "data")): - data_files.update(files) - data_files.remove("README.md") - for root, _, files in os.walk(path.join(cp2k_dir, "tests")): - d = path.relpath(root, cp2k_dir) - for c in data_files.intersection(files): - warnings += ["Data file %s copied to %s" % (c, d)] - - if filelist: - fileiter = [(cp2k_dir, [], filelist)] - else: - fileiter = os.walk(cp2k_dir) - - # check linebreaks and encoding - for root, dirs, files in fileiter: - dirs[:] = [d for d in dirs if path.join(root, d) not in excluded_dirs] - - for fn in files: - absfn = path.join(root, fn) - shortfn = path.relpath(absfn, cp2k_dir) - - if not portable_filename_re.match(fn): - warnings += ["Filename %s not portable" % shortfn] - - if not path.exists(absfn): - continue # skip broken symlinks - - with open(absfn, "rb") as fhandle: - content = fhandle.read() - - if b"\0" in content: - continue # skip binary files - if b"\r\n" in content: - warnings += ["Text file %s contains DOS linebreaks" % shortfn] - if b"\t" in content: - warnings += ["Text file %s contains tab character" % shortfn] - - return warnings - - -if __name__ == "__main__": - parser = argparse.ArgumentParser( - description="Check source code for coding convention violations" - ) - parser.add_argument( - "-b", "--base-dir", type=str, default=".", help="CP2K base directory to check" - ) - parser.add_argument( - "file", - type=str, - nargs="*", - help="Limit the test to given files (given as relative paths to the base dir, otherwise all relevant files will be scanned)", - ) - parser.add_argument( - "--fail", - action="store_true", - help="return non-0 exit code if warnings have been found (useful for pre-commit scripts)", - ) - parser.add_argument( - "--suppressions", - type=argparse.FileType("r"), - help="Specify a suppression file with messages to not count towards the number of warnings", - ) - args = parser.parse_args() - - warnings = validate(args.base_dir, args.file) - - if args.suppressions: - suppress = [ - l.rstrip() for l in args.suppressions if l and not l.startswith("#") - ] - warnings = [w for w in warnings if not any(s in w for s in suppress)] - - for warning in warnings: - print(warning) - - if args.fail and warnings: - sys.exit(1) diff --git a/tools/conventions/conventions.supp b/tools/conventions/conventions.supp index 18272ae0f3..f3ae33f50f 100644 --- a/tools/conventions/conventions.supp +++ b/tools/conventions/conventions.supp @@ -1,40 +1,3 @@ -Data file BASIS_SET copied to tests/QMMM/QS -Data file GTH_BASIS_SETS copied to tests/QMMM/QS -Data file MM_POTENTIAL copied to tests/QMMM/QS -Data file POTENTIAL copied to tests/QMMM/QS -Flag FD_DEBUG not mentioned in INSTALL.md -Flag FD_DEBUG not mentioned in cp2k_flags() -Flag INTEL_MKL_VERSION not mentioned in INSTALL.md -Flag INTEL_MKL_VERSION not mentioned in cp2k_flags() -Flag LIBINT2_MAX_AM_eri not mentioned in INSTALL.md -Flag LIBINT2_MAX_AM_eri not mentioned in cp2k_flags() -Flag LIBINT_CONTRACTED_INTS not mentioned in INSTALL.md -Flag LIBINT_CONTRACTED_INTS not mentioned in cp2k_flags() -Flag XC_MAJOR_VERSION not mentioned in INSTALL.md -Flag XC_MAJOR_VERSION not mentioned in cp2k_flags() -Flag XC_MINOR_VERSION not mentioned in INSTALL.md -Flag XC_MINOR_VERSION not mentioned in cp2k_flags() -Flag __CRAY_PM_FAKE_ENERGY not mentioned in INSTALL.md -Flag __DATA_DIR not mentioned in cp2k_flags() -Flag __FFTW3_UNALIGNED not mentioned in cp2k_flags() -Flag __FORCE_USE_FAST_MATH not mentioned in INSTALL.md -Flag __FORCE_USE_FAST_MATH not mentioned in cp2k_flags() -Flag __HAS_smm_cnt not mentioned in INSTALL.md -Flag __HAS_smm_ctn not mentioned in INSTALL.md -Flag __HAS_smm_ctt not mentioned in INSTALL.md -Flag __HAS_smm_dnt not mentioned in INSTALL.md -Flag __HAS_smm_dtn not mentioned in INSTALL.md -Flag __HAS_smm_dtt not mentioned in INSTALL.md -Flag __HAS_smm_snt not mentioned in INSTALL.md -Flag __HAS_smm_stn not mentioned in INSTALL.md -Flag __HAS_smm_stt not mentioned in INSTALL.md -Flag __HAS_smm_znt not mentioned in INSTALL.md -Flag __HAS_smm_ztn not mentioned in INSTALL.md -Flag __HAS_smm_ztt not mentioned in INSTALL.md -Flag __PILAENV_BLOCKSIZE not mentioned in cp2k_flags() -Flag __PW_CUDA_NO_HOSTALLOC not mentioned in INSTALL.md -Flag __RELEASE_VERSION not mentioned in INSTALL.md -Flag __RELEASE_VERSION not mentioned in cp2k_flags() al_system_dynamics.F: Found WRITE statement with hardcoded unit in "dump_vel" almo_scf.F: Found WRITE statement with hardcoded unit in "almo_scf_init" base_hooks.F: Found CALL m_abort in procedure "cp_abort" @@ -246,8 +209,6 @@ xyz2dcd.F: Found STOP statement in procedure "xyz2dcd" xyz2dcd.F: Found WRITE statement with hardcoded unit in "abort_program" xyz2dcd.F: Found WRITE statement with hardcoded unit in "print_help" xyz2dcd.F: Found WRITE statement with hardcoded unit in "xyz2dcd" -Flag GRID_DO_COLLOCATE not mentioned in INSTALL.md -Flag GRID_DO_COLLOCATE not mentioned in cp2k_flags() memory_utilities_unittest.F: Found WRITE statement with hardcoded unit in "check_real_rank1_allocated" memory_utilities_unittest.F: Found WRITE statement with hardcoded unit in "check_real_rank1_unallocated" memory_utilities_unittest.F: Found WRITE statement with hardcoded unit in "check_real_rank2_allocated" diff --git a/tools/conventions/test_conventions.sh b/tools/conventions/test_conventions.sh index bf7a8d994f..42a2e8f1af 100755 --- a/tools/conventions/test_conventions.sh +++ b/tools/conventions/test_conventions.sh @@ -31,6 +31,5 @@ else ./analyze_gfortran_ast.py ../../obj/Linux-x86-64-gfortran/dumpast/*.ast > ast.issues ./analyze_gfortran_warnings.py ../../obj/local_warn/psmp/*.warn > warn.issues - ./analyze_src.py -b ../../ > src.issues ./summarize_issues.py --suppressions=conventions.supp ./*.issues fi diff --git a/tools/precommit/check_file_properties.py b/tools/precommit/check_file_properties.py new file mode 100755 index 0000000000..430764b107 --- /dev/null +++ b/tools/precommit/check_file_properties.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python3 + +# author: Ole Schuett + +import argparse +import re +import sys +import os +from os import path +from datetime import datetime + +FLAG_EXCEPTIONS = ( + "\$\{.*\}\$", + "__.*__", + "CUDA_VERSION", + "FD_DEBUG", + "GRID_DO_COLLOCATE", + "GRID_DO_COLLOCATE", + "INTEL_MKL_VERSION", + "LIBINT2_MAX_AM_eri", + "LIBINT_CONTRACTED_INTS", + "XC_MAJOR_VERSION", + "XC_MINOR_VERSION", + "_OPENMP", + "__COMPILE_ARCH", + "__COMPILE_DATE", + "__COMPILE_HOST", + "__COMPILE_REVISION", + "__CRAY_PM_FAKE_ENERGY", + "__DATA_DIR", + "__FFTW3_UNALIGNED", + "__FFTW3_UNALIGNED", + "__FORCE_USE_FAST_MATH", + "__FORCE_USE_FAST_MATH", + "__HAS_smm_cnt", + "__HAS_smm_ctn", + "__HAS_smm_ctt", + "__HAS_smm_dnt", + "__HAS_smm_dtn", + "__HAS_smm_dtt", + "__HAS_smm_snt", + "__HAS_smm_stn", + "__HAS_smm_stt", + "__HAS_smm_znt", + "__HAS_smm_ztn", + "__HAS_smm_ztt", + "__INTEL_COMPILER", + "__PILAENV_BLOCKSIZE", + "__PW_CUDA_NO_HOSTALLOC", + "__PW_CUDA_NO_HOSTALLOC", + "__RELEASE_VERSION", + "__RELEASE_VERSION", + "__T_C_G0", + "__YUKAWA", + "__cplusplus", +) +flag_exceptions_re = re.compile("|".join(FLAG_EXCEPTIONS)) + +portable_filename_re = re.compile(r"^[a-zA-Z0-9._/#~=+-]*$") + +BANNER_F = """\ +!--------------------------------------------------------------------------------------------------! +! CP2K: A general program to perform molecular dynamics simulations ! +! Copyright 2000-{:d} CP2K developers group ! +! ! +! SPDX-License-Identifier: GPL-2.0-or-later ! +!--------------------------------------------------------------------------------------------------! +""" + +BANNER_Fypp = """\ +#!-------------------------------------------------------------------------------------------------! +#! CP2K: A general program to perform molecular dynamics simulations ! +#! Copyright 2000-{:d} CP2K developers group ! +#! ! +#! SPDX-License-Identifier: GPL-2.0-or-later ! +#!-------------------------------------------------------------------------------------------------! +""" + +BANNER_C = """\ +/*----------------------------------------------------------------------------*/ +/* CP2K: A general program to perform molecular dynamics simulations */ +/* Copyright 2000-{:d} CP2K developers group */ +/* */ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/*----------------------------------------------------------------------------*/ +""" + +C_FAMILY_EXTENSIONS = ("c", "cu", "cpp", "h", "hpp") + +DEFAULT_EXCLUDED_DIRS = ( + ".git", + "obj", + "lib", + "exe", + "regtesting", + "tools/toolchain/build", + "tools/toolchain/install", + "tools/autotools", +) + +# ======================================================================================= +def check_file(fn): + """ + Check the source files in the given directory/filelist for convention violations, like: + + - correct copyright headers + - undocumented preprocessor flags + - stray unicode characters + """ + warnings = [] + + fn_ext = fn.rsplit(".", 1)[-1] + absfn = path.abspath(fn) + basefn = path.basename(fn) + + if not portable_filename_re.match(fn): + warnings += ["Filename %s not portable" % fn] + + if not path.exists(absfn): + return warnings # skip broken symlinks + + with open(absfn, "rb") as fhandle: + raw_content = fhandle.read() + + if b"\0" in raw_content: + return warnings # skip binary files + + content = raw_content.decode("utf8") + if "\r\n" in content: + warnings += ["Text file %s contains DOS linebreaks" % fn] + + if fn_ext not in ("pot", "patch") and basefn != "Makefile" and "\t" in content: + warnings += ["Text file %s contains tab character" % fn] + + # check banner + year = datetime.utcnow().year + if fn_ext == "F" and not content.startswith(BANNER_F.format(year)): + warnings += ["%s: Copyright banner malformed" % fn] + if fn_ext == "fypp" and not content.startswith(BANNER_Fypp.format(year)): + warnings += ["%s: Copyright banner malformed" % fn] + + if fn_ext in C_FAMILY_EXTENSIONS and not content.startswith(BANNER_C.format(year)): + warnings += ["%s: Copyright banner malformed" % fn] + + # find all flags + flags = set() + line_continuation = False + for line in content.split("\n"): + if not line_continuation: + if len(line) == 0 or line[0] != "#": + continue + if line.split()[0] not in ("#if", "#ifdef", "#ifndef", "#elif"): + continue + line = line.split("//", 1)[0] + line_continuation = line.strip().endswith("\\") + line = re.sub(r"[\\|()!&><=*/+-]", " ", line) + line = line.replace("defined", " ") + for m in line.split()[1:]: + if re.match("[0-9]+[ulUL]*", m): + continue # skip numbers + if fn_ext in ("h", "hpp") and basefn.upper().replace(".", "_") == m: + continue # ignore aptly named inclusion guards + flags.add(m) + + flags = [f for f in flags if not flag_exceptions_re.match(f)] + + cp2k_dir = path.realpath(path.join(path.dirname(__file__), "../../")) + with open(path.join(cp2k_dir, "INSTALL.md"), encoding="utf8") as fhandle: + install_txt = fhandle.read() + with open(path.join(cp2k_dir, "src/cp2k_info.F"), encoding="utf8") as fhandle: + cp2k_info = fhandle.read() + + cp2k_flags_pattern = r"FUNCTION cp2k_flags\(\)(.*)END FUNCTION cp2k_flags" + flags_src = re.search(cp2k_flags_pattern, cp2k_info, re.DOTALL).group(1) + + for f in sorted(flags): + if f not in install_txt: + warnings += ["Flag %s not mentioned in INSTALL.md" % f] + if f not in flags_src: + warnings += ["Flag %s not mentioned in cp2k_flags()" % f] + + return warnings + + +# ======================================================================================= +if __name__ == "__main__": + if len(sys.argv) != 2: + print("Usage: check_file_properties.py ") + sys.exit(1) + + fn = sys.argv[1] + warnings = check_file(fn) + + for warning in warnings: + print(warning) + + if warnings: + sys.exit(1) + + +# EOF diff --git a/tools/precommit/entrypoint.sh b/tools/precommit/entrypoint.sh index f2e74a0fd8..59aa904fbb 100755 --- a/tools/precommit/entrypoint.sh +++ b/tools/precommit/entrypoint.sh @@ -19,9 +19,9 @@ else echo -e "\n========== Starting Precommit Server ==========" cd ./tools/precommit/ - gunicorn --bind=:8080 --workers=1 --threads=8 --timeout=0 precommit_server:app &> server.logs & + gunicorn --bind=:8080 --workers=1 --threads=8 --timeout=0 precommit_server:app &> /workspace/precommit_server.logs & sleep 3 - cat server.logs + cat /workspace/precommit_server.logs echo -e "\n========== Running Precommit Checks ==========" export CP2K_PRECOMMIT_SERVER="http://127.0.0.1:8080" diff --git a/tools/precommit/precommit.py b/tools/precommit/precommit.py index 73c621fce8..c29eb462b6 100755 --- a/tools/precommit/precommit.py +++ b/tools/precommit/precommit.py @@ -68,13 +68,13 @@ def main(): assert server_hello.startswith("cp2k precommit server") # Store candidate before changing base directory and creating scratch dir. - candidate_file_list = [os.path.abspath(fn) for fn in args.files] + file_list = [os.path.abspath(fn) for fn in args.files] base_dir = Path(__file__).resolve().parent.parent.parent os.chdir(base_dir) SCRATCH_DIR.mkdir(parents=True, exist_ok=True) # Collect candidate files. - if not candidate_file_list: + if not file_list: sys.stdout.write("Searching for files...\r") sys.stdout.flush() for root, dirs, files in os.walk("."): @@ -86,7 +86,11 @@ def main(): continue if root.startswith("./tools/build_utils/fypp"): continue - if root.startswith("./tools/autotune_grid"): + if root.startswith("./tools/autotools"): + continue + if root.startswith("./data/DFTB/scc"): + continue + if root.startswith("./arch"): continue if root.startswith("./doxygen"): continue @@ -102,13 +106,14 @@ def main(): continue if root.startswith("./.git"): continue - candidate_file_list += [os.path.join(root, fn) for fn in files] + file_list += [os.path.join(root, fn) for fn in files] - # Find eligible files and sort by size as larger ones will take longer to process. - eligible_file_pattern = re.compile( - r"(./data/.*POTENTIALS?)|(.*/PACKAGE)|(.*\.(F|fypp|c|cu|h|py|md|sh))$" - ) - file_list = [fn for fn in candidate_file_list if eligible_file_pattern.match(fn)] + # Filter symlinks, backup copies, and hidden files. + file_list = [fn for fn in file_list if not os.path.islink(fn)] + file_list = [fn for fn in file_list if not fn[-1] in ("~", "#")] + file_list = [fn for fn in file_list if not os.path.basename(fn).startswith(".")] + + # Sort files by size as larger ones will take longer to process. file_list.sort(reverse=True, key=lambda fn: os.path.getsize(fn)) # Load cache. @@ -177,29 +182,25 @@ def process_file(fn, allow_modifications): if re.match(r".*\.(F|fypp)$", fn): run_local_tool("./tools/doxify/doxify.sh", fn) run_prettify(fn) - run_analyze_src(fn) - elif re.match(r".*\.(c|cu|cpp|h|hpp)$", fn): + if re.match(r".*\.(c|cu|cpp|h|hpp)$", fn): run_remote_tool("clangformat", fn) - run_analyze_src(fn) - elif re.match(r"(.*/PACKAGE)|(.*\.py)$", fn): + if re.match(r"(.*/PACKAGE)|(.*\.py)$", fn): ast.parse(orig_content, filename=fn) run_remote_tool("black", fn) - run_analyze_src(fn) - elif re.match(r".*\.sh$", fn): + if re.match(r".*\.sh$", fn): run_remote_tool("shfmt", fn) run_remote_tool("shellcheck", fn) - elif re.match(r".*\.md$", fn): + if re.match(r".*\.md$", fn): run_remote_tool("markdownlint", fn) - elif re.match(r"./data/.*POTENTIALS?$", fn): + if re.match(r"./data/.*POTENTIALS?$", fn): check_data_files() - else: - raise Exception("Unknown file extension: " + fn) + run_check_file_properties(fn) new_content = Path(fn).read_bytes() if new_content == orig_content: @@ -227,14 +228,8 @@ def run_prettify(fn): # ====================================================================================== -def run_analyze_src(fn): - run_local_tool( - "./tools/conventions/analyze_src.py", - "--fail", - "--suppressions", - "./tools/conventions/conventions.supp", - fn, - ) +def run_check_file_properties(fn): + run_local_tool("./tools/precommit/check_file_properties.py", fn) # ======================================================================================