precommit: Enable MarkdownLint and various small improvements

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).

tools/precommit/README.md

@@ -1,41 +1,68 @@
 # 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
+<https://precommit.cp2k.org> 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
 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`.

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"]

tools/precommit/precommit.py

@@ -8,7 +8,6 @@
 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.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)
-
-    # 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)]
+    SCRATCH_DIR.mkdir(parents=True, exist_ok=True)
+
+    # 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,23 +245,23 @@ 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
 
 
 # ======================================================================================
 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"),
         ]
     )

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"])
 
 
 # ======================================================================================