Lint: call tools inside venv using make's PYTHON

[hugovk]

For #2402.
Fixes #2403.

The pre-commit tool can be called as a module as pre_commit: pre-commit/pre-commit#1627 (comment)

This allows us to choose which Python version to use:

make lint
make PYTHON=python3.7 lint
make PYTHON=/Users/huvankem/.pyenv/shims/python3.8 lint

And the tools are installed inside a venv using the given Python version.

This is based on the CPython docs makefile and devguide makefile, so should follow a familiar pattern.

[AA-Turner]

This sort of goes against the point of make as I understand it -- if the name exists on the filesystem then make won't run the target, whereas we need a lot of logic here to do the same thing and support customisation of the venv directory.

I really don't want to overcomplicate this -- whilst I have a desire to make usability better for PEP authors, no-one complained up until now, and so we should balance usability to PEP authors for whom the current Makefile doesn't work and maintainability to ourselves.

A

[hugovk]

Yep, make is primarily for building/compiling files but this sort of thing is pretty common too.

This one is based on the CPython docs Makefile:

venv:
	@if [ -d $(VENVDIR) ] ; then \
		echo "venv already exists."; \
		echo "To recreate it, remove it first with \`make clean-venv'."; \
	else \
		$(PYTHON) -m venv $(VENVDIR); \
		$(VENVDIR)/bin/python3 -m pip install -U pip setuptools; \
		$(VENVDIR)/bin/python3 -m pip install -r requirements.txt; \
		echo "The venv has been created in the $(VENVDIR) directory"; \
	fi

The devguide's Makefile is similar:

venv:
	$(PYTHON) -m venv venv
	./venv/bin/python3 -m pip install --upgrade pip
	./venv/bin/python3 -m pip install -r requirements.txt

I'm sure I've seen similar in other https://github.com/python Makefiles, so using venvs is a fairy common pattern.

[CAM-Gerlach]

I'm not a make expert, but as a general principle, I don't think we should let purity override practicality here, particularly when it comes to usability for PEP authors and contributors. We certainly want to reduce friction there as much as practical, and given all the changes lately and the remaining ones we want to do, making things easier rather than harder for users helps all that go more smoothly for everyone.

[AA-Turner]

If one installs make on windows, this would fail, right? As the executables directory is Scripts.

A

[hugovk]

I don't use make on Windows, I've often seen a separate make.bat though so I think that is used instead of Makefile.

• https://github.com/python/cpython/blob/main/Doc/Makefile
• https://github.com/python/cpython/blob/main/Doc/make.bat

[brettcannon]

@AA-Turner correct, this is Unix-only. The PYTHON variable could be updated to use VENVDIR, but that would also allow for overriding the path to the Python executable.

[CAM-Gerlach]

I do have make on my Windows dev machines and use it when necessary sometimes, but at least on this repo I don't really do so, and instead run the various commands I need directly (usually with custom invocations). I wouldn't expect very many if any non-WSL Windows users to do so either.

[CAM-Gerlach]

I'm not a make expert, but this LGTM aside from one not directly related concern. Thanks, @hugovk

[AA-Turner]

Not a domain expert, so leaving this to someone else to merge!

A