From 26785753947962e785eb88310ebe685c519d0fbc Mon Sep 17 00:00:00 2001
From: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
Date: Fri, 23 Feb 2024 21:10:15 +0600
Subject: [PATCH 1/7] chore: add lint workflows, configs and scripts

---
 .github/scripts/lint/lint.py          | 177 ++++++++++++++++++++++++++
 .github/scripts/lint/requirements.txt |   1 +
 .github/workflows/lint.yaml           |  98 ++++++++++++++
 .github/yamllint.yaml                 |  33 +++++
 4 files changed, 309 insertions(+)
 create mode 100755 .github/scripts/lint/lint.py
 create mode 100644 .github/scripts/lint/requirements.txt
 create mode 100644 .github/workflows/lint.yaml
 create mode 100644 .github/yamllint.yaml

diff --git a/.github/scripts/lint/lint.py b/.github/scripts/lint/lint.py
new file mode 100755
index 000000000..5ab6acddc
--- /dev/null
+++ b/.github/scripts/lint/lint.py
@@ -0,0 +1,177 @@
+#!/usr/bin/python3
+
+"""
+Custom linter for chisel slice definition files.
+
+Use in addition with yamllint (https://yamllint.readthedocs.io), since this
+script only covers slice definition file specifc rules.
+"""
+
+import argparse
+import sys
+import typing
+from dataclasses import dataclass
+
+import yaml
+
+
+def parse_args() -> argparse.Namespace:
+    """
+    Parse CLI args passed to this script.
+    """
+    parser = argparse.ArgumentParser(
+        description="Lint slice definition files",
+    )
+    parser.add_argument(
+        "files",
+        metavar="file",
+        help="Chisel slice definition file(s)",
+        nargs="*",
+    )
+    parser.add_argument(
+        "--sorted-slices",
+        action=argparse.BooleanOptionalAction,
+        help="Slice names must be sorted",
+    )
+    parser.add_argument(
+        "--sorted-essential",
+        action=argparse.BooleanOptionalAction,
+        help="Entries in 'essential' must be sorted",
+    )
+    parser.add_argument(
+        "--sorted-contents",
+        action=argparse.BooleanOptionalAction,
+        help="Entries in 'contents' need to be sorted",
+    )
+    return parser.parse_args()
+
+
+def is_sorted(entries: list) -> bool:
+    """
+    Return true if a list is sorted in ASCENDING order.
+    """
+    for i in range(len(entries) - 1):
+        if entries[i] > entries[i + 1]:
+            return False
+    return True
+
+
+def lint_sorted_slices(yaml_data: dict) -> list[str] | None:
+    """
+    Slice names must be sorted.
+    """
+    slices = list(yaml_data["slices"].keys())
+    if not is_sorted(slices):
+        return ["slice names are not sorted (--sorted-slices)"]
+    return None
+
+
+def lint_sorted_essential(yaml_data: dict) -> list[str] | None:
+    """
+    'essential' entries must be sorted in a slice.
+    """
+    slices = yaml_data["slices"]
+    errs = []
+    for key, slice in slices.items():
+        if "essential" not in slice:
+            continue
+        entries = slice["essential"]
+        if is_sorted(entries):
+            continue
+        errs.append(
+            f'{key}: "essential" entries are not sorted (--sorted-essential)',
+        )
+    if len(errs) > 0:
+        return errs
+    return None
+
+
+def lint_sorted_contents(yaml_data: dict) -> list[str] | None:
+    """
+    'contents' entries must be sorted in a slice.
+    """
+    slices = yaml_data["slices"]
+    errs = []
+    for key, slice in slices.items():
+        if "contents" not in slice:
+            continue
+        entries = list(slice["contents"].keys())
+        if is_sorted(entries):
+            continue
+        errs.append(
+            f'{key}: "contents" entries are not sorted (--sorted-contents)',
+        )
+    if len(errs) > 0:
+        return errs
+    return None
+
+
+@dataclass
+class LintOptions:
+    sorted_slices: bool = True
+    sorted_essential: bool = True
+    sorted_contents: bool = True
+
+
+def lint(filename: str, opts: LintOptions) -> list[str] | None:
+    """
+    Run all lint rules on a file using the provided options.
+    """
+    with open(filename, "r", encoding="utf-8") as f:
+        data = f.read()
+    yaml_data = yaml.safe_load(data)
+
+    all_errs = []
+
+    def lint_yaml_data(func: typing.Callable):
+        errs = func(yaml_data)
+        if errs:
+            all_errs.extend(errs)
+
+    if opts.sorted_slices is not False:
+        lint_yaml_data(lint_sorted_slices)
+    if opts.sorted_essential is not False:
+        lint_yaml_data(lint_sorted_essential)
+    if opts.sorted_contents is not False:
+        lint_yaml_data(lint_sorted_contents)
+
+    if len(all_errs) > 0:
+        return all_errs
+    return None
+
+
+def print_errors(errs: dict[str, list[str]] | None) -> None:
+    """
+    Print the found linting errors.
+    """
+    if not errs:
+        return
+    for filename in sorted(errs.keys()):
+        print(f"\033[4m{filename}\033[0m")
+        for e in sorted(errs[filename]):
+            print(f"  \033[91m{'error':8s}\033[0m{e}")
+        print()
+
+
+def main() -> None:
+    """
+    The main function -- execution should start from here.
+    """
+    args = parse_args()
+    files = args.files
+    opts = LintOptions(args.sorted_slices, args.sorted_essential, args.sorted_contents)
+    #
+    ok = True
+    errs = {}
+    for file in files:
+        e = lint(file, opts)
+        if e:
+            errs[file] = e
+            ok = False
+    print_errors(errs)
+    if not ok:
+        sys.exit(1)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/.github/scripts/lint/requirements.txt b/.github/scripts/lint/requirements.txt
new file mode 100644
index 000000000..c3726e8bf
--- /dev/null
+++ b/.github/scripts/lint/requirements.txt
@@ -0,0 +1 @@
+pyyaml
diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
new file mode 100644
index 000000000..ba0670e03
--- /dev/null
+++ b/.github/workflows/lint.yaml
@@ -0,0 +1,98 @@
+name: Lint
+
+on:
+  push:
+    branches:
+      - "main"
+    paths:
+      - ".github/**"
+  pull_request:
+    branches:
+      - "main"
+    paths:
+      - ".github/**"
+  schedule:
+    # Run at 00:00 every day.
+    # Ref: https://man7.org/linux/man-pages/man5/crontab.5.html
+    - cron: "0 0 * * *"
+  workflow_call:
+
+env:
+  # chisel-releases branches to lint on.
+  RELEASES: ${{ toJson('["ubuntu-20.04","ubuntu-22.04","ubuntu-23.10","ubuntu-24.04"]') }}
+
+jobs:
+  prepare-lint:
+    runs-on: ubuntu-latest
+    name: "Prepare to lint"
+    outputs:
+      matrix: ${{ steps.set-output.outputs.matrix }}
+    steps:
+      - name: Set output
+        id: set-output
+        run: |
+          set -ex
+
+          if [[
+            "${{ github.base_ref || github.ref_name }}" == "main" ||
+            "${{ github.event_name }}" == "schedule"
+          ]]; then
+            echo "matrix={\"ref\":${{ env.RELEASES }}}" >> $GITHUB_OUTPUT
+          else
+            echo "matrix={\"ref\":[\"\"]}" >> $GITHUB_OUTPUT
+          fi
+
+  lint:
+    runs-on: ubuntu-latest
+    name: "Lint"
+    needs: prepare-lint
+    strategy:
+      fail-fast: false
+      matrix: ${{ fromJson(needs.prepare-lint.outputs.matrix) }}
+    env:
+      main-branch-path: files-from-main
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ matrix.ref }}
+
+      - name: Setup Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.10'
+
+      - name: Checkout main branch
+        uses: actions/checkout@v4
+        with:
+          ref: main
+          path: ${{ env.main-branch-path }}
+
+      - name: Install dependencies
+        env:
+          script-dir: "${{ env.main-branch-path }}/.github/scripts/lint"
+        run: |
+          set -ex
+          pip install --upgrade pip
+          pip install yamllint
+          pip install -r "${{ env.script-dir }}/requirements.txt"
+          ln -s "${{ env.script-dir }}/lint.py" lint
+
+      - name: Lint with yamllint
+        env:
+          config-path: "${{ env.main-branch-path }}/.github/yamllint.yaml"
+        run: |
+          set -ex
+          # We need to enable globstar to use the ** patterns below.
+          shopt -s globstar
+
+          yamllint -c "${{ env.config-path }}" \
+            chisel.yaml \
+            slices/
+
+      - name: Lint with SDF-specific custom linter
+        run: |
+          set -ex
+          # We need to enable globstar to use the ** patterns below.
+          shopt -s globstar
+
+          ./lint slices/**/*.yaml
diff --git a/.github/yamllint.yaml b/.github/yamllint.yaml
new file mode 100644
index 000000000..0a0811f11
--- /dev/null
+++ b/.github/yamllint.yaml
@@ -0,0 +1,33 @@
+# yamllint configurations.
+# Ref: https://yamllint.readthedocs.io/en/stable/configuration.html
+
+extends: default
+
+# Ref: https://yamllint.readthedocs.io/en/stable/rules.html
+rules:
+  braces:
+    forbid: false
+    min-spaces-inside: 0
+    max-spaces-inside: 1
+    min-spaces-inside-empty: 0
+    max-spaces-inside-empty: 0
+  brackets:
+    forbid: false
+    min-spaces-inside: 0
+    max-spaces-inside: 1
+    min-spaces-inside-empty: 0
+    max-spaces-inside-empty: 0
+  comments:
+    level: error
+  comments-indentation:
+    level: error
+  document-end:
+    present: false
+  document-start:
+    present: false
+  empty-lines:
+    max: 1
+  indentation:
+    spaces: 2
+  line-length:
+    max: 80

