From 8c9b754b3f8cb7830627840315c319c71587d889 Mon Sep 17 00:00:00 2001 From: VintagePC <53943260+vintagepc@users.noreply.github.com> Date: Sat, 16 Sep 2023 20:40:28 -0400 Subject: [PATCH 1/6] Add actions workflow for firmware build --- .github/workflows/build.yml | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 000000000..607434871 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,58 @@ +name: ci-build + +on: + - push + - pull_request + +jobs: + build: + runs-on: ubuntu-latest + steps: + + # setup base required dependencies + - name: Setup dependencies + run: | + sudo apt-get install cmake ninja-build python3-pyelftools python3-regex python3-polib + + # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it + - name: Checkout ${{ github.event.pull_request.head.ref }} + uses: actions/checkout@v3 + if: ${{ github.event.pull_request }} + with: + ref: ${{ github.event.pull_request.head.sha }} + submodules: true + + - name: Checkout ${{ github.event.ref }} + uses: actions/checkout@v3 + if: ${{ !github.event.pull_request }} + with: + ref: ${{ github.event.ref }} + submodules: true + + - name: Cache Dependencies + uses: actions/cache@v3.0.11 + id: cache-pkgs + with: + path: ".dependencies" + key: "build-deps-1_0_0-linux" + + - name: Setup build dependencies + run: | + ./utils/bootstrap.py + + - name: Cache permissions + run: sudo chmod -R 744 .dependencies + + - name: Build + run: | + mkdir build + cd build + cmake .. -DCMAKE_TOOLCHAIN_FILE="../cmake/AvrGcc.cmake" -DCMAKE_BUILD_TYPE=Release -G Ninja + ninja + + - name: Upload artifacts + if: ${{ !github.event.pull_request }} + uses: actions/upload-artifact@v3.1.1 + with: + name: Firmware + path: build/*.hex From 5ddac5cb6c725972bdf2f6861f4999ef13fe1193 Mon Sep 17 00:00:00 2001 From: VintagePC <53943260+vintagepc@users.noreply.github.com> Date: Wed, 20 Sep 2023 19:05:46 -0400 Subject: [PATCH 2/6] - Make size share cache - Add tests step - Add lang-check step --- .github/workflows/build.yml | 97 ++++++++++++++++++++++++++++++++++- .github/workflows/pr-size.yml | 10 ++++ tests/CMakeLists.txt | 24 +++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 607434871..7aa91ab94 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,8 +1,11 @@ name: ci-build on: - - push - - pull_request + pull_request: + branches: + - '*' + push: + branches: [ MK3, MK3_* ] jobs: build: @@ -56,3 +59,93 @@ jobs: with: name: Firmware path: build/*.hex + + check-lang: + runs-on: ubuntu-latest + + steps: + # setup base required dependencies + - name: Setup dependencies + run: | + sudo apt-get install gcc-11 g++11 lcov cmake ninja-build python3-pyelftools python3-regex python3-polib + + # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it + - name: Checkout ${{ github.event.pull_request.head.ref }} + uses: actions/checkout@v3 + if: ${{ github.event.pull_request }} + with: + ref: ${{ github.event.pull_request.head.sha }} + submodules: true + + - name: Checkout ${{ github.event.ref }} + uses: actions/checkout@v3 + if: ${{ !github.event.pull_request }} + with: + ref: ${{ github.event.ref }} + submodules: true + + - name: Cache Dependencies + uses: actions/cache@v3.0.11 + id: cache-pkgs + with: + path: ".dependencies" + key: "build-deps-1_0_0-linux" + + - name: Setup build dependencies + run: | + ./utils/bootstrap.py + + - name: Cache permissions + run: sudo chmod -R 744 .dependencies + + - name: Run check + run: | + mkdir build + cd build + cmake .. -G Ninja + ninja check_lang + + tests: + runs-on: ubuntu-latest + + steps: + # setup base required dependencies + - name: Setup dependencies + run: | + sudo apt-get install gcc-11 g++11 lcov cmake ninja-build python3-pyelftools python3-regex python3-polib + + # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it + - name: Checkout ${{ github.event.pull_request.head.ref }} + uses: actions/checkout@v3 + if: ${{ github.event.pull_request }} + with: + ref: ${{ github.event.pull_request.head.sha }} + submodules: true + + - name: Checkout ${{ github.event.ref }} + uses: actions/checkout@v3 + if: ${{ !github.event.pull_request }} + with: + ref: ${{ github.event.ref }} + submodules: true + + - name: Cache Dependencies + uses: actions/cache@v3.0.11 + id: cache-pkgs + with: + path: ".dependencies" + key: "build-deps-1_0_0-linux" + + - name: Setup build dependencies + run: | + ./utils/bootstrap.py + + - name: Cache permissions + run: sudo chmod -R 744 .dependencies + + - name: Run check + run: | + mkdir build + cd build + cmake .. -G Ninja + ninja test_run_all diff --git a/.github/workflows/pr-size.yml b/.github/workflows/pr-size.yml index 2c68acd88..891336456 100644 --- a/.github/workflows/pr-size.yml +++ b/.github/workflows/pr-size.yml @@ -24,10 +24,20 @@ jobs: - name: Checkout base uses: actions/checkout@v3 + - name: Cache Dependencies + uses: actions/cache@v3.0.11 + id: cache-pkgs + with: + path: ".dependencies" + key: "build-deps-1_0_0-linux" + - name: Setup build dependencies run: | ./utils/bootstrap.py + - name: Cache permissions + run: sudo chmod -R 744 .dependencies + - name: Build base run: | rm -rf build-base diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7bab4fea2..b20179864 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -16,3 +16,27 @@ add_executable(tests ${TEST_SOURCES}) target_include_directories(tests PRIVATE tests) target_link_libraries(tests Catch2::Catch2WithMain) catch_discover_tests(tests) + +set(ctest_test_args --output-on-failure) + +include(ProcessorCount) +ProcessorCount(N) +if(N EQUAL 0) + message( + WARNING "CTest: There was an issue reading the core count, tests won't be run in parallel" + ) +else() + message(STATUS "CTest: Detected ${N} CPU threads") + set(ctest_test_args -j${N} ${ctest_test_args}) +endif() + +# This step needs to always return OK but log whether it was successful or not. The thought here +# is that if the tests all pass, .ctest-finished is created and we can check for its existance +# after generating the report to determine if the overall build result is a pass or fail. +add_custom_target( + test_run_all + COMMAND ${CMAKE_CTEST_COMMAND} ${ctest_test_args} + COMMAND ${CMAKE_COMMAND} -E touch .ctest-finished || exit 0 + BYPRODUCTS ${PROJECT_BINARY_DIR}/.ctest-finished + WORKING_DIRECTORY "${PROJECT_BINARY_DIR}" + ) From 6ceb750999532ae1f95e0b3b825f0b97183d67cb Mon Sep 17 00:00:00 2001 From: VintagePC <53943260+vintagepc@users.noreply.github.com> Date: Wed, 20 Sep 2023 19:13:12 -0400 Subject: [PATCH 3/6] Fix missing toolchain --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7aa91ab94..2ffb1b759 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -102,7 +102,7 @@ jobs: run: | mkdir build cd build - cmake .. -G Ninja + cmake .. -G Ninja -DCMAKE_TOOLCHAIN_FILE="../cmake/AvrGcc.cmake" -DCMAKE_BUILD_TYPE=Release -G Ninja ninja check_lang tests: From f7fbdadae72c1bf3f24329b96e2e588835e6698a Mon Sep 17 00:00:00 2001 From: VintagePC <53943260+vintagepc@users.noreply.github.com> Date: Thu, 21 Sep 2023 18:44:43 -0400 Subject: [PATCH 4/6] Test annotations for language checks --- lang/lang-check.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lang/lang-check.py b/lang/lang-check.py index 618b6b03d..ffdfd1c03 100755 --- a/lang/lang-check.py +++ b/lang/lang-check.py @@ -40,10 +40,19 @@ import os from lib import charset as cs from lib.io import load_map +from enum import Enum COLORIZE = (stdout.isatty() and os.getenv("TERM", "dumb") != "dumb") or os.getenv('NO_COLOR') == "0" LCD_WIDTH = 20 +GH_ANNOTATIONS = os.getenv('GH_ANNOTATIONS') == "1" +CURRENT_PO="Unknown file" + +class AN_TYPE(Enum): + ERROR = "error" + WARNING = "warning" + NOTICE = "notice" + def color_maybe(color_attr, text): if COLORIZE: return '\033[0;' + str(color_attr) + 'm' + text + '\033[0m' @@ -119,6 +128,17 @@ def ign_char_first(c): def ign_char_last(c): return c.isalnum() or c in {'.', "'"} +def gh_annotate(type, start_line, message, end_line = None ): + if not GH_ANNOTATIONS: + return + if end_line is not None: + line_info = "line={},endLine={}".format(start_line,end_line) + else: + line_info = "line={}".format(start_line) + + print("::{} file={},{}::{}".format(type.value, CURRENT_PO, line_info, message)) + + def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, warn_same, information, shorter): """Check strings to display definition.""" @@ -220,6 +240,7 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, print_ruler(6, cols); print_wrapped(wrapped_source, rows, cols) print() + gh_annotate(AN_TYPE.WARNING, line, "Empty translation", line + rows ) # Check for translation length too long if (rows_count_translation > rows) or (rows == 1 and len(translation) > cols): @@ -350,6 +371,8 @@ def main(): # check each translation in turn status = True for translation in polib.pofile(args.po): + global CURRENT_PO + CURRENT_PO=args.po status &= check_translation(translation, msgids, args.pot, args.no_warning, args.no_suggest, args.warn_empty, args.warn_same, args.information, args.shorter) return 0 if status else 1 From 605cc5d83a8d714bc3f11bc14090c3ca756ef8f4 Mon Sep 17 00:00:00 2001 From: VintagePC <53943260+vintagepc@users.noreply.github.com> Date: Thu, 21 Sep 2023 18:46:43 -0400 Subject: [PATCH 5/6] Actually enable annotations via ENV --- .github/workflows/build.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2ffb1b759..0f793558d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,6 +7,9 @@ on: push: branches: [ MK3, MK3_* ] +env: + GH_ANNOTATIONS: 1 + jobs: build: runs-on: ubuntu-latest From 2a2932e66deede65e8a5ba70cd92433bb2a1564a Mon Sep 17 00:00:00 2001 From: VintagePC <53943260+vintagepc@users.noreply.github.com> Date: Thu, 21 Sep 2023 19:32:32 -0400 Subject: [PATCH 6/6] Reduce message duplication --- lang/lang-check.py | 85 +++++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 27 deletions(-) diff --git a/lang/lang-check.py b/lang/lang-check.py index ffdfd1c03..1c651b692 100755 --- a/lang/lang-check.py +++ b/lang/lang-check.py @@ -40,18 +40,29 @@ import os from lib import charset as cs from lib.io import load_map -from enum import Enum +import enum COLORIZE = (stdout.isatty() and os.getenv("TERM", "dumb") != "dumb") or os.getenv('NO_COLOR') == "0" LCD_WIDTH = 20 GH_ANNOTATIONS = os.getenv('GH_ANNOTATIONS') == "1" -CURRENT_PO="Unknown file" +CURRENT_PO = "Unknown file" +GH_ERR_COUNT = 0 -class AN_TYPE(Enum): - ERROR = "error" - WARNING = "warning" - NOTICE = "notice" +class AN_TYPE(enum.Enum): + + def __new__(cls, *args, **kwds): + value = len(cls.__members__) + 1 + obj = object.__new__(cls) + obj._value_ = value + return obj + def __init__(self, a, b): + self.prefix = a + self.print_fmt = b + + ERROR = "error", "[E]" + WARNING = "warning", "[W]" + NOTICE = "notice", "[S]" def color_maybe(color_attr, text): if COLORIZE: @@ -128,15 +139,31 @@ def ign_char_first(c): def ign_char_last(c): return c.isalnum() or c in {'.', "'"} -def gh_annotate(type, start_line, message, end_line = None ): +# Print_anyway is used to reduce code copypasta. +# specifically, if we have all the info here to construct the "normal" message as well, it's done here + +def gh_annotate(an_type, start_line, message, end_line = None, print_anyway = False): if not GH_ANNOTATIONS: + if print_anyway: + if end_line is not None: + line_text = "lines {}-{}".format(start_line, end_line) + else: + line_text = "line {}".format(start_line) + message_simple = "{} on {}".format(message, line_text) + if an_type == AN_TYPE.ERROR: + print(red("{}: {}".format(an_type.print_fmt, message_simple))) + else: + print(yellow("{}: {}".format(an_type.print_fmt, message_simple))) return if end_line is not None: line_info = "line={},endLine={}".format(start_line,end_line) else: line_info = "line={}".format(start_line) - print("::{} file={},{}::{}".format(type.value, CURRENT_PO, line_info, message)) + print("::{} file={},{}::{}".format(an_type.prefix, CURRENT_PO, line_info, message)) + if an_type == AN_TYPE.ERROR: + global GH_ERR_COUNT + GH_ERR_COUNT += 1 def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, warn_same, information, shorter): @@ -157,10 +184,10 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, # Check comment syntax (non-empty and include a MSG id) if known_msgid or warn_empty: if len(meta) == 0: - print(red("[E]: Translation doesn't contain any comment metadata on line %d" % line)) + gh_annotate(AN_TYPE.ERROR, line, "Translation missing comment metadata", None, True) return False if not meta.startswith('MSG'): - print(red("[E]: Critical syntax error: comment doesn't start with MSG on line %d" % line)) + gh_annotate(AN_TYPE.ERROR, line, "Critical Syntax Error: comment doesn't start with MSG", None, True) print(red(" comment: " + meta)) return False @@ -178,29 +205,29 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, else: raise ValueError except ValueError: - print(red("[E]: Invalid display definition on line %d" % line)) + gh_annotate(AN_TYPE.ERROR, line, "Invalid display definition", None, True) print(red(" definition: " + meta)) return False if not cols: if not no_warning and known_msgid and not rows: errors += 1 - print(yellow("[W]: No usable display definition on line %d" % line)) + gh_annotate(AN_TYPE.WARNING, line, "No usable display definition", None, True) # probably fullscreen, guess from the message length to continue checking cols = LCD_WIDTH if cols > LCD_WIDTH: errors += 1 - print(yellow("[W]: Invalid column count on line %d" % line)) + gh_annotate(AN_TYPE.WARNING, line, "Invalid column count", None, True) if not rows: rows = 1 elif rows > 1 and cols != LCD_WIDTH: errors += 1 - print(yellow("[W]: Multiple rows with odd number of columns on line %d" % line)) + gh_annotate(AN_TYPE.WARNING, line, "Multiple rows with odd number of columns", None, True) # Check if translation contains unsupported characters invalid_char = cs.translation_check(cs.unicode_to_source(translation)) if invalid_char is not None: - print(red('[E]: Critical syntax: Unhandled char %s found on line %d' % (repr(invalid_char), line))) + gh_annotate(AN_TYPE.ERROR, line, "Critical syntax: Unhandled char %s found".format(repr(invalid_char)), None, True ) print(red(' translation: ' + translation)) return False @@ -215,13 +242,13 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, # Incorrect number of rows/cols on the definition if rows == 1 and (len(source) > cols or rows_count_source > rows): errors += 1 - print(yellow('[W]: Source text longer than %d cols as defined on line %d:' % (cols, line))) + gh_annotate(AN_TYPE.WARNING, line, "Source text longer than %d cols as defined".format(cols), None, True) print_ruler(4, cols); print_truncated(source, cols) print() elif rows_count_source > rows: errors += 1 - print(yellow('[W]: Wrapped source text longer than %d rows as defined on line %d:' % (rows, line))) + gh_annotate(AN_TYPE.WARNING, line, "Source text longer than %d rows as defined".format(rows), None, True) print_ruler(6, cols); print_wrapped(wrapped_source, rows, cols) print() @@ -234,17 +261,17 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, if len(translation) == 0 and (warn_empty or (not no_warning and known_msgid)): errors += 1 if rows == 1: - print(yellow("[W]: Empty translation for \"%s\" on line %d" % (source, line))) + gh_annotate(AN_TYPE.WARNING, line, "Empty translation for \"{}\"".format(source), line + rows, True ) else: - print(yellow("[W]: Empty translation on line %d" % line)) + gh_annotate(AN_TYPE.WARNING, line, "Empty translation", line + rows, True ) print_ruler(6, cols); print_wrapped(wrapped_source, rows, cols) print() - gh_annotate(AN_TYPE.WARNING, line, "Empty translation", line + rows ) # Check for translation length too long if (rows_count_translation > rows) or (rows == 1 and len(translation) > cols): errors += 1 + gh_annotate(AN_TYPE.ERROR, line, "Text is longer than definition", line + rows) print(red('[E]: Text is longer than definition on line %d: cols=%d rows=%d (rows diff=%d)' % (line, cols, rows, rows_count_translation-rows))) print_source_translation(source, translation, @@ -253,6 +280,7 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, # Check for translation length shorter if shorter and (rows_count_translation < rows-1): + gh_annotate(AN_TYPE.NOTICE, line, "Text is shorter than definition", line + rows) print(yellow('[S]: Text is shorter than definition on line %d: cols=%d rows=%d (rows diff=%d)' % (line, cols, rows, rows_count_translation-rows))) print_source_translation(source, translation, @@ -262,7 +290,7 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, # Different count of % sequences if source.count('%') != translation.count('%') and len(translation) > 0: errors += 1 - print(red('[E]: Unequal count of %% escapes on line %d:' % (line))) + gh_annotate(AN_TYPE.ERROR, line, "Unequal count of %% escapes", None, True) print_source_translation(source, translation, wrapped_source, wrapped_translation, rows, cols) @@ -275,14 +303,14 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, end_diff = not (ign_char_last(source_end) and ign_char_last(translation_end)) and source_end != translation_end if start_diff or end_diff: if start_diff: - print(yellow('[S]: Differing first punctuation character (%s => %s) on line %d:' % (source[0], translation[0], line))) + gh_annotate(AN_TYPE.NOTICE, line, "Differing first punctuation character: ({} => {})".format(source[0],translation[0]), None, True) if end_diff: - print(yellow('[S]: Differing last punctuation character (%s => %s) on line %d:' % (source[-1], translation[-1], line))) + gh_annotate(AN_TYPE.NOTICE, line, "Differing last punctuation character: ({} => {})".format(source[-1],translation[-1]), None, True) print_source_translation(source, translation, wrapped_source, wrapped_translation, rows, cols) if not no_suggest and source == translation and (warn_same or len(source.split(' ', 1)) > 1): - print(yellow('[S]: Translation same as original on line %d:' %line)) + gh_annotate(AN_TYPE.NOTICE, line, "Translation same as original text", None, True) print_source_translation(source, translation, wrapped_source, wrapped_translation, rows, cols) @@ -290,7 +318,7 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, # Short translation if not no_suggest and len(source) > 0 and len(translation) > 0: if len(translation.rstrip()) < len(source.rstrip()) / 2: - print(yellow('[S]: Short translation on line %d:' % (line))) + gh_annotate(AN_TYPE.NOTICE, line, "Short translation", None, True) print_source_translation(source, translation, wrapped_source, wrapped_translation, rows, cols) @@ -301,7 +329,7 @@ def check_translation(entry, msgids, is_pot, no_warning, no_suggest, warn_empty, translation.rstrip() != translation and \ (rows > 1 or len(translation) != len(source)): errors += 1 - print(yellow('[W]: Incorrect trailing whitespace for translation on line %d:' % (line))) + gh_annotate(AN_TYPE.WARNING, line, "Incorrect trailing whitespace for translation", None, True) source = highlight_trailing_white(source) translation = highlight_trailing_white(translation) wrapped_translation = highlight_trailing_white(wrapped_translation) @@ -375,7 +403,10 @@ def main(): CURRENT_PO=args.po status &= check_translation(translation, msgids, args.pot, args.no_warning, args.no_suggest, args.warn_empty, args.warn_same, args.information, args.shorter) - return 0 if status else 1 + if GH_ANNOTATIONS: + return GH_ERR_COUNT > 0 # Do not cause a failure if only warnings or notices. + else: + return 0 if status else 1 if __name__ == "__main__": exit(main())