Incorporate some lint checks into meson test

This will make it easier and more obvious for developers to run them
locally: I'm sure I'm not the only developer who had assumed that
`.gitlab-ci/` is private to the CI environment and inappropriate (or
perhaps even destructive) to run on a developer/user system.

The lint checks are automatically skipped (with TAP SKIP syntax) if we
are not in a git checkout, or if git or the lint tool is missing. They
can also be disabled explicitly with `meson test --no-suite=lint`,
which downstream distributions will probably want to do.

By default, most lint checks are reported as an "expected failure"
(with TAP TODO syntax) rather than a hard failure, because they do not
indicate a functional problem with GLib and there is a tendency for
lint tools to introduce additional checks or become more strict over
time. Developers can override this by configuring with `-Dwerror=true`
(which also makes compiler warnings into fatal errors), or by running
the test suite like `LINT_WARNINGS_ARE_ERRORS=1 meson test --suite=lint`.

One exception to this is tests/check-missing-install-tag.py, which is
checking a functionally significant feature of our build system, and
seems like it is unlikely to have false positives: if that one fails,
it is reported as a hard failure.

run-style-check-diff.sh and run-check-todos.sh are not currently given
this treatment, because they require search-common-ancestor.sh, which
uses Gitlab-CI-specific information to find out which commits are in-scope
for checking.

Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie 2024-02-07 12:16:22 +00:00 committed by Philip Withnall
parent 27fb6ffce2
commit d7601f7eed
14 changed files with 225 additions and 51 deletions

View File

@ -137,9 +137,11 @@ sh-and-py-check:
allow_failure: false
script:
- git config --global --add safe.directory "${PWD}"
- .gitlab-ci/run-shellcheck.sh
- .gitlab-ci/run-black.sh
- .gitlab-ci/run-flake8.sh
- tests/shellcheck.sh
- tests/black.sh
- tests/flake8.sh
variables:
LINT_WARNINGS_ARE_ERRORS: '1'
only:
changes:
- "**/*.py"
@ -151,7 +153,9 @@ style-check-mandatory:
stage: style-check
allow_failure: false
script:
- .gitlab-ci/run-reuse.sh
- tests/reuse.sh
variables:
LINT_WARNINGS_ARE_ERRORS: '1'
fedora-x86_64:
extends:

View File

@ -1,6 +0,0 @@
#!/bin/bash
set -e
# shellcheck disable=SC2046
black --diff --check $(git ls-files '*.py')

View File

@ -1,9 +0,0 @@
#!/bin/bash
set -e
# Disable formatting warnings in flake8, as we use `black` to handle that.
formatting_warnings=E101,E111,E114,E115,E116,E117,E12,E13,E2,E3,E401,E5,E70,W1,W2,W3,W5
# shellcheck disable=SC2046
flake8 --max-line-length=88 --ignore="$formatting_warnings" $(git ls-files '*.py')

View File

@ -1,7 +0,0 @@
#!/bin/bash
set -e
# Ignoring third-party directories that we don't want to parse
# shellcheck disable=SC2046
shellcheck $(git ls-files '*.sh' | grep -Ev "glib/libcharset|glib/dirent")

View File

@ -2,8 +2,6 @@
set -ex
./.gitlab-ci/check-missing-install-tag.py _build
meson test -v \
-C _build \
--timeout-multiplier "${MESON_TEST_TIMEOUT_MULTIPLIER}" \

View File

@ -14,7 +14,6 @@ set args=%args:~1%
:: FIXME: make warnings fatal
pip3 install --upgrade --user meson==1.2.3 packaging==23.2 || goto :error
meson setup %args% _build || goto :error
python .gitlab-ci/check-missing-install-tag.py _build || goto :error
meson compile -C _build || goto :error
meson test -v -C _build --timeout-multiplier %MESON_TEST_TIMEOUT_MULTIPLIER% || goto :error

View File

@ -171,6 +171,10 @@ common_test_env = [
'MALLOC_CHECK_=2',
]
if get_option('werror')
common_test_env += 'LINT_WARNINGS_ARE_ERRORS=1'
endif
# Note: this may cause the tests output not to be printed when running in
# verbose mode, see https://github.com/mesonbuild/meson/issues/11185
# Can be changed it to 'exitcode' if required during development.
@ -2545,6 +2549,7 @@ subdir('gmodule')
subdir('gio')
subdir('girepository')
subdir('fuzzing')
subdir('tests')
# xgettext is optional (on Windows for instance)
if find_program('xgettext', required : get_option('nls')).found()

23
tests/black.sh Executable file
View File

