Fix brittany formatter & misc improvements (#222)

* Fix brittany installation, needed the `--reorder-goals` argument to be
added so that a version that was not the latest version could be
installed so that it was compatible with the base lib version shipped
with Ubuntu 20.04. We'll upgrade to 22.04 eventually (before it falls
out of LTS). Ref: #221
* Add a `make fmt-build-common` target which allows tagging a docker
image containing just the base software and not any formatters, to make
it easy to debug formatter installation manually.
* Update `apheleia-ft` to also run formatter tests when any files
affecting a formatter are changed, which includes the installation
script, the sample input/output, and also any scripts (e.g.
`apheleia-phpcs`) that it uses. We don't have any logic that will run
all formatter tests at once, because that is unwieldy. That can be done
manually if making a big change.
* Update to actions/checkout@v4 from v2 because the older one was listed
as deprecated.
* Print full stacktraces when `apheleia-ft` fails with an Elisp error.

Author: raxod502

.github/workflows/formatters.yml

@@ -5,12 +5,12 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout pull request
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
         with:
           ref: ${{ github.event.pull_request.head.sha }}
-      - name: Fetch master
-        run: |
-          git fetch
+          # No shallow clone, we want to be able to compare PR branch
+          # to main.
+          fetch-depth: 0
       - name: Test changed formatters
         run: |
           set -euo pipefail

.github/workflows/lint.yml

@@ -12,7 +12,7 @@ jobs:
         emacs_version: [26, 27, 28, "master"]
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
       - name: Run linters
         env:
           VERSION: ${{ matrix.emacs_version }}

Makefile

@@ -80,7 +80,11 @@ docker: ## Start a Docker shell; e.g. make docker VERSION=25.3
 
 .PHONY: fmt-build  # env vars: FORMATTERS, TAG
 fmt-build: ## Build a Docker image with formatters installed
-	@test/formatters/build-image.bash
+	@COMMON=0 test/formatters/build-image.bash
+
+.PHONY: fmt-build-common  # env var: TAG
+fmt-build-common: ## Build a Docker image with just the common base
+	@COMMON=1 test/formatters/build-image.bash
 
 .PHONY: fmt-docker  # env var: TAG
 fmt-docker: ## Start a Docker shell for testing formatters

test/formatters/Dockerfile

@@ -1,10 +1,15 @@
 # Ubuntu 20.04 LTS supported until April 2025
-FROM ubuntu:20.04
+FROM ubuntu:20.04 AS common
 
 WORKDIR /build
 COPY install-common.bash /build/
 RUN ./install-common.bash
 
+# Add an intermediate tag so that it is possible to execute debugging
+# code before formatter installation. This is necessary because with
+# the newer docker build based on buildkit, you can't access
+# intermediate image layers by default anymore.
+FROM common
 ARG FORMATTERS
 COPY install-formatters.bash /build/
 COPY installers /build/installers/

test/formatters/apheleia-ft.el

