From 127560ae223255d0e081b932e902d2da242abf06 Mon Sep 17 00:00:00 2001 From: Nick Brassel Date: Fri, 6 Oct 2023 10:34:23 +1100 Subject: [PATCH] Add `qmk ci-validate-aliases` (#22205) --- .github/workflows/lint.yml | 25 +-------- data/mappings/keyboard_aliases.hjson | 64 +++++++++++++---------- lib/python/qmk/cli/__init__.py | 1 + lib/python/qmk/cli/ci/__init__.py | 0 lib/python/qmk/cli/ci/validate_aliases.py | 46 ++++++++++++++++ lib/python/qmk/commands.py | 11 ++-- lib/python/qmk/keyboard.py | 5 +- 7 files changed, 97 insertions(+), 55 deletions(-) create mode 100644 lib/python/qmk/cli/ci/__init__.py create mode 100644 lib/python/qmk/cli/ci/validate_aliases.py diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9862ff502c..19dd7c70a9 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -74,31 +74,10 @@ jobs: fi exit $exit_code - - name: Verify at most one added keyboard + - name: Verify keyboard aliases if: always() shell: 'bash {0}' run: | git reset --hard git clean -xfd - - # Get the keyboard list and count for the target branch - git checkout -f ${{ github.base_ref }} - git pull --ff-only - QMK_KEYBOARDS_BASE=$(qmk list-keyboards) - QMK_KEYBOARDS_BASE_COUNT=$(qmk list-keyboards | wc -l) - - # Get the keyboard list and count for the PR - git checkout -f ${{ github.head_ref }} - git merge --no-commit --squash ${{ github.base_ref }} - QMK_KEYBOARDS_PR=$(qmk list-keyboards) - QMK_KEYBOARDS_PR_COUNT=$(qmk list-keyboards | wc -l) - - echo "::group::Keyboards changes in this PR" - diff -d -U 0 <(echo "$QMK_KEYBOARDS_BASE") <(echo "$QMK_KEYBOARDS_PR") | grep -vE '^(---|\+\+\+|@@)' | sed -e 's@^-@Removed: @g' -e 's@^+@ Added: @g' - echo "::endgroup::" - - if [[ $QMK_KEYBOARDS_PR_COUNT -gt $(($QMK_KEYBOARDS_BASE_COUNT + 1)) ]]; then - echo "More than one keyboard added in this PR -- see the PR Checklist." - echo "::error::More than one keyboard added in this PR -- see the PR Checklist." - exit 1 - fi + qmk ci-validate-aliases diff --git a/data/mappings/keyboard_aliases.hjson b/data/mappings/keyboard_aliases.hjson index 60c5deaad5..8e4265dc01 100644 --- a/data/mappings/keyboard_aliases.hjson +++ b/data/mappings/keyboard_aliases.hjson @@ -4,6 +4,13 @@ // "target": "" // } // + + /* This list of aliases is for testing purposes -- ensures "linked list" recursive traversal works correctly. */ + "_test_a": { "target": "_test_b" }, + "_test_b": { "target": "_test_c" }, + "_test_c": { "target": "planck/rev6" }, + + /* The main list of aliases for moved keyboards within QMK. */ "2_milk": { "target": "spaceman/2_milk" }, @@ -35,7 +42,7 @@ "target": "amjkeyboard/amjpad" }, "angel64": { - "target": "angel64/alpha" + "target": "kakunpc/angel64/alpha" }, "ashpil/modelm_usbc": { "target": "ibm/model_m/ashpil_usbc" @@ -47,10 +54,10 @@ "target": "viktus/at101_bh" }, "atom47/rev2": { - "target": "maartenwut/atom47/rev2" + "target": "evyd13/atom47/rev2" }, "atom47/rev3": { - "target": "maartenwut/atom47/rev3" + "target": "evyd13/atom47/rev3" }, "bakeneko60": { "target": "kkatano/bakeneko60" @@ -65,7 +72,7 @@ "target": "bear_face/v1" }, "bm16a": { - "target": "kprepublic/bm16a" + "target": "kprepublic/bm16a/v1" }, "bm16s": { "target": "kprepublic/bm16s" @@ -77,16 +84,16 @@ "target": "kprepublic/bm43a" }, "bm60poker": { - "target": "kprepublic/bm60poker" + "target": "kprepublic/bm60hsrgb_poker/rev1" }, "bm60rgb": { - "target": "kprepublic/bm60rgb" + "target": "kprepublic/bm60hsrgb/rev1" }, "bm60rgb_iso": { - "target": "kprepublic/bm60rgb_iso" + "target": "kprepublic/bm60hsrgb_iso/rev1" }, "bm68rgb": { - "target": "kprepublic/bm68rgb" + "target": "kprepublic/bm68hsrgb/rev1" }, "bpiphany/pegasushoof": { "target": "bpiphany/pegasushoof/2013" @@ -140,7 +147,10 @@ "target": "jagdpietr/drakon" }, "durgod/k320": { - "target": "durgod/k3x0/k320" + "target": "durgod/k320/base" + }, + "durgod/k3x0/k320": { + "target": "durgod/k320/base" }, "durgod/hades": { "target": "durgod/dgk6x/hades_ansi" @@ -269,7 +279,7 @@ "target": "idb/idb_60" }, "idobo": { - "target": "idobao/id75" + "target": "idobao/id75/v1" }, "jacky_studio/piggy60": { "target": "jacky_studio/piggy60/rev1" @@ -401,7 +411,7 @@ "target": "mechlovin/adelais/rgb_led/rev1" }, "mechlovin/adelais/standard_led": { - "target": "mechlovin/adelais/standard_led/rev2" + "target": "mechlovin/adelais/standard_led/arm/rev2" }, "mechlovin/delphine": { "target": "mechlovin/delphine/mono_led" @@ -455,10 +465,10 @@ "target": "pabile/p20/ver1" }, "pancake/feather": { - "target": "spaceman/pancake/feather" + "target": "spaceman/pancake/rev1/feather" }, "pancake/promicro": { - "target": "spaceman/pancake/promicro" + "target": "spaceman/pancake/rev1/promicro" }, "peiorisboards/ixora": { "target": "coarse/ixora" @@ -467,7 +477,7 @@ "target": "dm9records/plaid" }, "plain60": { - "target": "maartenwut/plain60" + "target": "evyd13/plain60" }, "ploopyco/trackball": { "target": "ploopyco/trackball/rev1_005" @@ -503,10 +513,10 @@ "target": "wilba_tech/rama_works_u80_a" }, "ramonimbao/herringbone": { - "target": "ramonimbao/herringbone/v1" + "target": "rmi_kb/herringbone/v1" }, "ramonimbao/mona": { - "target": "ramonimbao/mona/v1" + "target": "rmi_kb/mona/v1" }, "rgbkb/pan": { "target": "rgbkb/pan/rev1/32a" @@ -542,10 +552,10 @@ "target": "tkw/stoutgat/v1" }, "suihankey": { - "target": "suihankey/split/alpha" + "target": "kakunpc/suihankey/split/alpha" }, "ta65": { - "target": "maartenwut/ta65" + "target": "evyd13/ta65" }, "tartan": { "target": "dm9records/tartan" @@ -563,13 +573,13 @@ "target": "matthewdias/txuu" }, "underscore33": { - "target": "underscore33/rev1" + "target": "tominabox1/underscore33/rev1" }, "vinta": { "target": "coarse/vinta" }, "wasdat": { - "target": "maartenwut/wasdat" + "target": "evyd13/wasdat" }, "westfoxtrot/cypher": { "target": "westfoxtrot/cypher/rev1" @@ -581,10 +591,10 @@ "target": "xiudi/xd002" }, "xd004": { - "target": "xiudi/xd004" + "target": "xiudi/xd004/v1" }, "xd60": { - "target": "xiudi/xd60" + "target": "xiudi/xd60/rev2" }, "xd68": { "target": "xiudi/xd68" @@ -831,7 +841,7 @@ "target": "kagizaraya/halberd" }, "handwired/hillside/0_1": { - "target": "handwired/hillside/48" + "target": "hillside/48/0_1" }, "hecomi/alpha": { "target": "takashiski/hecomi/alpha" @@ -843,10 +853,10 @@ "target": "bpiphany/hid_liber" }, "id67/default_rgb": { - "target": "idobao/id67/default_rgb" + "target": "idobao/id67" }, "id67/rgb": { - "target": "idobao/id67/rgb" + "target": "idobao/id67" }, "id80": { "target": "idobao/id80/v2/ansi" @@ -1236,7 +1246,7 @@ "target": "marksard/treadstone48/rev2" }, "tronguylabs/m122_3270": { - "target": "ibm/model_m_122/m122_3270" + "target": "ibm/model_m_122/m122_3270/teensy" }, "ua62": { "target": "nacly/ua62" @@ -1290,7 +1300,7 @@ "target": "ydkb/yd68" }, "ymd75": { - "target": "ymdk/ymd75" + "target": "ymdk/ymd75/rev1" }, "ymd96": { "target": "ymdk/ymd96" diff --git a/lib/python/qmk/cli/__init__.py b/lib/python/qmk/cli/__init__.py index 9c3decf4f7..b8bc99aa0d 100644 --- a/lib/python/qmk/cli/__init__.py +++ b/lib/python/qmk/cli/__init__.py @@ -31,6 +31,7 @@ safe_commands = [ ] subcommands = [ + 'qmk.cli.ci.validate_aliases', 'qmk.cli.bux', 'qmk.cli.c2json', 'qmk.cli.cd', diff --git a/lib/python/qmk/cli/ci/__init__.py b/lib/python/qmk/cli/ci/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lib/python/qmk/cli/ci/validate_aliases.py b/lib/python/qmk/cli/ci/validate_aliases.py new file mode 100644 index 0000000000..a205d03cff --- /dev/null +++ b/lib/python/qmk/cli/ci/validate_aliases.py @@ -0,0 +1,46 @@ +"""Validates the list of keyboard aliases. +""" +from pathlib import Path + +from milc import cli + +from qmk.json_schema import json_load +from qmk.keyboard import resolve_keyboard, keyboard_folder + + +def _safe_keyboard_folder(target): + try: + return keyboard_folder(target) # throws ValueError if it's invalid + except Exception: + return None + + +def _target_keyboard_exists(target): + # If there's no target, then we can't build it. + if not target: + return False + + # If the target directory existed but there was no rules.mk or rules.mk was incorrectly parsed, then we can't build it. + if not resolve_keyboard(target): + return False + + # If the target directory exists but it itself has an invalid alias or invalid rules.mk, then we can't build it either. + if not _safe_keyboard_folder(target): + return False + + # As far as we can tell, we can build it! + return True + + +@cli.subcommand('Validates the list of keyboard aliases.', hidden=True) +def ci_validate_aliases(cli): + aliases = json_load(Path('data/mappings/keyboard_aliases.hjson')) + + success = True + for alias in aliases.keys(): + target = aliases[alias].get('target', None) + if not _target_keyboard_exists(target): + cli.log.error(f'Keyboard alias {alias} has a target that doesn\'t exist: {target}') + success = False + + return success diff --git a/lib/python/qmk/commands.py b/lib/python/qmk/commands.py index 660b2ff72e..34696e3793 100644 --- a/lib/python/qmk/commands.py +++ b/lib/python/qmk/commands.py @@ -212,13 +212,16 @@ def parse_configurator_json(configurator_file): cli.log.error(f'Invalid JSON keymap: {configurator_file} : {e.message}') exit(1) - orig_keyboard = user_keymap['keyboard'] + keyboard = user_keymap['keyboard'] aliases = json_load(Path('data/mappings/keyboard_aliases.hjson')) - if orig_keyboard in aliases: - if 'target' in aliases[orig_keyboard]: - user_keymap['keyboard'] = aliases[orig_keyboard]['target'] + while keyboard in aliases: + last_keyboard = keyboard + keyboard = aliases[keyboard].get('target', keyboard) + if keyboard == last_keyboard: + break + user_keymap['keyboard'] = keyboard return user_keymap diff --git a/lib/python/qmk/keyboard.py b/lib/python/qmk/keyboard.py index 18ca5a9534..9826f3f887 100644 --- a/lib/python/qmk/keyboard.py +++ b/lib/python/qmk/keyboard.py @@ -92,8 +92,11 @@ def keyboard_folder(keyboard): """ aliases = json_load(Path('data/mappings/keyboard_aliases.hjson')) - if keyboard in aliases: + while keyboard in aliases: + last_keyboard = keyboard keyboard = aliases[keyboard].get('target', keyboard) + if keyboard == last_keyboard: + break rules_mk_file = Path(base_path, keyboard, 'rules.mk')