@ -0,0 +1,23 @@
#!/usr/bin/env bash
# Copyright 2020 Frederic Martinsons
# Copyright 2020 Endless OS Foundation, LLC
# Copyright 2024 Collabora Ltd.
# SPDX-License-Identifier: LGPL-2.1-or-later
set -eu
if [ -z "${G_TEST_SRCDIR-}" ]; then
me="$(readlink -f "$0")"
G_TEST_SRCDIR="${me%/*}"
fi
export TEST_NAME=black
export TEST_REQUIRES_TOOLS="black git"
run_lint () {
# shellcheck disable=SC2046
black --diff --check $(git ls-files '*.py')
}
# shellcheck source=tests/lint-common.sh
. "$G_TEST_SRCDIR/lint-common.sh"

View File

@ -1,6 +1,6 @@
#!/usr/bin/env python3
#
# Copyright © 2022 Collabora, Ltd.
# Copyright © 2022-2024 Collabora, Ltd.
#
# SPDX-License-Identifier: LGPL-2.1-or-later
#
@ -18,19 +18,33 @@ from pathlib import Path
def main():
parser = argparse.ArgumentParser()
parser.add_argument("builddir", type=Path)
parser.add_argument("builddir", type=Path, nargs="?", default=".")
args = parser.parse_args()
success = True
print("# TAP version 13")
count = 0
bad = 0
path = args.builddir / "meson-info" / "intro-install_plan.json"
with path.open(encoding="utf-8") as f:
install_plan = json.load(f)
for target in install_plan.values():
for info in target.values():
count += 1
if not info["tag"]:
print("Missing install_tag for", info["destination"])
success = False
return 0 if success else 1
bad += 1
dest = info["destination"]
print(f"not ok {bad} - Missing install_tag for {dest}")
if bad == 0:
print(f"ok 1 - All {count} installed files have install_tag")
print("1..1")
return 0
else:
print(f"# {bad}/{count} installed files do not have install_tag")
print(f"1..{bad}")
return 1
if __name__ == "__main__":

28
tests/flake8.sh Executable file
View File

@ -0,0 +1,28 @@
#!/usr/bin/env bash
# Copyright 2020 Frederic Martinsons
# Copyright 2020 Endless OS Foundation, LLC
# Copyright 2024 Collabora Ltd.
# SPDX-License-Identifier: LGPL-2.1-or-later
set -eu
if [ -z "${G_TEST_SRCDIR-}" ]; then
me="$(readlink -f "$0")"
G_TEST_SRCDIR="${me%/*}"
fi
export TEST_NAME=flake8
export TEST_REQUIRES_TOOLS="flake8 git"
run_lint () {
# Disable formatting warnings in flake8, as we use `black` to handle that.
local formatting_warnings=E101,E111,E114,E115,E116,E117,E12,E13,E2,E3,E401,E5,E70,W1,W2,W3,W5
# shellcheck disable=SC2046
flake8 \
--max-line-length=88 --ignore="$formatting_warnings" \
$(git ls-files '*.py')
}
# shellcheck source=tests/lint-common.sh
. "$G_TEST_SRCDIR/lint-common.sh"

46
tests/lint-common.sh Normal file
View File

@ -0,0 +1,46 @@
#!/usr/bin/env bash
# Copyright 2016-2018 Simon McVittie
# Copyright 2018-2024 Collabora Ltd.
# SPDX-License-Identifier: LGPL-2.1-or-later
set -eu
skip_all () {
echo "1..0 # SKIP $*"
exit 0
}
main () {
local need_git=
local tool
cd "$G_TEST_SRCDIR/.."
echo "TAP version 13"
# shellcheck disable=SC2046
for tool in ${TEST_REQUIRES_TOOLS-}; do
command -v "$tool" >/dev/null || skip_all "$tool not found"
if [ "$tool" = git ]; then
need_git=1
fi
done
if [ -n "${need_git-}" ] && ! test -e .git; then
skip_all "not a git checkout"
fi
echo "1..1"
if run_lint >&2; then
echo "ok 1"
exit 0
elif [ -n "${LINT_WARNINGS_ARE_ERRORS-}" ]; then
echo "not ok 1 - warnings from ${TEST_NAME-"lint tool"}"
exit 1
else
echo "not ok 1 # TO""DO warnings from ${TEST_NAME-"lint tool"}"
exit 0
fi
}
main

27
tests/meson.build Normal file
View File

@ -0,0 +1,27 @@
# Copyright 2024 Collabora Ltd.
# SPDX-License-Identifier: LGPL-2.1-or-later
lint_scripts = [
'black.sh',
'flake8.sh',
'reuse.sh',
'shellcheck.sh',
]
foreach test_name : lint_scripts
test(
test_name, files(test_name),
env : common_test_env,
suite : 'lint',
protocol : 'tap',
)
endforeach
test(
'check-missing-install-tag.py',
python,
args : ['-B', files('check-missing-install-tag.py')],
env : common_test_env,
suite : 'lint',
protocol : 'tap',
)

