Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
1012f74
initial lint gh actions
DaAwesomeP Feb 10, 2023
836567f
gh actions lint attempt cpplint fix/troubleshoot
DaAwesomeP Feb 17, 2023
337a4b4
cpplint fix indents for goto targets in protoc/StrUtil.cpp
DaAwesomeP Feb 17, 2023
ba7d9e3
change wording in count_generic_nolints.sh (@peternewman)
DaAwesomeP Feb 17, 2023
bcff274
lint switch to GITHUB_ACTIONS env var
DaAwesomeP Feb 17, 2023
2fd1d98
add note from where to install cpplint
DaAwesomeP Feb 17, 2023
de4b59c
speed up gh actions lint artifacts
DaAwesomeP Feb 17, 2023
9008033
speed up gh actions lint artifacts switch from xz to gz
DaAwesomeP Feb 17, 2023
db6d295
attempt gh actions lint problem matcher for check-licences, intention…
DaAwesomeP Feb 17, 2023
47a6dee
gh actions lint fix path to check-licences problem matcher
DaAwesomeP Feb 17, 2023
1c8eef2
attempt fix check-licence problem matcher, fix flake8 for check-licence
DaAwesomeP Feb 17, 2023
f007ee5
check-licence problem matcher works, revert force-fail files
DaAwesomeP Feb 17, 2023
1e94a76
attempt gh actions lint problem matcher for generic-nolints, intentio…
DaAwesomeP Feb 17, 2023
bf337f6
generic-nolints problem matcher works, revert force-fail files
DaAwesomeP Feb 17, 2023
5242ccc
attempt gh actions lint problem matcher for cpplint, intentionally fa…
DaAwesomeP Feb 17, 2023
fd60a4b
gh actions cpplint problem matcher again with debug
DaAwesomeP Feb 17, 2023
18e11cf
gh actions cpplint problem matcher again with debug 2
DaAwesomeP Feb 18, 2023
2a4978c
fix cpplint and flake8 problem matcher annotations file matching
DaAwesomeP Feb 18, 2023
d090920
gh actions lint cpplint matcher show error code in message text, remo…
DaAwesomeP Feb 18, 2023
a9d0f62
gh actions cpplint problem matcher annotations work with file matchin…
DaAwesomeP Feb 18, 2023
d06f590
gh actions lint problem matcher change info to notice to match proble…
DaAwesomeP Feb 18, 2023
6be6c78
gh actions lint licences problem matcher does not find dirs, problem …
DaAwesomeP Feb 18, 2023
573a83e
remove old flake8 gh actions workflow that does not build first
DaAwesomeP Feb 18, 2023
cbe7dc4
add gh actions note to AUTHORS
DaAwesomeP Feb 18, 2023
652603f
revert license check to be non-specific to python3
DaAwesomeP Feb 20, 2023
05ab75a
add back Python 2 flake8 checking
DaAwesomeP Feb 20, 2023
4b4b6b6
all lint tasks depend on build
DaAwesomeP Feb 20, 2023
718c89f
gh actions lint fix 80 character line limit comment
DaAwesomeP Feb 20, 2023
4ee032a
gh actions lint fix python for check licences
DaAwesomeP Feb 20, 2023
b639fef
gh actions lint check-licences use python-is-python3
DaAwesomeP Feb 20, 2023
9ceee09
gh actions lint remove cpplint repository flag
DaAwesomeP Feb 20, 2023
02252de
specify which cpplint in error
DaAwesomeP Feb 20, 2023
f28976a
fix english comma errors for check licences
DaAwesomeP Feb 20, 2023
0ea3b73
licence check remove relative dir ./ style
DaAwesomeP Feb 20, 2023
c1594fb
gh actions lint add touch before tar note
DaAwesomeP Feb 20, 2023
5434a7a
gh actions remove additional python3 flake8
DaAwesomeP Feb 20, 2023
91c7988
gh actions lint enable dir listing only for debug logging
DaAwesomeP Feb 20, 2023
5428e8f
gh actions lint build job step clarify which build make task
DaAwesomeP Feb 20, 2023
cee6919
Merge remote-tracking branch 'upstream/0.10' into DaAwesomeP-GitHubAc…
DaAwesomeP Feb 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/problem-matcher-lint-check-licences.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"problemMatcher": [
{
"owner": "lint-check-licences",
"pattern": [
{
"regexp": "^(notice|error)(:(file|dir):([^:]+)(:lines? (\\d+)(-(\\d+))?)?)?: (.+)$",
"severity": 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think GitHub recognises notice as a severity, just error and warning.
https://github.com/actions/toolkit/blob/master/docs/problem-matchers.md#single-line-matchers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does! I found it in the source. It causes nice messages like these: https://github.com/DaAwesomeP/ola/actions/runs/4218995033

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ 🤦
Perhaps you'd care to open a PR to improve their docs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

"file": 4,
"line": 6,
"message": 9
}
]
}
]
}
17 changes: 17 additions & 0 deletions .github/problem-matcher-lint-cpplint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"problemMatcher": [
{
"owner": "lint-cpplint",
"severity": "error",
"pattern": [
{
"regexp": "^(.+):(\\d+):\\s+((.+)\\s+\\[(.*)\\]\\s\\[(.*)\\])$",
"file": 1,
"line": 2,
"message": 3,
"code": 5
}
]
}
]
}
16 changes: 16 additions & 0 deletions .github/problem-matcher-lint-generic-nolints.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"problemMatcher": [
{
"owner": "lint-generic-nolints",
"pattern": [
{
"regexp": "^(notice|error)(:(file|dir):([^:]+)(:lines? (\\d+)(-(\\d+))?)?)?: (.+)$",
"severity": 1,
"file": 4,
"line": 6,
"message": 9
}
]
}
]
}
10 changes: 0 additions & 10 deletions .github/workflows/annotation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,3 @@ jobs:
- name: Flake8 with annotations - Python 2
# Using the flake8 options in the .flake8 file
uses: TrueBrain/actions-flake8@main
flake8-annotation-python3:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@master
- uses: actions/setup-python@v2
with:
python-version: '3.10'
- name: Flake8 with annotations - Python 3
# Using the flake8 options in the .flake8 file
uses: TrueBrain/actions-flake8@main
196 changes: 196 additions & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
name: lint
on: [push, pull_request]
jobs:
build:
name: Build for Lint Tasks
runs-on: ubuntu-latest
container: debian:stable
steps:
- name: Update package database
run: apt-get update -y
# See comments beginning at
# https://github.com/actions/runner/issues/763#issuecomment-1435474884
# Without Git, actions/checkout@v3 will resort to REST and will not
# create a .git folder or .git.config. The Problem Matcher looks for
# .git/config to find where the root of the repo is, so it must be
# present.
- name: Install Git
run: apt-get -y install git
- uses: actions/checkout@v3
- name: Install build tools
shell: bash
run: |
apt-get -y install pkg-config libtool autoconf \
automake g++ bison flex make bash-completion dh-autoreconf \
debhelper devscripts wget python3-pip
- name: Install Python lint tools
run: python3 -m pip install --no-input cpplint flake8
- name: Install build dependencies
shell: bash
run: |
apt-get -y install libcppunit-dev uuid-dev libncurses5-dev \
libmicrohttpd-dev protobuf-compiler python3-protobuf \
libprotobuf-dev libprotoc-dev zlib1g-dev libftdi-dev \
libusb-1.0-0-dev liblo-dev libavahi-client-dev python3-numpy
- name: Autoreconf
run: autoreconf -i
- name: Configure
run: ./configure --enable-rdm-tests --enable-ja-rule --enable-e133
- name: Build builtfiles
run: make builtfiles VERBOSE=1
- name: Display structure of the built files
if: env.ACTIONS_STEP_DEBUG == 'true'
run: ls -alR
- name: Archive artifacts to speed up slow GH Actions upload/download
shell: bash
# If the file does not exist when tar excludes it, then it will not
# actually exclude it, so it must first be touched
run: |
touch ola-debian-stable-built-source-tree.tar.gz
tar --exclude=ola-debian-stable-built-source-tree.tar.gz -cvzf ola-debian-stable-built-source-tree.tar.gz .
- name: SHA256 artifact archive
run: sha256sum ola-debian-stable-built-source-tree.tar.gz
- uses: actions/upload-artifact@v3
with:
name: ola-debian-stable-built-source-tree
path: ola-debian-stable-built-source-tree.tar.gz
check-licences:
name: Check Licences
runs-on: ubuntu-latest
container: debian:stable
needs: build
steps:
- name: Download built source tree archive
uses: actions/download-artifact@v3
with:
name: ola-debian-stable-built-source-tree
path: .
- name: SHA256 artifact archive
run: sha256sum ola-debian-stable-built-source-tree.tar.gz
- name: Unarchive artifacts and delete archive
shell: bash
run: |
tar -xvzf ola-debian-stable-built-source-tree.tar.gz .
rm ola-debian-stable-built-source-tree.tar.gz
- name: Display structure of extracted files
if: env.ACTIONS_STEP_DEBUG == 'true'
run: ls -alR
- name: Update package database
run: apt-get update -y
- name: Install Python
run: apt-get -y install python3 python-is-python3
- name: Enable Problem Matcher for GitHub annotations
run: echo "::add-matcher::.github/problem-matcher-lint-check-licences.json"
- name: Check licenses
shell: bash
run: ./scripts/enforce_licence.py
generic-nolints:
name: Count generic NOLINTs
runs-on: ubuntu-latest
container: debian:stable
needs: build
steps:
- name: Download built source tree archive
uses: actions/download-artifact@v3
with:
name: ola-debian-stable-built-source-tree
path: .
- name: SHA256 artifact archive
run: sha256sum ola-debian-stable-built-source-tree.tar.gz
- name: Unarchive artifacts and delete archive
shell: bash
run: |
tar -xvzf ola-debian-stable-built-source-tree.tar.gz .
rm ola-debian-stable-built-source-tree.tar.gz
- name: Display structure of extracted files
if: env.ACTIONS_STEP_DEBUG == 'true'
run: ls -alR
- name: Enable Problem Matcher for GitHub annotations
run: echo "::add-matcher::.github/problem-matcher-lint-generic-nolints.json"
- name: Count the number of generic NOLINTs
shell: bash
run: ./scripts/count_generic_nolints.sh
cpplint:
name: cpplint
runs-on: ubuntu-latest
container: debian:stable
needs: build
steps:
- name: Download built source tree archive
uses: actions/download-artifact@v3
with:
name: ola-debian-stable-built-source-tree
path: .
- name: SHA256 artifact archive
run: sha256sum ola-debian-stable-built-source-tree.tar.gz
- name: Unarchive artifacts and delete archive
shell: bash
run: |
tar -xvzf ola-debian-stable-built-source-tree.tar.gz .
rm ola-debian-stable-built-source-tree.tar.gz
- name: Display structure of extracted files
if: env.ACTIONS_STEP_DEBUG == 'true'
run: ls -alR
- name: Update package database
run: apt-get update -y
- name: Install build tools
shell: bash
run: |
apt-get -y install pkg-config libtool autoconf \
automake g++ bison flex make bash-completion dh-autoreconf \
debhelper devscripts wget python3-pip
- name: Install Python lint tools
run: python3 -m pip install --no-input cpplint flake8
- name: Install build dependencies
shell: bash
run: |
apt-get -y install libcppunit-dev uuid-dev libncurses5-dev \
libmicrohttpd-dev protobuf-compiler python3-protobuf \
libprotobuf-dev libprotoc-dev zlib1g-dev libftdi-dev \
libusb-1.0-0-dev liblo-dev libavahi-client-dev python3-numpy
- name: Enable Problem Matcher for GitHub annotations
run: echo "::add-matcher::.github/problem-matcher-lint-cpplint.json"
- name: cpplint
run: make cpplint VERBOSE=1
flake8:
name: flake8
runs-on: ubuntu-latest
container: debian:stable
needs: build
steps:
- name: Download built source tree archive
uses: actions/download-artifact@v3
with:
name: ola-debian-stable-built-source-tree
path: .
- name: SHA256 artifact archive
run: sha256sum ola-debian-stable-built-source-tree.tar.gz
- name: Unarchive artifacts and delete archive
shell: bash
run: |
tar -xvzf ola-debian-stable-built-source-tree.tar.gz .
rm ola-debian-stable-built-source-tree.tar.gz
- name: Display structure of extracted files
if: env.ACTIONS_STEP_DEBUG == 'true'
run: ls -alR
- name: Update package database
run: apt-get update -y
- name: Install build tools
shell: bash
run: |
apt-get -y install pkg-config libtool autoconf \
automake g++ bison flex make bash-completion dh-autoreconf \
debhelper devscripts wget python3-pip
- name: Install Python lint tools
run: python3 -m pip install --no-input cpplint flake8
- name: Setup flake8 annotations
uses: rbialon/flake8-annotations@v1
- name: Install build dependencies
shell: bash
run: |
apt-get -y install libcppunit-dev uuid-dev libncurses5-dev \
libmicrohttpd-dev protobuf-compiler python3-protobuf \
libprotobuf-dev libprotoc-dev zlib1g-dev libftdi-dev \
libusb-1.0-0-dev liblo-dev libavahi-client-dev python3-numpy
- name: flake8
run: make flake8 VERBOSE=1
2 changes: 1 addition & 1 deletion AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Contributors:
Masaki Muranaka, various patches
Nicolas, for the win32 port of libartnet
Nils Van Zuijlen, typos and python lib small update
Perry Naseck, EPoll delay fix
Perry Naseck, EPoll delay fix, various GitHub Actions CI workflows
Peter Newman, MilInst Plugin, Scanlime Fadecandy support and numerous fixes
Ravindra Nath Kakarla, RDM Test Server (Google Summer of Code 2012)
Rowan Maclachlan (hippy) for various changes
Expand Down
16 changes: 4 additions & 12 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,12 @@ lint: flake8 cpplint
.PHONY : flake8
flake8: Makefile.am
if FOUND_FLAKE8
flake8
flake8 --statistics --count
else
$(error flake8 not found. Install flake8 and re-run configure.)
endif