@@ -9,12 +9,26 @@
 
 (require 'cl-lib)
 (require 'map)
+(require 'subr-x)
 
 (defvar apheleia-ft--test-dir
   (file-name-directory
    (or load-file-name buffer-file-name))
   "Directory containing this module.")
 
+(defvar apheleia-ft--repo-dir
+  (expand-file-name (locate-dominating-file apheleia-ft--test-dir ".git"))
+  "Root directory of the Git repository.
+Guaranteed to be absolute and expanded.")
+
+(defun apheleia-ft--relative-truename (path)
+  "Given PATH relative to repo root, resolve symlinks.
+Return another path relative to repo root."
+  (string-remove-prefix
+   apheleia-ft--repo-dir
+   (file-truename
+    (expand-file-name path apheleia-ft--repo-dir))))
+
 (defun apheleia-ft--get-formatters (&optional all)
   "Return list of strings naming the formatters to run.
 This is determined by the environment variable FORMATTERS,
@@ -56,6 +70,44 @@ already in memory on the current branch."
       (goto-char (point-min))
       (read (current-buffer)))))
 
+(defun apheleia-ft--files-changed-since (ref)
+  "Get a list of the files changed between REF and HEAD."
+  (let ((stderr-file (make-temp-file "apheleia-ft-stderr-")))
+    (with-temp-buffer
+      (let ((exit-status
+             (call-process
+              "git" nil (list (current-buffer) stderr-file) nil
+              "diff" "--name-only" "--diff-filter=d" (format "%s..." ref))))
+        (unless (zerop exit-status)
+          (with-temp-buffer
+            (insert-file-contents stderr-file)
+            (princ (buffer-string)))
+          (error "Failed to 'git diff', got exit status %S" exit-status)))
+      (split-string (buffer-string)))))
+
+(defun apheleia-ft--formatters-depending-on-file (changed-file)
+  "Given CHANGED-FILE, return list of formatters affected by it.
+Return formatters as string names. This is used to determine
+which formatters need tests to be run. CHANGED-FILE should be
+relative to repo root, as returned by git diff --name-only."
+  (setq changed-file (apheleia-ft--relative-truename changed-file))
+  (save-match-data
+    (cond
+     ((string-match
+       "^test/formatters/installers/\\([^/]+\\)\\.bash$" changed-file)
+      (list (match-string 1 changed-file)))
+     ((string-match
+       "^test/formatters/samplecode/\\([^/]+\\)/[^/]+$" changed-file)
+      (list (match-string 1 changed-file)))
+     ((string-match
+       "^scripts/formatters/\\([^/]+\\)$" changed-file)
+      (let ((script (match-string 1 changed-file)))
+        (map-keys
+         (map-filter
+          (lambda (fmt def)
+            (member script def))
+          apheleia-formatters)))))))
+
 (defun apheleia-ft--get-formatters-for-pull-request ()
   "Return list of formatter string names that were touched in this PR.
 This means their commands in `apheleia-formatters' are different
@@ -69,6 +121,13 @@ main."
        (unless (equal command (alist-get formatter old-formatters))
          (push (symbol-name formatter) touched-formatters)))
      new-formatters)
+    (mapc
+     (lambda (changed-file)
+       (setq touched-formatters
+             (nconc
+              (apheleia-ft--formatters-depending-on-file changed-file)
+              touched-formatters)))
+     (apheleia-ft--files-changed-since "origin/main"))
     touched-formatters))
 
 (defun apheleia-ft-changed ()

test/formatters/build-image.bash

@@ -4,7 +4,9 @@ set -euo pipefail
 
 echo >&2 "build-image.bash: tagging apheleia-formatters:${TAG:-latest}"
 
-if [[ -n "${FORMATTERS:-}" ]]; then
+if [[ "${COMMON}" == "1" ]]; then
+    echo "build-image.bash: will tag common base image only"
+elif [[ -n "${FORMATTERS:-}" ]]; then
     echo "build-image.bash: will install these formatters: ${FORMATTERS}"
 else
     echo "build-image.bash: will install all formatters by default"
@@ -17,6 +19,11 @@ if [[ "$OSTYPE" != darwin* ]] && [[ "$EUID" != 0 ]]; then
     docker=(sudo -E "${docker[@]}")
 fi
 
-exec "${docker[@]}" build .                    \
+args=()
+if [[ "${COMMON}" == "1" ]]; then
+    args+=(--target=common)
+fi
+
+exec "${docker[@]}" build . "${args[@]}"       \
      -t "apheleia-formatters:${TAG:-latest}"   \
      --build-arg "FORMATTERS=${FORMATTERS:-}"

test/formatters/installers/brittany.bash

@@ -7,5 +7,5 @@ apt-get install -y cabal-install-3.4 ghc
 ln -s /opt/cabal/bin/cabal /usr/local/bin/
 
 cabal v2-update
-cabal v2-install brittany
+cabal v2-install brittany --reorder-goals
 cp -L "$HOME/.cabal/bin/brittany" /usr/local/bin/

test/formatters/run-func.bash

@@ -8,4 +8,5 @@ set -euo pipefail
 cd "$(dirname "$0")"
 repo="$(cd ../.. && pwd)"
 
-exec emacs --batch -L "${repo}" -L . -l apheleia-ft -f "$1"
+exec emacs --batch -L "${repo}" -L . -l apheleia-ft \
+     --eval "(setq debug-on-error t)" -f "$1"