From d726b262874c41a1fd11653422791835d793e07c Mon Sep 17 00:00:00 2001 From: alex Date: Tue, 24 Aug 2021 14:30:35 +0100 Subject: [PATCH] Enforce warnings count in pull requests (#380) * add warnings_count Stolen from https://github.com/zeldaret/mm. Co-authored-by: Anghelo Carvajal * emit only new warnings * add jp warnings * fix ccache (lmao) * slug comments about warnings * oops * oops again * oops again again * adjust message * truncate warnings list if there are more than 100 Co-authored-by: Anghelo Carvajal --- .github/workflows/{coverage.yaml => pr.yaml} | 6 +- Jenkinsfile | 10 ++- tools/build/configure.py | 4 +- tools/warnings_count/.gitignore | 1 + tools/warnings_count/check_new_warnings.sh | 11 ++++ tools/warnings_count/compare_warnings.py | 61 +++++++++++++++++++ .../warnings_count/update_current_warnings.sh | 7 +++ 7 files changed, 93 insertions(+), 7 deletions(-) rename .github/workflows/{coverage.yaml => pr.yaml} (77%) create mode 100644 tools/warnings_count/.gitignore create mode 100755 tools/warnings_count/check_new_warnings.sh create mode 100755 tools/warnings_count/compare_warnings.py create mode 100755 tools/warnings_count/update_current_warnings.sh diff --git a/.github/workflows/coverage.yaml b/.github/workflows/pr.yaml similarity index 77% rename from .github/workflows/coverage.yaml rename to .github/workflows/pr.yaml index 9d7af8b980..ccc926718a 100644 --- a/.github/workflows/coverage.yaml +++ b/.github/workflows/pr.yaml @@ -1,8 +1,8 @@ -name: Coverage -on: [push, pull_request] +name: PR +on: [pull_request] jobs: - build: + delete-matched-asm: name: Check matched assembly files are deleted runs-on: ubuntu-latest steps: diff --git a/Jenkinsfile b/Jenkinsfile index a6bb024017..1c391301cf 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -29,6 +29,7 @@ pipeline { if (env.CHANGE_ID) { def us_progress = sh(returnStdout: true, script: "python3 progress.py us --pr-comment").trim() def jp_progress = sh(returnStdout: true, script: "python3 progress.py jp --pr-comment").trim() + def warnings = sh(returnStdout: true, script: "./tools/warnings_count/check_new_warnings.sh --pr-message").trim() def comment_id = -1 for (comment in pullRequest.comments) { @@ -37,9 +38,9 @@ pipeline { } } - def message = "${us_progress}\n${jp_progress}" + def message = "${us_progress}\n${jp_progress}\n${warnings}" - if (message != "\n") { + if (message != "\n\n") { if (comment_id == -1) { pullRequest.comment(message) } else { @@ -63,6 +64,9 @@ pipeline { sh 'python3 progress.py jp --csv >> reports/progress_jp.csv' sh 'python3 progress.py jp --shield-json > reports/progress_jp_shield.json' + sh './tools/warnings_count/update_current_warnings.sh' + sh 'cp tools/warnings_count/warnings.txt reports/warnings.txt' + stash includes: 'reports/*', name: 'reports' } } @@ -80,6 +84,8 @@ pipeline { sh 'cat reports/progress_jp.csv >> /var/www/papermar.io/html/reports/progress_jp.csv' sh 'cat reports/progress_jp_shield.json > /var/www/papermar.io/html/reports/progress_jp_shield.json' + + sh 'cat reports/warnings.txt > /var/www/papermar.io/html/reports/warnings.txt' } } } diff --git a/tools/build/configure.py b/tools/build/configure.py index 6a349a787b..41d7541609 100755 --- a/tools/build/configure.py +++ b/tools/build/configure.py @@ -41,7 +41,7 @@ def write_ninja_rules(ninja: ninja_syntax.Writer, cpp: str, cppflags: str, extra else: raise Exception(f"unsupported platform {sys.platform}") - ccache = "" + ccache = "ccache " try: subprocess.call(["ccache"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) @@ -56,7 +56,7 @@ def write_ninja_rules(ninja: ninja_syntax.Writer, cpp: str, cppflags: str, extra CPPFLAGS = "-w -Iver/$version/build/include -Iinclude -Isrc -Iassets/$version -D_LANGUAGE_C -D_FINALROM -DVERSION=$version " \ "-ffreestanding -DF3DEX_GBI_2 -D_MIPS_SZLONG=32" - cflags = f"-c -G0 -O2 -fno-common -Wuninitialized -Wmissing-braces -B {BUILD_TOOLS}/cc/gcc/ {extra_cflags}" + cflags = f"-c -G0 -O2 -fno-common -Wuninitialized -Wmissing-braces -Wimplicit -Wredundant-decls -Wstrict-prototypes -B {BUILD_TOOLS}/cc/gcc/ {extra_cflags}" ninja.variable("python", sys.executable) diff --git a/tools/warnings_count/.gitignore b/tools/warnings_count/.gitignore new file mode 100644 index 0000000000..2211df63dd --- /dev/null +++ b/tools/warnings_count/.gitignore @@ -0,0 +1 @@ +*.txt diff --git a/tools/warnings_count/check_new_warnings.sh b/tools/warnings_count/check_new_warnings.sh new file mode 100755 index 0000000000..6d2c6c103e --- /dev/null +++ b/tools/warnings_count/check_new_warnings.sh @@ -0,0 +1,11 @@ +#!/bin/bash +set -e + +# This script can be used when you want to test locally the amount of warnings produced by your changes before doing a PR. + +curl -L https://papermar.io/reports/warnings.txt > tools/warnings_count/warnings.txt + +rm -rf ver/*/build/src +ninja | grep warning | sort | uniq > tools/warnings_count/warnings_new.txt + +python3 tools/warnings_count/compare_warnings.py tools/warnings_count/warnings.txt tools/warnings_count/warnings_new.txt $@ diff --git a/tools/warnings_count/compare_warnings.py b/tools/warnings_count/compare_warnings.py new file mode 100755 index 0000000000..789f1133ea --- /dev/null +++ b/tools/warnings_count/compare_warnings.py @@ -0,0 +1,61 @@ +#!/usr/bin/env python3 + +import argparse +import sys + + +def countFileLines(filename: str) -> int: + with open(filename) as f: + return len(f.readlines()) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('currentwarnings', help="Name of file which contains the current warnings of the repo.") + parser.add_argument('newwarnings', help="Name of file which contains the *new* warnings of the repo.") + parser.add_argument("--pr-message", action="store_true") + args = parser.parse_args() + + currentLines = countFileLines(args.currentwarnings) + newLines = countFileLines(args.newwarnings) + if newLines > currentLines: + stderr = False + if args.pr_message: + delta = newLines - currentLines + + if delta == 1: + print(f"⚠️ This PR introduces a warning:") + else: + print(f"⚠️ This PR introduces {delta} warnings:") + + if delta > 100: + stderr = True + print("See log for details.") + else: + print() + print("There are more warnings now. Go fix them!") + print("\tCurrent warnings: " + str(currentLines)) + print("\tNew warnings: " + str(newLines)) + print() + + with open(args.newwarnings) as new: + new = new.readlines() + with open(args.currentwarnings) as current: + current = current.readlines() + for newLine in new: + if newLine not in current: + if stderr: + print(newLine.strip(), file=sys.stderr) + else: + print("- " + newLine.strip()) + elif newLines < currentLines: + delta = currentLines - newLines + + if args.pr_message: + print(f"✅ This PR fixes {delta} warnings!") + else: + print(f"{delta} warnings fixed.") + + +if __name__ == "__main__": + main() diff --git a/tools/warnings_count/update_current_warnings.sh b/tools/warnings_count/update_current_warnings.sh new file mode 100755 index 0000000000..826fc078fa --- /dev/null +++ b/tools/warnings_count/update_current_warnings.sh @@ -0,0 +1,7 @@ +#!/bin/bash +set -e + +# This script should only be used when we need to modify the accepted amount of warnings. + +rm -rf ver/*/build/src +ninja | grep warning | sort | uniq > tools/warnings_count/warnings.txt