From 412012b9d0f30c319fef7813f5e99ca99c9265d8 Mon Sep 17 00:00:00 2001
From: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
Date: Tue, 27 Feb 2024 11:40:40 +0600
Subject: [PATCH 2/7] chore: use sorted() fn instead of comparing

---
 .github/scripts/lint/lint.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/.github/scripts/lint/lint.py b/.github/scripts/lint/lint.py
index 5ab6acddc..5c6d2377e 100755
--- a/.github/scripts/lint/lint.py
+++ b/.github/scripts/lint/lint.py
@@ -50,10 +50,7 @@ def is_sorted(entries: list) -> bool:
     """
     Return true if a list is sorted in ASCENDING order.
     """
-    for i in range(len(entries) - 1):
-        if entries[i] > entries[i + 1]:
-            return False
-    return True
+    return sorted(entries) == entries
 
 
 def lint_sorted_slices(yaml_data: dict) -> list[str] | None:

From 894c8e389519213f2f6f580b660aad5f32fcdcd5 Mon Sep 17 00:00:00 2001
From: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
Date: Mon, 4 Mar 2024 18:55:57 +0600
Subject: [PATCH 3/7] chore: replace custom python lint script with bash

---
 .github/scripts/lint/lint.py          | 174 --------------------------
 .github/scripts/lint/requirements.txt |   1 -
 .github/workflows/lint.yaml           |  48 ++++---
 3 files changed, 33 insertions(+), 190 deletions(-)
 delete mode 100755 .github/scripts/lint/lint.py
 delete mode 100644 .github/scripts/lint/requirements.txt