View File

@ -1,23 +1,43 @@
#!/bin/bash
#!/usr/bin/env bash
#
# Copyright 2022 Endless OS Foundation, LLC
# Copyright 2024 Collabora Ltd.
#
# SPDX-License-Identifier: LGPL-2.1-or-later
#
# Original author: Philip Withnall
set -e
set -eu
if [ -z "${G_TEST_SRCDIR-}" ]; then
me="$(readlink -f "$0")"
G_TEST_SRCDIR="${me%/*}"
fi
skip_all () {
echo "1..0 # SKIP $*"
exit 0
}
cd "$G_TEST_SRCDIR/.."
echo "TAP version 13"
command -v git >/dev/null || skip_all "git not found"
command -v reuse >/dev/null || skip_all "reuse not found"
test -e .git || skip_all "not a git checkout"
echo "1..1"
# We need to make sure the submodules are up to date, or `reuse lint` will fail
# when it tries to run `git status` internally
git submodule update --init
git submodule update --init >&2
# Run `reuse lint` on the code base and see if the number of files without
# suitable copyright/licensing information has increased from a baseline
# FIXME: Eventually this script can check whether *any* files are missing
# information. But for now, lets slowly improve the baseline.
files_without_copyright_information_max=346
files_without_license_information_max=417
files_without_copyright_information_max=343
files_without_license_information_max=414
# The || true is because `reuse lint` will exit with status 1 if the project is not compliant
# FIXME: Once https://github.com/fsfe/reuse-tool/issues/512 or
@ -36,27 +56,36 @@ files_without_license_information="$(( total_files - files_with_license_informat
if [ "${files_without_copyright_information}" -gt "${files_without_copyright_information_max}" ] || \
[ "${files_without_license_information}" -gt "${files_without_license_information_max}" ]; then
echo "${lint_output}"
echo "${lint_output}" >&2
fi
if [ "${files_without_copyright_information}" -gt "${files_without_copyright_information_max}" ]; then
echo ""
echo "Error: New files added without REUSE-compliant copyright information"
echo "Please make sure that all files added in this branch/merge request have correct copyright information"
echo "" >&2
echo "Error: New files added without REUSE-compliant copyright information" >&2
echo "Please make sure that all files added in this branch/merge request have correct copyright information" >&2
error=1
fi
if [ "${files_without_license_information}" -gt "${files_without_license_information_max}" ]; then
echo ""
echo "Error: New files added without REUSE-compliant licensing information"
echo "Please make sure that all files added in this branch/merge request have correct license information"
echo "" >&2
echo "Error: New files added without REUSE-compliant licensing information" >&2
echo "Please make sure that all files added in this branch/merge request have correct license information" >&2
error=1
fi
if [ "${error}" -eq "1" ]; then
echo ""
echo "See https://reuse.software/tutorial/#step-2 for information on how to add REUSE information"
echo "Also see https://gitlab.gnome.org/GNOME/glib/-/issues/1415"
echo "" >&2
echo "See https://reuse.software/tutorial/#step-2 for information on how to add REUSE information" >&2
echo "Also see https://gitlab.gnome.org/GNOME/glib/-/issues/1415" >&2
fi
exit "${error}"
if [ "${error}" -eq 0 ]; then
echo "ok 1"
exit 0
elif [ -n "${LINT_WARNINGS_ARE_ERRORS-}" ]; then
echo "not ok 1 - warnings from reuse"
exit "${error}"
else
echo "not ok 1 # TO""DO warnings from reuse"
exit 0
fi

23
tests/shellcheck.sh Executable file
View File

@ -0,0 +1,23 @@
#!/usr/bin/env bash
# Copyright 2020 Frederic Martinsons
# Copyright 2024 Collabora Ltd.
# SPDX-License-Identifier: LGPL-2.1-or-later
set -eu
if [ -z "${G_TEST_SRCDIR-}" ]; then
me="$(readlink -f "$0")"
G_TEST_SRCDIR="${me%/*}"
fi
export TEST_NAME=shellcheck
export TEST_REQUIRES_TOOLS="git shellcheck"
run_lint () {
# Ignoring third-party directories that we don't want to parse
# shellcheck disable=SC2046
shellcheck $(git ls-files '*.sh' | grep -Ev "glib/libcharset|glib/dirent")
}
# shellcheck source=tests/lint-common.sh
. "$G_TEST_SRCDIR/lint-common.sh"