Skip to content
This repository has been archived by the owner on Mar 18, 2019. It is now read-only.

Perform sanity checks with Python3 #25

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
29 changes: 26 additions & 3 deletions mk/python.mk
Original file line number Diff line number Diff line change
@@ -1,12 +1,35 @@
PYTHON = python3.5
PYTHON := $(shell which $(PYTHON))
# check Python version
PYTHON := $(shell which "python")
PYTHON_COMPATIBLE = python3.5
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename it to PYTHON35 to reflect its functionality.


Copy link
Collaborator

Choose a reason for hiding this comment

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

No blank line here.

ifndef PYTHON
Copy link
Collaborator

@jserv jserv Aug 18, 2018

Choose a reason for hiding this comment

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

Add extra condition if python indicated by $PATH is exactly of version 3. Think of the case that somebody built Python3 from source and the generated executable is python.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If python is not present, try python3. Then, if version is mismatching, try python3.5.

$(error "python3.5 is required.")
$(error "Python is not executable, try to check the PATH environment variable or install $(PYTHON_COMPATIBLE) package.")
endif # PYTHON

ifdef PYTHON
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't have to check $(PYTHON) since you already exclude its absence.

PY_CHECK_VERSION := $(shell python -c "print(__import__('sys').version_info[0:2])")

ifneq ($(PY_CHECK_VERSION), (3, 5))
PYTHON_COMPATIBLE_PATH := $(shell which $(PYTHON_COMPATIBLE))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't rely on new var PYTHON_COMPATIBLE_PATH. Instead, override PYTHON since you always block the one which is not accepted by each qualification.


ifdef PYTHON_COMPATIBLE_PATH
$(error "Found $(PYTHON_COMPATIBLE) in current environment, but it should be setting as \
default interpreter, try to create a symbolic link for $(PYTHON_COMPATIBLE_PATH)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't suggest to make symbolic link. Instead, configure via environment variables.

else
$(error "$(shell python -V):Invalid Python version, only available for $(PYTHON_COMPATIBLE).")
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is NOT Invalid Python version. Instead, only Python 3.5 is known to work with PyOTA. You have to address.

endif
endif
endif # PYTHON

ifndef PYTHON
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto. Focus on detailed items.


endif # PYTHON

# check "iota" module in Python installation
ifdef PYTHON
PY_CHECK_MOD_IOTA := $(shell $(PYTHON) -c "import iota" 2>/dev/null && \
echo 1 || echo 0)
ifneq ("$(PY_CHECK_MOD_IOTA)","1")
$(error "dependency error $@ because PyOTA is not installed, to install the latest version: pip3 install pyota")
Copy link
Collaborator

Choose a reason for hiding this comment

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

If Python executable is restricted, the validation of pip would be considered.

endif
endif # PYTHON
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avoid nested ifdef and endif pair as possible as you can.