diff --git a/.github/scripts/lint/lint.py b/.github/scripts/lint/lint.py
deleted file mode 100755
index 5c6d2377e..000000000
--- a/.github/scripts/lint/lint.py
+++ /dev/null
@@ -1,174 +0,0 @@
-#!/usr/bin/python3
-
-"""
-Custom linter for chisel slice definition files.
-
-Use in addition with yamllint (https://yamllint.readthedocs.io), since this
-script only covers slice definition file specifc rules.
-"""
-
-import argparse
-import sys
-import typing
-from dataclasses import dataclass
-
-import yaml
-
-
-def parse_args() -> argparse.Namespace:
-    """
-    Parse CLI args passed to this script.
-    """
-    parser = argparse.ArgumentParser(
-        description="Lint slice definition files",
-    )
-    parser.add_argument(
-        "files",
-        metavar="file",
-        help="Chisel slice definition file(s)",
-        nargs="*",
-    )
-    parser.add_argument(
-        "--sorted-slices",
-        action=argparse.BooleanOptionalAction,
-        help="Slice names must be sorted",
-    )
-    parser.add_argument(
-        "--sorted-essential",
-        action=argparse.BooleanOptionalAction,
-        help="Entries in 'essential' must be sorted",
-    )
-    parser.add_argument(
-        "--sorted-contents",
-        action=argparse.BooleanOptionalAction,
-        help="Entries in 'contents' need to be sorted",
-    )
-    return parser.parse_args()
-
-
-def is_sorted(entries: list) -> bool:
-    """
-    Return true if a list is sorted in ASCENDING order.
-    """
-    return sorted(entries) == entries
-
-
-def lint_sorted_slices(yaml_data: dict) -> list[str] | None:
-    """
-    Slice names must be sorted.
-    """
-    slices = list(yaml_data["slices"].keys())
-    if not is_sorted(slices):
-        return ["slice names are not sorted (--sorted-slices)"]
-    return None
-
-
-def lint_sorted_essential(yaml_data: dict) -> list[str] | None:
-    """
-    'essential' entries must be sorted in a slice.
-    """
-    slices = yaml_data["slices"]
-    errs = []
-    for key, slice in slices.items():
-        if "essential" not in slice:
-            continue
-        entries = slice["essential"]
-        if is_sorted(entries):
-            continue
-        errs.append(
-            f'{key}: "essential" entries are not sorted (--sorted-essential)',
-        )
-    if len(errs) > 0:
-        return errs
-    return None
-
-
-def lint_sorted_contents(yaml_data: dict) -> list[str] | None:
-    """
-    'contents' entries must be sorted in a slice.
-    """
-    slices = yaml_data["slices"]
-    errs = []
-    for key, slice in slices.items():
-        if "contents" not in slice:
-            continue
-        entries = list(slice["contents"].keys())
-        if is_sorted(entries):
-            continue
-        errs.append(
-            f'{key}: "contents" entries are not sorted (--sorted-contents)',
-        )
-    if len(errs) > 0:
-        return errs
-    return None
-
-
-@dataclass
-class LintOptions:
-    sorted_slices: bool = True
-    sorted_essential: bool = True
-    sorted_contents: bool = True
-
-
-def lint(filename: str, opts: LintOptions) -> list[str] | None:
-    """
-    Run all lint rules on a file using the provided options.
-    """
-    with open(filename, "r", encoding="utf-8") as f:
-        data = f.read()
-    yaml_data = yaml.safe_load(data)
-
-    all_errs = []
-
-    def lint_yaml_data(func: typing.Callable):
-        errs = func(yaml_data)
-        if errs:
-            all_errs.extend(errs)
-
-    if opts.sorted_slices is not False:
-        lint_yaml_data(lint_sorted_slices)
-    if opts.sorted_essential is not False:
-        lint_yaml_data(lint_sorted_essential)
-    if opts.sorted_contents is not False:
-        lint_yaml_data(lint_sorted_contents)
-
-    if len(all_errs) > 0:
-        return all_errs
-    return None
-
-
-def print_errors(errs: dict[str, list[str]] | None) -> None:
-    """
-    Print the found linting errors.
-    """
-    if not errs:
-        return
-    for filename in sorted(errs.keys()):
-        print(f"\033[4m{filename}\033[0m")
-        for e in sorted(errs[filename]):
-            print(f"  \033[91m{'error':8s}\033[0m{e}")
-        print()
-
-
-def main() -> None:
-    """
-    The main function -- execution should start from here.
-    """
-    args = parse_args()
-    files = args.files
-    opts = LintOptions(args.sorted_slices, args.sorted_essential, args.sorted_contents)
-    #
-    ok = True
-    errs = {}
-    for file in files:
-        e = lint(file, opts)
-        if e:
-            errs[file] = e
-            ok = False
-    print_errors(errs)
-    if not ok:
-        sys.exit(1)
-
-
-if __name__ == "__main__":
-    main()
diff --git a/.github/scripts/lint/requirements.txt b/.github/scripts/lint/requirements.txt
deleted file mode 100644
index c3726e8bf..000000000
--- a/.github/scripts/lint/requirements.txt
+++ /dev/null
@@ -1 +0,0 @@
-pyyaml
diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
index ba0670e03..04f2dd13f 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/lint.yaml
@@ -58,8 +58,6 @@ jobs:
 
       - name: Setup Python
         uses: actions/setup-python@v5
