Makefile: Distinguish between Python and legacy Python

[cclauss]

@rvagg Your review please.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[rvagg]

How about we leave PYTHON and just add PYTHON3, then when we migrate properly to 3 we can just drop PYTHON3. The existing PYTHON has been in Makefile for a very long time and is in use in a our infra already and probably in others' infra that we're not even aware of.

[rvagg]

@cclauss do you know whether python2 and python3 are standard names across platforms? Should we expect these things to exist in most places?

I'm also wondering if we're going to break existing use by switching out the default from python. Perhaps there are folks who are already using python 3 and it's being picked up with PYTHON ?= python and if we go to python2 they may not have that in place.

We could leave PYTHON ?= python and just add PYTHON3 ?= python3, then supply PYTHON=python2 in CI. That way we won't be breaking existing users' workflows. Would that achieve what we're after?

I doubt anyone's using lint-py in the wild (well, I suppose I don't know) so we probably don't need to guard against python3 existing.

[cclauss]

It would be much more with the trend if we change $PYTHON --> $LEGACY_PYTHON and use $PYTHON for Python 3.

[rvagg]

And then only use LEGACY_PYTHON in the lint-py job? leaving all of the existing PYTHON references as python3? What would the default for both of those be?

[cclauss]

After reflecting over my morning ☕️ ...

python2 and python3 are standard names across platforms. Let's track with that in our Makefile variable names. On current versions of several Linux distros python --version already defaults to Python 3. $LEGACY_PYTHON is just too long-winded. Given that part of the Python philosophy is that explicit is better than implicit, let's stick with PYTHON2 and PYTHON3 as is already the case in this PR.

Let's transition our use in Makefile one step at a time... Step one (this PR) is to run flake8 on both PYTHON2 and PYTHON3 because the two have different Abstract Syntax Trees so they generate different reports. Once this PR lands, we can walk thru Makefile and change PYTHON2 --> PYTHON3 and fix issues as we go. This follows the Facebook model for how they successfully completed their transitions: Treat Python 3 incompatibility as a bug and fix bugs as we go.

[rvagg]

@cclauss but that doesn't address the fact that PYTHON is being used in the wild to interact with the Makefile, so it would nice to have a migration path that doesn't just drop it on the floor. For one, it'll probably prevent any improvements here from being backported to other release lines (10.x will be supported well beyond the Python 2 EOL).

You could introduce special behaviour that is only invoked if someone supplies a PYTHON, maybe it just uses that value for both PYTHON2 and PYTHON3 or skips certain paths, or even inspects what version of Python is being provided and does something based on that.

We could deprecate PYTHON entirely but we should probably do that in stages like we do with other parts of Node that users interact with. Perhaps print a warning to start, but continue observing it for now.

[cclauss]

Sorry... I do not understand. PYTHON is a local variable created on line 4 of this Makefile. It is not in the wild, it is defined and used in this Makefile.

Try which -a python python2 python3 and you will probably discover python and python2 are both pointing to the exact same executable. This PR is merely proposing that we leverage that convention to always be explicit about which Python we want to use. This enables us to port in a controlled way. Initially, we will leave all current uses on PYTHON2 and add just one new run of flake8 testing on PYTHON3. Once we have proven that this works, future PRs can modify other tasks in this Makefile to use PYTHON3 instead of PYTHON2. Sometime in the next 12 months, we should remove the PYTHON2 variable from this Makefile because we do not want to build software on an insecure base.

[rvagg]

PYTHON ?= python - the ?= means it can be user supplied, the python in this case is just the default.

make PYTHON=/opt/some/custom/python.thing is possible and we use it in various parts of our infrastructure and can't rule out it being used in the wild.

[thefourtheye]

We could leave PYTHON ?= python and just add PYTHON3 ?= python3, then supply PYTHON=python2 in CI. That way we won't be breaking existing users' workflows. Would that achieve what we're after?

I think we can follow this route, given that removing PYTHON is not possible at this point of time. One question, should we allow conditional assignment for Python 3 as well in the Makefile (PYTHON3 ?= python3)?

[cclauss]

@rvagg Thanks for your explanation... Now all of your points make sense to me.

Could we move to a model where we allow the caller to specify:

1. make
2. make PYTHON=x
3. make PYTHON2=y
4. make PYTHON3=z
5. make PYTHON2=y PYTHON3=z

And this Makefile does the right thing to cover all cases?

My thought was this (please help with Makefile syntax!)...

PYTHON ?= python
PYTHON2 ?= python2
PYTHON3 ?= python3

PY_MAJOR := $(shell $(PYTHON) -c "import sys ; print(sys.version_info.major)")
if $(PY_MAJOR) < 3:
	PYTHON2 := $(PYTHON)
else:
	PYTHON3 := $(PYTHON)

echo "$(PYTHON) $(PYTHON2) $(PYTHON3)"

[cclauss]

Maybe one more corner case... If Python 3 is not installed then PYTHON3 := $(PYTHON2)

[thefourtheye]

I am kinda worried about introducing conditional assignment for PYTHON3. If we allow that and if people start using it, getting rid of that variable is going to be difficult again.

[cclauss]

Understood. What about an approach where PYTHON2 and PYTHON3 are merely local variables.?

We allow the caller to specify:

1. make
2. make PYTHON=x

And this Makefile does the right thing to cover both cases?

My thought was this (please help with Makefile syntax!)...

PYTHON ?= python
PY_MAJOR := $(shell $(PYTHON) -c "import sys ; print(sys.version_info.major)")
if $(PY_MAJOR) < 3:
	PYTHON2 := $(PYTHON)
        PYTHON3 := python3
else:
        PYTHON2 := python2
	PYTHON3 := $(PYTHON)
endif
echo "$(PYTHON) $(PYTHON2) $(PYTHON3)"

With the corner cases being where python2 is not installed and where python3 is not installed.

[cclauss]

Thanks folks. I am going to close this one down. I have been slow to understand what you both probably understood from the beginning. We can get everything that we are looking for just by modifying the call:

• make lint-py PYTHON=python2
• make lint-py PYTHON=python3