python3 setup.py lint is deprecated: Let's lint with pre-commit

[cclauss]

Running python3 setup.py lint is deprecated. https://packaging.python.org/en/latest/discussions/setup-py-deprecated

The setup.py logic for running black and clang-format is ~100 lines long and is skipping many Python and C files.

pygame-ce/setup.py

Lines 786 to 875 in 65a256d

	class LintFormatCommand(Command):
	""" Used for formatting or linting. See Lint and Format Sub classes.
	"""
	user_options = []
	lint = False
	format = False
	def initialize_options(self):
	pass
	def finalize_options(self):
	pass
	def run(self):
	"""Check the existence and launch linters."""
	import subprocess
	import sys
	import warnings
	import pathlib
	def check_linter_exists(linter):
	if shutil.which(linter) is None:
	msg = "Please install '%s' in your environment. (hint: 'python3 -m pip install %s')"
	warnings.warn(msg % (linter, linter))
	sys.exit(1)
	def filter_files(path_obj, all_files, allowed_files, disallowed_files):
	files = []
	for file in all_files:
	for disallowed in disallowed_files:
	if file.match(str(path_obj / disallowed)):
	break
	else: # no-break
	files.append(str(file))
	continue
	for allowed in allowed_files:
	if file.match(str(path_obj / allowed)):
	files.append(str(file))
	break
	return files
	path_obj = pathlib.Path(path, "src_c")
	c_files_unfiltered = path_obj.glob("**/*.[ch]")
	c_file_disallow = [
	"_sdl2/**",
	"pypm.c",
	"**/sse2neon.h",
	"doc/**",
	]
	c_file_allow = ["_sdl2/touch.c"]
	c_files = filter_files(path_obj, c_files_unfiltered, c_file_allow, c_file_disallow)
	# Other files have too many issues for now. setup.py, buildconfig, etc
	python_directories = ["src_py", "test", "examples", "docs", "--exclude", "docs/reST"]
	if self.lint:
	commands = {
	"clang-format": ["--dry-run", "--Werror", "-i"] + c_files,
	"black": ["--check", "--diff"] + python_directories,
	# Test directory has too much pylint warning for now
	"pylint": ["src_py", "docs"],
	}
	else:
	commands = {
	"clang-format": ["-i"] + c_files,
	"black": python_directories,
	}
	formatters = ["black", "clang-format"]
	for linter, option in commands.items():
	print(" ".join([linter] + option))
	check_linter_exists(linter)
	result = subprocess.run([linter] + option)
	if result.returncode:
	msg = f"'{linter}' failed."
	msg += " Please run: python setup.py format" if linter in formatters else ""
	msg += f" Do you have the latest version of {linter}?"
	raise SystemExit(msg)
	@add_command("lint")
	class LintCommand(LintFormatCommand):
	lint = True
	@add_command("format")
	class FormatCommand(LintFormatCommand):
	format = True

This pull request proposes using pre-commit with ~25 lines of configuration to rapidly run these two tools locally on contributors' machines and also in our GitHub Actions to cover any contributors who do not have pre-commit installed.

The 15 exclude lines should be removed gradually as more of the codebase passes the formatting and linting process.

How to use:

brew install pre-commit  # or `python3 -m pip install pre-commit`

pre-commit install
    > pre-commit installed at .git/hooks/pre-commit

pre-commit autoupdate
    > [https://github.com/psf/black-pre-commit-mirror] already up to date!
    > [https://github.com/pre-commit/mirrors-clang-format] already up to date!

pre-commit run --all-files
    > black....................................................................Passed
    > clang-format.............................................................Passed

[ankith26]

Our middle term goal is to move away from the current setup.py based buildconfig entirely (ref: #2557)

I think this PR is a step in the right direction, thanks!

[cclauss]

@ankith26 @Starbuck5 @oddbookworm Your reviews, please.

[ankith26]

Thanks for contributing! 🎉

I have tested this PR locally by deliberately making bad formatting changes. Both hooks ran correctly and fixed formatting. It also aborted the commit, but I think that's expected behaviour, one has to attempt another commit after a failed pre commit hook?

So the pre-commit hooks are all good.

I am still not clear how the action works exactly, but I'd like to merge this PR anyways and then look into the results

[oddbookworm]

LGTM!

[oddbookworm]

For future reference if/when these want to be removed, I'm leaving some rationale for keeping them excluded:
src_c/_sdl2 is autogenerated from cython, we shouldn't block merges based on the linting of autogen code
src_c/doc is the same story, it's autogenerated from docs generation
src_c/include/sse2neon.h is a header that we're vendoring in, I'm not sure we need to lint it because it's from a 3rd party
src_c/pypm.c is autogenerated from cython
docs/reST/_templates/header.h is the template for the src_c/doc generated files and isn't a valid C header file itself, so using a C linter would probably cause a black hole or something, I dunno
docs/reST/_static/script.js I dunno about this one, something something sphinx, maybe someone else can enlighten me on this file