-        with:
-          python-version: '3.10'
 
       - name: Checkout main branch
         uses: actions/checkout@v4
@@ -68,31 +66,51 @@ jobs:
           path: ${{ env.main-branch-path }}
 
       - name: Install dependencies
-        env:
-          script-dir: "${{ env.main-branch-path }}/.github/scripts/lint"
         run: |
           set -ex
           pip install --upgrade pip
           pip install yamllint
-          pip install -r "${{ env.script-dir }}/requirements.txt"
-          ln -s "${{ env.script-dir }}/lint.py" lint
+
+      - name: Install yq
+        uses: mikefarah/yq@v4.42.1
 
       - name: Lint with yamllint
         env:
           config-path: "${{ env.main-branch-path }}/.github/yamllint.yaml"
         run: |
-          set -ex
-          # We need to enable globstar to use the ** patterns below.
-          shopt -s globstar
-
+          set -e
           yamllint -c "${{ env.config-path }}" \
             chisel.yaml \
             slices/
 
-      - name: Lint with SDF-specific custom linter
+      - name: Lint with SDF-specific rules
         run: |
-          set -ex
-          # We need to enable globstar to use the ** patterns below.
-          shopt -s globstar
+          set -e
+
+          export LC_COLLATE=C
+          EXIT_CODE=0
+
+          err() {
+            echo "error:   " "$@" >&2
+            EXIT_CODE=1
+          }
+          warn() {
+            echo "warning: " "$@" >&2
+          }
+
+          for filename in $(find slices/ | grep "\.yaml$" | sort); do
+            yq '.slices | keys | .[]' "$filename" | sort -C || \
+              warn "$filename: slice names are not sorted"
+            for slice in $(yq '.slices | keys | .[]' "$filename"); do
+              key="$slice" yq \
+                '.slices | with_entries(select(.key == env(key))) | .[].essential[]' \
+                "$filename" | sort -C || \
+                err "$filename: $slice: \"essential\" entries are not sorted"
+              key="$slice" yq \
+                '.slices | with_entries(select(.key == env(key))) | .[].contents | select(.) | keys | .[]' \
+                "$filename" | sort -C || \
+                err "$filename: $slice: \"contents\" entries are not sorted"
+            done
+          done
 
