From 247e989377fe7e4a81e5307c1ffdf7e9930c0855 Mon Sep 17 00:00:00 2001 From: Jason Smith Date: Mon, 22 Apr 2024 19:10:35 -0700 Subject: [PATCH] =?UTF-8?q?=E2=9C=85=20=20CI=20-=20Validate=20Pins=20Forma?= =?UTF-8?q?tting=20(#26996)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/ci-validate-pins.yml | 51 +++++++ Makefile | 12 +- buildroot/share/git/mfhelp | 2 +- buildroot/share/scripts/pinsformat.js | 197 ------------------------- buildroot/share/scripts/pinsformat.py | 24 ++- 5 files changed, 78 insertions(+), 208 deletions(-) create mode 100644 .github/workflows/ci-validate-pins.yml delete mode 100755 buildroot/share/scripts/pinsformat.js diff --git a/.github/workflows/ci-validate-pins.yml b/.github/workflows/ci-validate-pins.yml new file mode 100644 index 00000000000..e093eb92c7b --- /dev/null +++ b/.github/workflows/ci-validate-pins.yml @@ -0,0 +1,51 @@ +# +# ci-validate-pins.yml +# Validate that all of the pins files are unchanged by pinsformat.py +# + +name: CI - Validate Pins Files + +on: + pull_request: + branches: + - bugfix-2.1.x + # Cannot be enabled on 2.1.x until it contains the unit test framework + #- 2.1.x + paths: + - 'Marlin/src/pins/*/**' + push: + branches: + - bugfix-2.1.x + # Cannot be enabled on 2.1.x until it contains the unit test framework + #- 2.1.x + paths: + - 'Marlin/src/pins/*/**' + +jobs: + validate_pins_files: + name: Validate Pins Files + if: github.repository == 'MarlinFirmware/Marlin' + + runs-on: ubuntu-latest + + steps: + - name: Check out the PR + uses: actions/checkout@v4 + + - name: Cache pip + uses: actions/cache@v4 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + + - name: Select Python 3.9 + uses: actions/setup-python@v5 + with: + python-version: '3.9' + architecture: 'x64' + + - name: Validate all pins files + run: | + make validate-pins -j diff --git a/Makefile b/Makefile index db4eeba378b..ce925a58438 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,8 @@ UNIT_TEST_CONFIG ?= default help: @echo "Tasks for local development:" @echo "make marlin : Build marlin for the configured board" - @echo "make format-pins : Reformat all pins files" + @echo "make format-pins -j : Reformat all pins files (-j for parallel execution)" + @echo "make validate-pins -j : Validate all pins files, fails if any require reformatting" @echo "make tests-single-ci : Run a single test from inside the CI" @echo "make tests-single-local : Run a single test locally" @echo "make tests-single-local-docker : Run a single test locally, using docker" @@ -81,7 +82,14 @@ setup-local-docker: PINS := $(shell find Marlin/src/pins -mindepth 2 -name '*.h') +.PHONY: $(PINS) format-pins validate-pins + $(PINS): %: - @echo "Formatting $@" && node $(SCRIPTS_DIR)/pinsformat.js $@ + @echo "Formatting $@" + @python $(SCRIPTS_DIR)/pinsformat.py $< $@ format-pins: $(PINS) + +validate-pins: format-pins + @echo "Validating pins files" + @git diff --exit-code || (git status && echo "\nError: Pins files are not formatted correctly. Run \"make format-pins\" to fix.\n" && exit 1) diff --git a/buildroot/share/git/mfhelp b/buildroot/share/git/mfhelp index 46a0ebfc533..aff34b866f6 100755 --- a/buildroot/share/git/mfhelp +++ b/buildroot/share/git/mfhelp @@ -41,6 +41,6 @@ Modify Configuration.h / Configuration_adv.h: Modify pins files: pins_set ............. Set the value of a pin in a pins file - pinsformat.js ........ Node.js script to format pins files + pinsformat.py ........ Python script to format pins files THIS diff --git a/buildroot/share/scripts/pinsformat.js b/buildroot/share/scripts/pinsformat.js deleted file mode 100755 index 16e9dcb88f0..00000000000 --- a/buildroot/share/scripts/pinsformat.js +++ /dev/null @@ -1,197 +0,0 @@ -#!/usr/bin/env node - -// -// Formatter script for pins_MYPINS.h files -// -// Usage: mffmt [infile] [outfile] -// -// With no parameters convert STDIN to STDOUT -// - -const fs = require("fs"); - -var do_log = false -function logmsg(msg, line='') { - if (do_log) console.log(msg, line); -} - -// String lpad / rpad -String.prototype.lpad = function(len, chr) { - if (!len) return this; - if (chr === undefined) chr = ' '; - var s = this+'', need = len - s.length; - if (need > 0) s = new Array(need+1).join(chr) + s; - return s; -}; - -String.prototype.rpad = function(len, chr) { - if (!len) return this; - if (chr === undefined) chr = ' '; - var s = this+'', need = len - s.length; - if (need > 0) s += new Array(need+1).join(chr); - return s; -}; - -// Concatenate a string, adding a space if necessary -// to avoid merging two words -String.prototype.concat_with_space = function(str) { - const c = this.substr(-1), d = str.charAt(0); - if (c !== ' ' && c !== '' && d !== ' ' && d !== '') - str = ' ' + str; - return this + str; -}; - -const mpatt = [ '-?\\d{1,3}', 'P[A-I]\\d+', 'P\\d_\\d+', 'Pin[A-Z]\\d\\b' ], - definePatt = new RegExp(`^\\s*(//)?#define\\s+[A-Z_][A-Z0-9_]+\\s+(${mpatt.join('|')})\\s*(//.*)?$`, 'gm'), - ppad = [ 3, 4, 5, 5 ], - col_comment = 50, - col_value_rj = col_comment - 3; - -var mexpr = []; -for (let m of mpatt) mexpr.push(new RegExp('^' + m + '$')); - -const argv = process.argv.slice(2), argc = argv.length; - -var src_file = 0, dst_file; -if (argc > 0) { - let ind = 0; - if (argv[0] == '-v') { do_log = true; ind++; } - dst_file = src_file = argv[ind++]; - if (ind < argc) dst_file = argv[ind]; -} - -// Read from file or STDIN until it terminates -const filtered = process_text(fs.readFileSync(src_file).toString()); -if (dst_file) - fs.writeFileSync(dst_file, filtered); -else - console.log(filtered); - -// Find the pin pattern so non-pin defines can be skipped -function get_pin_pattern(txt) { - var r, m = 0, match_count = [ 0, 0, 0, 0 ]; - var max_match_count = 0, max_match_index = -1; - definePatt.lastIndex = 0; - while ((r = definePatt.exec(txt)) !== null) { - let ind = -1; - if (mexpr.some((p) => { - ind++; - const didmatch = r[2].match(p); - return r[2].match(p); - }) ) { - const m = ++match_count[ind]; - if (m > max_match_count) { - max_match_count = m; - max_match_index = ind; - } - } - } - if (max_match_index === -1) return null; - - return { match:mpatt[max_match_index], pad:ppad[max_match_index] }; -} - -function process_text(txt) { - if (!txt.length) return '(no text)'; - const patt = get_pin_pattern(txt); - if (!patt) return txt; - const pindefPatt = new RegExp(`^(\\s*(//)?#define)\\s+([A-Z_][A-Z0-9_]+)\\s+(${patt.match})\\s*(//.*)?$`), - noPinPatt = new RegExp(`^(\\s*(//)?#define)\\s+([A-Z_][A-Z0-9_]+)\\s+(-1)\\s*(//.*)?$`), - skipPatt1 = new RegExp('^(\\s*(//)?#define)\\s+(AT90USB|USBCON|(BOARD|DAC|FLASH|HAS|IS|USE)_.+|.+_(ADDRESS|AVAILABLE|BAUDRATE|CLOCK|CONNECTION|DEFAULT|ERROR|EXTRUDERS|FREQ|ITEM|MKS_BASE_VERSION|MODULE|NAME|ONLY|ORIENTATION|PERIOD|RANGE|RATE|READ_RETRIES|SERIAL|SIZE|SPI|STATE|STEP|TIMER|VERSION))\\s+(.+)\\s*(//.*)?$'), - skipPatt2 = new RegExp('^(\\s*(//)?#define)\\s+([A-Z_][A-Z0-9_]+)\\s+(0x[0-9A-Fa-f]+|\d+|.+[a-z].+)\\s*(//.*)?$'), - skipPatt3 = /^\s*#e(lse|ndif)\b.*$/, - aliasPatt = new RegExp('^(\\s*(//)?#define)\\s+([A-Z_][A-Z0-9_]+)\\s+([A-Z_][A-Z0-9_()]+)\\s*(//.*)?$'), - switchPatt = new RegExp('^(\\s*(//)?#define)\\s+([A-Z_][A-Z0-9_]+)\\s*(//.*)?$'), - undefPatt = new RegExp('^(\\s*(//)?#undef)\\s+([A-Z_][A-Z0-9_]+)\\s*(//.*)?$'), - defPatt = new RegExp('^(\\s*(//)?#define)\\s+([A-Z_][A-Z0-9_]+)\\s+([-_\\w]+)\\s*(//.*)?$'), - condPatt = new RegExp('^(\\s*(//)?#(if|ifn?def|elif)(\\s+\\S+)*)\\s+(//.*)$'), - commPatt = new RegExp('^\\s{20,}(//.*)?$'); - const col_value_lj = col_comment - patt.pad - 2; - var r, out = '', check_comment_next = false; - txt.split('\n').forEach((line) => { - if (check_comment_next) - check_comment_next = ((r = commPatt.exec(line)) !== null); - - if (check_comment_next) - // Comments in column 45 - line = ''.rpad(col_comment) + r[1]; - - else if (skipPatt1.exec(line) !== null) { - // - // #define SKIP_ME - // - logmsg("skip:", line); - } - else if ((r = pindefPatt.exec(line)) !== null) { - // - // #define MY_PIN [pin] - // - logmsg("pin:", line); - const pinnum = r[4].charAt(0) == 'P' ? r[4] : r[4].lpad(patt.pad); - line = r[1] + ' ' + r[3]; - line = line.rpad(col_value_lj).concat_with_space(pinnum); - if (r[5]) line = line.rpad(col_comment).concat_with_space(r[5]); - } - else if ((r = noPinPatt.exec(line)) !== null) { - // - // #define MY_PIN -1 - // - logmsg("pin -1:", line); - line = r[1] + ' ' + r[3]; - line = line.rpad(col_value_lj).concat_with_space('-1'); - if (r[5]) line = line.rpad(col_comment).concat_with_space(r[5]); - } - else if (skipPatt2.exec(line) !== null || skipPatt3.exec(line) !== null) { - // - // #define SKIP_ME - // #else, #endif - // - logmsg("skip:", line); - } - else if ((r = aliasPatt.exec(line)) !== null) { - // - // #define ALIAS OTHER - // - logmsg("alias:", line); - line = r[1] + ' ' + r[3]; - line = line.concat_with_space(r[4].lpad(col_value_rj + 1 - line.length)); - if (r[5]) line = line.rpad(col_comment).concat_with_space(r[5]); - } - else if ((r = switchPatt.exec(line)) !== null) { - // - // #define SWITCH - // - logmsg("switch:", line); - line = r[1] + ' ' + r[3]; - if (r[4]) line = line.rpad(col_comment).concat_with_space(r[4]); - check_comment_next = true; - } - else if ((r = defPatt.exec(line)) !== null) { - // - // #define ... - // - logmsg("def:", line); - line = r[1] + ' ' + r[3] + ' '; - line = line.concat_with_space(r[4].lpad(col_value_rj + 1 - line.length)); - if (r[5]) line = line.rpad(col_comment - 1) + ' ' + r[5]; - } - else if ((r = undefPatt.exec(line)) !== null) { - // - // #undef ... - // - logmsg("undef:", line); - line = r[1] + ' ' + r[3]; - if (r[4]) line = line.rpad(col_comment).concat_with_space(r[4]); - } - else if ((r = condPatt.exec(line)) !== null) { - // - // #if, #ifdef, #ifndef, #elif ... - // - logmsg("cond:", line); - line = r[1].rpad(col_comment).concat_with_space(r[5]); - check_comment_next = true; - } - out += line + '\n'; - }); - return out.replace(/\n\n+/g, '\n\n').replace(/\n\n$/g, '\n'); -} diff --git a/buildroot/share/scripts/pinsformat.py b/buildroot/share/scripts/pinsformat.py index b49ae4931d0..e4bd69d855f 100755 --- a/buildroot/share/scripts/pinsformat.py +++ b/buildroot/share/scripts/pinsformat.py @@ -27,6 +27,13 @@ def rpad(astr, fill, c=' '): need = fill - len(astr) return astr if need <= 0 else astr + (need * c) +# Concatenate a string, adding a space if necessary +# to avoid merging two words +def concat_with_space(s1, s2): + if not s1.endswith(' ') and not s2.startswith(' '): + s1 += ' ' + return s1 + s2 + # Pin patterns mpatt = [ r'-?\d{1,3}', r'P[A-I]\d+', r'P\d_\d+', r'Pin[A-Z]\d\b' ] mstr = '|'.join(mpatt) @@ -45,6 +52,7 @@ def format_pins(argv): scnt = 0 for arg in argv: if arg == '-v': + global do_log do_log = True elif scnt == 0: # Get a source file if specified. Default destination is the same file @@ -135,7 +143,7 @@ def process_text(txt): logmsg("pin:", line) pinnum = r[4] if r[4][0] == 'P' else lpad(r[4], patt['pad']) line = f'{r[1]} {r[3]}' - line = rpad(line, col_value_lj) + pinnum + line = concat_with_space(rpad(line, col_value_lj), pinnum) if r[5]: line = rpad(line, col_comment) + r[5] d['line'] = line return True @@ -149,7 +157,7 @@ def process_text(txt): if r == None: return False logmsg("pin -1:", line) line = f'{r[1]} {r[3]}' - line = rpad(line, col_value_lj) + '-1' + line = concat_with_space(rpad(line, col_value_lj), '-1') if r[5]: line = rpad(line, col_comment) + r[5] d['line'] = line return True @@ -179,8 +187,8 @@ def process_text(txt): if r == None: return False logmsg("alias:", line) line = f'{r[1]} {r[3]}' - line += lpad(r[4], col_value_rj + 1 - len(line)) - if r[5]: line = rpad(line, col_comment) + r[5] + line = concat_with_space(line, lpad(r[4], col_value_rj + 1 - len(line))) + if r[5]: line = concat_with_space(rpad(line, col_comment), r[5]) d['line'] = line return True @@ -193,7 +201,7 @@ def process_text(txt): if r == None: return False logmsg("switch:", line) line = f'{r[1]} {r[3]}' - if r[4]: line = rpad(line, col_comment) + r[4] + if r[4]: line = concat_with_space(rpad(line, col_comment), r[4]) d['line'] = line d['check_comment_next'] = True return True @@ -207,7 +215,7 @@ def process_text(txt): if r == None: return False logmsg("def:", line) line = f'{r[1]} {r[3]} ' - line += lpad(r[4], col_value_rj + 1 - len(line)) + line = concat_with_space(line, lpad(r[4], col_value_rj + 1 - len(line))) if r[5]: line = rpad(line, col_comment - 1) + ' ' + r[5] d['line'] = line return True @@ -221,7 +229,7 @@ def process_text(txt): if r == None: return False logmsg("undef:", line) line = f'{r[1]} {r[3]}' - if r[4]: line = rpad(line, col_comment) + r[4] + if r[4]: line = concat_with_space(rpad(line, col_comment), r[4]) d['line'] = line return True @@ -233,7 +241,7 @@ def process_text(txt): r = condPatt.match(line) if r == None: return False logmsg("cond:", line) - line = rpad(r[1], col_comment) + r[5] + line = concat_with_space(rpad(r[1], col_comment), r[5]) d['line'] = line d['check_comment_next'] = True return True