diff --git a/tools/precommit/Dockerfile b/tools/precommit/Dockerfile index 1747d3a877..1a787b8a04 100644 --- a/tools/precommit/Dockerfile +++ b/tools/precommit/Dockerfile @@ -2,25 +2,30 @@ FROM ubuntu:20.04 # author: Ole Schuett -# Install Ubuntu packages. +# Install clang-format, shellcheck, and other Ubuntu packages. +# https://github.com/koalaman/shellcheck +# https://clang.llvm.org/docs/ClangFormat.html RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true && \ - apt-get update && apt-get install -y --no-install-recommends \ + apt-get update -qq && apt-get install -qq --no-install-recommends \ clang-format \ git \ less \ + npm \ python3 \ python3-pip \ python3-wheel \ python3-setuptools \ - rubygems \ shellcheck \ vim \ && rm -rf /var/lib/apt/lists/* -# Install Markdownlint. -RUN gem install mdl +# Install MarkdownLint. +# https://github.com/DavidAnson/markdownlint +# https://github.com/igorshubovych/markdownlint-cli +RUN npm install -g markdownlint-cli # Install Black Python Formatter. +# https://github.com/psf/black RUN pip3 install black # Clone cp2k repository (needed for CI mode). diff --git a/tools/precommit/README.md b/tools/precommit/README.md index 46514b8fc0..926e8ce23a 100644 --- a/tools/precommit/README.md +++ b/tools/precommit/README.md @@ -1,37 +1,53 @@ # CP2K Precommit -The precommit system consists of the following tools that analyze and format the source code: +The precommit system consists of the following tools that analyze and format the +source code: - - [doxify](../doxify/) to add Doxygen templates. - - [prettify](../prettify/) to format Fortran files. - - [analyze_src](../conventions/analyze_src.py) to check copyright banners and a few other things. - - [ast.parse](https://docs.python.org/3/library/ast.html) to check Python syntax. - - [clang-format](https://clang.llvm.org/docs/ClangFormat.html) to format C and Cuda files. - - [black](https://github.com/psf/black) to format Python scripts. - - [shellcheck](https://github.com/koalaman/shellcheck) to analyze Shell scripts. - - [markdownlint](https://github.com/markdownlint/markdownlint) to analyze Markdown files. +- [doxify](../doxify/) + to add Doxygen templates. +- [prettify](../prettify/) + to format Fortran files. +- [analyze_src](../conventions/analyze_src.py) + to check copyright banners and a few other things. +- [ast.parse](https://docs.python.org/3/library/ast.html) + to check Python syntax. +- [clang-format](https://clang.llvm.org/docs/ClangFormat.html) + to format C and Cuda files. +- [black](https://github.com/psf/black) + to format Python scripts. +- [shellcheck](https://github.com/koalaman/shellcheck) + to analyze Shell scripts. +- [markdownlint](https://github.com/markdownlint/markdownlint) + to analyze Markdown files. -In contrast to the [CP2K-CI](https://github.com/cp2k/cp2k-ci) these tools process each file individually, which makes them much more lightweight. +In contrast to the [CP2K-CI](https://github.com/cp2k/cp2k-ci) these tools +process each file individually, which makes them much more lightweight. ## Install Git Hook The [precommit.py](./precommit.py) script can be readily installed as git hook: + ``` ln -fs ../../tools/precommit/precommit.py .git/hooks/pre-commit ``` ## Server -Many of the tools listed above require more than just Python. To avoid their tedious installation a remote server is used by default. -It is hosted at https://precommit.cp2k.org via [Cloud Run](https://cloud.google.com/run). +Many of the tools listed above require more than just Python. To avoid their +tedious installation a remote server is used by default. It is hosted at + via [Cloud Run](https://cloud.google.com/run). The same server can also be started locally when Docker is available: + ``` ./start_local_server.sh ``` -The server can also be installed without Docker by following the steps in the [Dockerfile](./Dockerfile). + +The server can also be installed without Docker by following the steps in the +[Dockerfile](./Dockerfile). Once the server is up and running it can be used like this: + ``` $ export CP2K_PRECOMMIT_SERVER="http://127.0.0.1:8080" $ ./precommit.py @@ -39,3 +55,14 @@ Running precommit checks using 8 workers and server: http://127.0.0.1:8080 Searching for files... ``` +## Backups and Cache + +Before precommit run an external tool on a file it create a backup copy in +`obj/precommit`. If the tool leaves the file unmodified then the backup copy is +remove afterwards. If the tool modifies the file and precommit was invoked +without `--allow-modifications` then the backup copy is used to restore the +file's original content. + +After a successful tool run the file's timestamp is recorded in +`obj/precommit/cache.json` and precommit will skip it in future unless it's +invoked with `--no-cache`. diff --git a/tools/precommit/cloudbuild.yaml b/tools/precommit/cloudbuild.yaml index 02a18f314a..c3fcad7dc3 100644 --- a/tools/precommit/cloudbuild.yaml +++ b/tools/precommit/cloudbuild.yaml @@ -2,7 +2,7 @@ steps: - name: 'gcr.io/cloud-builders/docker' - args: ["build", "--build-arg ", "REVISION=$SHORT_SHA", "-t", "img_cp2kprecommit", "."] + args: ["build", "--build-arg", "REVISION=$SHORT_SHA", "-t", "img_cp2kprecommit", "."] - name: 'gcr.io/cloud-builders/docker' args: ["tag", "img_cp2kprecommit", "gcr.io/$PROJECT_ID/img_cp2kprecommit:$SHORT_SHA"] diff --git a/tools/precommit/precommit.py b/tools/precommit/precommit.py index ac1a70b3e4..ff18b0b4bf 100755 --- a/tools/precommit/precommit.py +++ b/tools/precommit/precommit.py @@ -8,7 +8,6 @@ import json from time import time, sleep import re import os -from os import path import sys import concurrent.futures from subprocess import PIPE, STDOUT @@ -18,9 +17,11 @@ from urllib.request import urlopen, Request from urllib.error import HTTPError import uuid from difflib import unified_diff +from pathlib import Path +import shutil -SCRATCH_DIR = "./obj/precommit" -CACHE_FILE = path.join(SCRATCH_DIR, "cache.json") +SCRATCH_DIR = Path("./obj/precommit") +CACHE_FILE = SCRATCH_DIR / "cache.json" SERVER = os.environ.get("CP2K_PRECOMMIT_SERVER", "https://precommit.cp2k.org") @@ -51,10 +52,12 @@ def main(): ) parser.add_argument( "--progressbar-wait", + metavar="SECONDS", type=int, default=1, help="number seconds in between progressbar updates", ) + parser.add_argument("files", metavar="FILE", nargs="*", help="files to process") args = parser.parse_args() # Say hello to the server. @@ -64,50 +67,55 @@ def main(): server_hello = urlopen(Request(SERVER + "/"), timeout=10).read().decode("utf8") assert server_hello.startswith("cp2k precommit server") - # Change into base dir and create scratch dir. - base_dir = path.realpath(path.join(path.dirname(__file__), "../../")) + # Store candidate before changing base directory and creating scratch dir. + candidate_file_list = [os.path.abspath(fn) for fn in args.files] + base_dir = Path(__file__).parent.parent.parent.resolve() os.chdir(base_dir) - os.makedirs(SCRATCH_DIR, exist_ok=True) + SCRATCH_DIR.mkdir(parents=True, exist_ok=True) - # Collect eligible files. - sys.stdout.write("Searching for files...\r") - sys.stdout.flush() - filename_pattern = re.compile(r".*\.(F|fypp|c|cu|h|py)$") - file_list = [] - for root, dirs, files in os.walk("."): - if root.startswith("./tools/toolchain/build"): - continue - if root.startswith("./tools/toolchain/install"): - continue - if root.startswith("./tools/prettify/fprettify"): - continue - if root.startswith("./tools/build_utils/fypp"): - continue - if root.startswith("./tools/autotune_grid"): - continue - if root.startswith("./exts"): - continue - if root.startswith("./obj"): - continue - if root.startswith("./lib"): - continue - if root.startswith("./exe"): - continue - if root.startswith("./regtesting"): - continue - if root.startswith("./.git"): - continue - file_list += [path.join(root, fn) for fn in files if filename_pattern.match(fn)] + # Collect candidate files. + if not candidate_file_list: + sys.stdout.write("Searching for files...\r") + sys.stdout.flush() + for root, dirs, files in os.walk("."): + if root.startswith("./tools/toolchain/build"): + continue + if root.startswith("./tools/toolchain/install"): + continue + if root.startswith("./tools/prettify/fprettify"): + continue + if root.startswith("./tools/build_utils/fypp"): + continue + if root.startswith("./tools/autotune_grid"): + continue + if root.startswith("./exts"): + continue + if root.startswith("./obj"): + continue + if root.startswith("./lib"): + continue + if root.startswith("./exe"): + continue + if root.startswith("./regtesting"): + continue + if root.startswith("./.git"): + continue + candidate_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".*\.(F|fypp|c|cu|h|py|md)$") + file_list = [fn for fn in candidate_file_list if eligible_file_pattern.match(fn)] + file_list.sort(reverse=True, key=lambda fn: os.path.getsize(fn)) # Load cache. - should_load_cache = path.exists(CACHE_FILE) and not args.no_cache - cache = json.load(open(CACHE_FILE)) if should_load_cache else {} + should_load_cache = CACHE_FILE.exists() and not args.no_cache + cache = json.loads(CACHE_FILE.read_text()) if should_load_cache else {} # Launch async processing of files. futures = {} executor = concurrent.futures.ThreadPoolExecutor(max_workers=args.num_workers) for fn in file_list: - if path.getmtime(fn) != cache.get(fn, -1): + if os.path.getmtime(fn) != cache.get(fn, -1): futures[fn] = executor.submit(process_file, fn, args.allow_modifications) num_skipped = len(file_list) - len(futures) @@ -119,11 +127,11 @@ def main(): if f.done(): num_done += 1 if not f.exception(): - cache[fn] = path.getmtime(fn) + cache[fn] = os.path.getmtime(fn) elif fn not in failed_files: failed_files.add(fn) print_box(fn, str(f.exception())) - json.dump(cache, open(CACHE_FILE, "w")) + CACHE_FILE.write_text(json.dumps(cache)) progressbar = "=" * int(60 * num_done / len(file_list)) sys.stdout.write( f"[{progressbar:60s}] {num_done} / {len(file_list)} files processed\r" @@ -157,7 +165,10 @@ def print_box(fn, message): # ====================================================================================== def process_file(fn, allow_modifications): - orig_content = open(fn, "rb").read() + # Make a backup copy. + orig_content = Path(fn).read_bytes() + bak_fn = SCRATCH_DIR / f"{Path(fn).name}_{int(time())}.bak" + shutil.copy2(fn, bak_fn) if re.match(r".*\.(F|fypp)$", fn): run_local_tool("./tools/doxify/doxify.sh", fn) @@ -182,16 +193,12 @@ def process_file(fn, allow_modifications): else: raise Exception("Unknown file extension: " + fn) - new_content = open(fn, "rb").read() + new_content = Path(fn).read_bytes() if new_content == orig_content: - return + bak_fn.unlink() # remove backup - # Deal with a modified file. - if allow_modifications: - bak_fn = path.join(SCRATCH_DIR, f"{path.basename(fn)}_{int(time())}.bak") - open(bak_fn, "wb").write(orig_content) # make a backup copy - else: - open(fn, "wb").write(orig_content) # restore origin content + elif not allow_modifications: + bak_fn.replace(fn) # restore origin content try: orig_lines = orig_content.decode("utf8").split("\n") new_lines = new_content.decode("utf8").split("\n") @@ -238,7 +245,7 @@ def run_remote_tool(tool, fn): if r.status == 304: pass # file not modified elif r.status == 200: - open(fn, "wb").write(r.read()) + Path(fn).write_bytes(r.read()) else: raise Exception(r.read().decode("utf8")) # something went wrong @@ -246,15 +253,15 @@ def run_remote_tool(tool, fn): # ====================================================================================== def http_post(url, fn): # This would be so much easier with the Requests library where it'd be a one-liner: - # return requests.post(url, files={path.basename(fn): open(fn, "rb").read()}) + # return requests.post(url, files={Path(fn).name: Path(fn).read_bytes()}) boundary = uuid.uuid1().hex - name = path.basename(fn) + name = Path(fn).name data = b"".join( [ f"--{boundary}\r\nContent-Disposition: ".encode("utf8"), f'form-data; name="{name}"; filename="{name}"\r\n\r\n'.encode("utf8"), - open(fn, "rb").read(), + Path(fn).read_bytes(), f"\r\n--{boundary}--\r\n".encode("utf8"), ] ) diff --git a/tools/precommit/precommit_server.py b/tools/precommit/precommit_server.py index 0bec5e629c..9421846a93 100755 --- a/tools/precommit/precommit_server.py +++ b/tools/precommit/precommit_server.py @@ -37,7 +37,7 @@ def shellcheck(): # ====================================================================================== @app.route("/markdownlint", methods=["POST"]) def markdownlint(): - return run_tool(["mdl"]) + return run_tool(["markdownlint"]) # ======================================================================================