-          ./lint slices/**/*.yaml
+          exit $EXIT_CODE

From 798cc9cf613659b9489d7aa8c21a5a45ae31b70d Mon Sep 17 00:00:00 2001
From: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
Date: Tue, 5 Mar 2024 15:57:51 +0600
Subject: [PATCH 4/7] chore: drop on:push and specify paths

---
 .github/workflows/lint.yaml | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
index 04f2dd13f..9f2697fc7 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/lint.yaml
@@ -1,16 +1,12 @@
 name: Lint
 
 on:
-  push:
-    branches:
-      - "main"
-    paths:
-      - ".github/**"
   pull_request:
     branches:
       - "main"
     paths:
-      - ".github/**"
+      - ".github/workflows/lint.yaml"
+      - ".github/yamllint.yaml"
   schedule:
     # Run at 00:00 every day.
     # Ref: https://man7.org/linux/man-pages/man5/crontab.5.html

From 029fdb53648d82abcae635ae5b435c5ef38202ea Mon Sep 17 00:00:00 2001
From: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
Date: Tue, 5 Mar 2024 15:58:38 +0600
Subject: [PATCH 5/7] chore: drop python setup

pip is already installed in GitHub runners.
---
 .github/workflows/lint.yaml | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
index 9f2697fc7..ccdc7895c 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/lint.yaml
@@ -52,9 +52,6 @@ jobs:
         with:
           ref: ${{ matrix.ref }}
 