# cpplint linter
CPP_LINT_URL = "https://raw.githubusercontent.com/google/styleguide/gh-pages/cpplint/cpplint.py"
CPP_LINT_FILTER = "-legal/copyright,-readability/streams,-runtime/arrays"
CPP_LINT_FILES = $(shell find . \( -name "*.h" -or -name "*.cpp" \) -and ! \( \
-wholename "./common/protocol/Ola.pb.*" -or \
Expand All @@ -258,16 +257,9 @@ CPP_LINT_FILES = $(shell find . \( -name "*.h" -or -name "*.cpp" \) -and ! \( \
-wholename "./tools/ola_trigger/config.tab.*" -or \
-wholename "./tools/ola_trigger/lex.yy.cpp" \) | xargs)
.PHONY : cpplint
if FOUND_CPPLINT
cpplint: Makefile.am
cpplint.py --filter=$(CPP_LINT_FILTER) $(CPP_LINT_FILES)
if FOUND_CPPLINT
cpplint --filter=$(CPP_LINT_FILTER) $(CPP_LINT_FILES)
else
cpplint: Makefile.am cpplint.py
./cpplint.py --filter=$(CPP_LINT_FILTER) $(CPP_LINT_FILES)
endif

if !FOUND_CPPLINT
cpplint.py:
wget -O cpplint.py $(CPP_LINT_URL)
chmod u+x cpplint.py
$(error cpplint not found. Install forked cpplint (e.g. via pip for the latest version) and re-run configure.)
endif
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ fi
# Linters
AC_CHECK_PROG([flake8],[flake8],[yes],[no])
AM_CONDITIONAL([FOUND_FLAKE8], [test "x$flake8" = xyes])
AC_CHECK_PROG([cpplint],[cpplint.py],[yes],[no])
AC_CHECK_PROG([cpplint],[cpplint],[yes],[no])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah this might make my backwards compatible comment a bit less relevant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could also completely take this and flake8 out of dependency checking, if you're OK with that. I don't think that someone building OLA should necessarily be required to have cpplint and flake8 (or maybe there is already a ./configure flag for that).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe it should block the build if they're not present.

Indeed my config.log has:

configure:27245: checking for flake8
configure:27273: result: yes
configure:27291: checking for cpplint.py
configure:27319: result: no

It just means we can present a more user-friendly message if they try to run the tools.

AM_CONDITIONAL([FOUND_CPPLINT], [test "x$cpplint" = xyes])

# Output
Expand Down
18 changes: 9 additions & 9 deletions protoc/StrUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -360,39 +360,39 @@ char* FastUInt32ToBufferLeft(uint32_t u, char* buffer) {
buffer[0] = ASCII_digits[0];
buffer[1] = ASCII_digits[1];
buffer += 2;
sublt100_000_000:
sublt100_000_000:
u -= digits * 100000000; // 100,000,000
lt100_000_000:
lt100_000_000:
digits = u / 1000000; // 1,000,000
ASCII_digits = two_ASCII_digits[digits];
buffer[0] = ASCII_digits[0];
buffer[1] = ASCII_digits[1];
buffer += 2;
sublt1_000_000:
sublt1_000_000:
u -= digits * 1000000; // 1,000,000
lt1_000_000:
lt1_000_000:
digits = u / 10000; // 10,000
ASCII_digits = two_ASCII_digits[digits];
buffer[0] = ASCII_digits[0];
buffer[1] = ASCII_digits[1];
buffer += 2;
sublt10_000:
sublt10_000:
u -= digits * 10000; // 10,000
lt10_000:
lt10_000:
digits = u / 100;
ASCII_digits = two_ASCII_digits[digits];
buffer[0] = ASCII_digits[0];
buffer[1] = ASCII_digits[1];
buffer += 2;
sublt100:
sublt100:
u -= digits * 100;
lt100:
lt100:
digits = u;
ASCII_digits = two_ASCII_digits[digits];
buffer[0] = ASCII_digits[0];
buffer[1] = ASCII_digits[1];
buffer += 2;
done:
done:
*buffer = 0;
return buffer;
}
Expand Down
29 changes: 29 additions & 0 deletions scripts/count_generic_nolints.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env bash
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Library General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
#
# count_generic_nolints.sh
# Copyright (C) 2023 Perry Naseck, Peter Newman

# This script is based on a Travis CI test by Peter Newman
nolints="$(grep -n --exclude "$(basename $0)" -IR NOLINT * | grep -v "NOLINT(" | sed 's/^\(.*\):\([0-9]\+\):\(.*\)$/error:file:\.\/\1:line \2: Generic NOLINT not permitted/g')"
nolints_count="$(grep --exclude "$(basename $0)" -IR NOLINT * | grep -c -v "NOLINT(")"
if [[ $nolints_count -ne 0 ]]; then
# print the output for info
printf "%s\n" "$nolints"
printf "error: Found $nolints_count generic NOLINTs\n"
exit 1
else
printf "notice: Found $nolints_count generic NOLINTs\n"
fi
Loading