-      - name: Setup Python
-        uses: actions/setup-python@v5
-
       - name: Checkout main branch
         uses: actions/checkout@v4
         with:

From e058ab623a4d5383e1c1014813fa3702d10df0d6 Mon Sep 17 00:00:00 2001
From: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
Date: Tue, 5 Mar 2024 16:08:34 +0600
Subject: [PATCH 6/7] chore: drop yq installation

yq is already installed in GitHub runners.
---
 .github/workflows/lint.yaml | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
index ccdc7895c..cfe09463b 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/lint.yaml
@@ -64,9 +64,6 @@ jobs:
           pip install --upgrade pip
           pip install yamllint
 
-      - name: Install yq
-        uses: mikefarah/yq@v4.42.1
-
       - name: Lint with yamllint
         env:
           config-path: "${{ env.main-branch-path }}/.github/yamllint.yaml"

From dbd388f4d03f3ec9e4b8e6184f6b385cd61cc422 Mon Sep 17 00:00:00 2001
From: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
Date: Wed, 6 Mar 2024 14:38:09 +0600
Subject: [PATCH 7/7] chore: checkout head_ref for PRs to main

---
 .github/workflows/lint.yaml | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
index cfe09463b..b0d5730f0 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/lint.yaml
@@ -23,6 +23,7 @@ jobs:
     name: "Prepare to lint"
     outputs:
       matrix: ${{ steps.set-output.outputs.matrix }}
+      main-ref: ${{ steps.set-output.outputs.main_ref }}
     steps:
       - name: Set output
         id: set-output
@@ -38,6 +39,12 @@ jobs:
             echo "matrix={\"ref\":[\"\"]}" >> $GITHUB_OUTPUT
           fi
 
+          # For PRs to main, use the updated files from PR head_ref by leaving
+          # main_ref unset. Otherwise, set main_ref to main.
+          if [[ "${{ github.base_ref }}" != "main" ]]; then
+            echo "main_ref=main" >> $GITHUB_OUTPUT
+          fi
+
   lint:
     runs-on: ubuntu-latest
     name: "Lint"
@@ -46,6 +53,7 @@ jobs:
       fail-fast: false
       matrix: ${{ fromJson(needs.prepare-lint.outputs.matrix) }}
     env:
+      main-branch-ref: ${{ needs.prepare-lint.outputs.main-ref }}
       main-branch-path: files-from-main
     steps:
       - uses: actions/checkout@v4
@@ -55,7 +63,7 @@ jobs:
       - name: Checkout main branch
         uses: actions/checkout@v4
         with:
-          ref: main
+          ref: ${{ env.main-branch-ref }}
           path: ${{ env.main-branch-path }}
 
       - name: Install dependencies