| Message ID | 20200714013026.9019-2-jsnow@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | python: add make check-python target | expand |
On Mon, Jul 13, 2020 at 09:30:26PM -0400, John Snow wrote: > Move pylintrc and flake8 up to the root of the python folder where > they're the most useful. Add a requirements.cqa.txt file to house > the requirements necessary to build a venv sufficient for running > code quality analysis on the python folder. Add a makefile that > will build the venv and run the tests. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > Makefile | 1 + > python/{qemu => }/.flake8 | 0 > python/Makefile.include | 33 +++++++++++++++++++++++++++++++++ > python/{qemu => }/pylintrc | 1 + > python/requirements.cqa.txt | 3 +++ > 5 files changed, 38 insertions(+) > rename python/{qemu => }/.flake8 (100%) > create mode 100644 python/Makefile.include > rename python/{qemu => }/pylintrc (99%) > create mode 100644 python/requirements.cqa.txt > > diff --git a/Makefile b/Makefile > index b1b8a5a6d0..41808be392 100644 > --- a/Makefile > +++ b/Makefile > @@ -478,6 +478,7 @@ dummy := $(call unnest-vars,, \ > trace-obj-y) > > include $(SRC_PATH)/tests/Makefile.include > +include $(SRC_PATH)/python/Makefile.include > > all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) recurse-all modules $(vhost-user-json-y) > > diff --git a/python/qemu/.flake8 b/python/.flake8 > similarity index 100% > rename from python/qemu/.flake8 > rename to python/.flake8 > diff --git a/python/Makefile.include b/python/Makefile.include > new file mode 100644 > index 0000000000..917808e2f1 > --- /dev/null > +++ b/python/Makefile.include > @@ -0,0 +1,33 @@ > +# -*- Mode: makefile -*- > + > +PYLIB_VENV_DIR=$(BUILD_DIR)/venv/cqa > +PYLIB_VENV_REQ=$(SRC_PATH)/python/requirements.cqa.txt > + > +$(PYLIB_VENV_DIR): $(PYLIB_VENV_REQ) > + $(call quiet-command, \ > + $(PYTHON) -m venv $@, \ > + VENV, $@) > + $(call quiet-command, \ > + $(PYLIB_VENV_DIR)/bin/python3 -m pip -q install -r $(PYLIB_VENV_REQ), \ > + PIP, $(PYLIB_VENV_REQ)) > + $(call quiet-command, touch $@) > + Maybe we should try to create a generic rule that takes a directory name and a requirements file and creates the venv accordingly, instead of duplicating the other similar rules under tests/Makefile.include? > +pylib-venv: $(PYLIB_VENV_DIR) > + > +check-python: pylib-venv > + $(call quiet-command, cd $(SRC_PATH)/python && \ > + $(PYLIB_VENV_DIR)/bin/python3 -m flake8 qemu, \ > + FLAKE8, \ > + $(SRC_PATH)/python/qemu \ I can see how this venv would be very useful to run the same checks on other Python code (for instance, the acceptance tests themselves), so we'd also need another "check-python"-like rule, or include those on the same call. Ideas? :) Thanks! - Cleber.
On 7/14/20 12:30 AM, Cleber Rosa wrote: > On Mon, Jul 13, 2020 at 09:30:26PM -0400, John Snow wrote: >> Move pylintrc and flake8 up to the root of the python folder where >> they're the most useful. Add a requirements.cqa.txt file to house >> the requirements necessary to build a venv sufficient for running >> code quality analysis on the python folder. Add a makefile that >> will build the venv and run the tests. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> Makefile | 1 + >> python/{qemu => }/.flake8 | 0 >> python/Makefile.include | 33 +++++++++++++++++++++++++++++++++ >> python/{qemu => }/pylintrc | 1 + >> python/requirements.cqa.txt | 3 +++ >> 5 files changed, 38 insertions(+) >> rename python/{qemu => }/.flake8 (100%) >> create mode 100644 python/Makefile.include >> rename python/{qemu => }/pylintrc (99%) >> create mode 100644 python/requirements.cqa.txt >> >> diff --git a/Makefile b/Makefile >> index b1b8a5a6d0..41808be392 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -478,6 +478,7 @@ dummy := $(call unnest-vars,, \ >> trace-obj-y) >> >> include $(SRC_PATH)/tests/Makefile.include >> +include $(SRC_PATH)/python/Makefile.include >> >> all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) recurse-all modules $(vhost-user-json-y) >> >> diff --git a/python/qemu/.flake8 b/python/.flake8 >> similarity index 100% >> rename from python/qemu/.flake8 >> rename to python/.flake8 >> diff --git a/python/Makefile.include b/python/Makefile.include >> new file mode 100644 >> index 0000000000..917808e2f1 >> --- /dev/null >> +++ b/python/Makefile.include >> @@ -0,0 +1,33 @@ >> +# -*- Mode: makefile -*- >> + >> +PYLIB_VENV_DIR=$(BUILD_DIR)/venv/cqa >> +PYLIB_VENV_REQ=$(SRC_PATH)/python/requirements.cqa.txt >> + >> +$(PYLIB_VENV_DIR): $(PYLIB_VENV_REQ) >> + $(call quiet-command, \ >> + $(PYTHON) -m venv $@, \ >> + VENV, $@) >> + $(call quiet-command, \ >> + $(PYLIB_VENV_DIR)/bin/python3 -m pip -q install -r $(PYLIB_VENV_REQ), \ >> + PIP, $(PYLIB_VENV_REQ)) >> + $(call quiet-command, touch $@) >> + > > Maybe we should try to create a generic rule that takes a directory > name and a requirements file and creates the venv accordingly, instead > of duplicating the other similar rules under tests/Makefile.include? > Maybe, but I have to admit that my Makefile prowess is lacking and would be at the mercy of Somebody Else(tm) to do that. Can I get away with saying "Patches welcome" here? >> +pylib-venv: $(PYLIB_VENV_DIR) >> + >> +check-python: pylib-venv >> + $(call quiet-command, cd $(SRC_PATH)/python && \ >> + $(PYLIB_VENV_DIR)/bin/python3 -m flake8 qemu, \ >> + FLAKE8, \ >> + $(SRC_PATH)/python/qemu \ > > I can see how this venv would be very useful to run the same checks on > other Python code (for instance, the acceptance tests themselves), so > we'd also need another "check-python"-like rule, or include those on > the same call. > > Ideas? :) > Not good ones at the moment... this is still fuzzy in my head. There's no reason to conflate development packages (mypy, flake8, and pylint) with runtime packages (avocado-framework, pycdlib). I consider the requirements.cqa.txt file I added the "development requirements". I pinned them to specific versions for the purposes of CI repeatability. (A future patch that may add a setup.py here would re-add these packages without pinned versions.) Since the acceptance test venv had a different use case (running the code) versus this one (analyzing the code) I kept them separate. That said; maybe it's not a problem to use the same actual venv, but use different pip setup steps as-needed. We would need to be careful not to pin conflicting versions between the two different directories! We'd also then want a test (somewhere) that did nothing but installed both sets of requirements and made sure it worked. Lastly, I want to point out that with future plans to package the python library as an independently installable entity I want to avoid putting anything in that directory that references python code it "doesn't manage". avocado-framework, for example, has no business being referenced in python/ yet. Ideally this folder would also have its own Makefile that ran the code quality analysis checks by itself without the Qemu infrastructure involved (e.g. you can just type 'make check' inside of ./qemu/python), but then there's code duplication between Makefile and Makefile.include. That got messy looking and stupid, so I opted for the top-level include instead for now so that it could be invoked from the build directory, but I'm not sure I made the right call. > Thanks! > - Cleber. > Oh, and the final step that's needed is to add a gitlab CI job to run check-python as a test step, but that should be easy after Dan's changes. --js
diff --git a/Makefile b/Makefile index b1b8a5a6d0..41808be392 100644 --- a/Makefile +++ b/Makefile @@ -478,6 +478,7 @@ dummy := $(call unnest-vars,, \ trace-obj-y) include $(SRC_PATH)/tests/Makefile.include +include $(SRC_PATH)/python/Makefile.include all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) recurse-all modules $(vhost-user-json-y) diff --git a/python/qemu/.flake8 b/python/.flake8 similarity index 100% rename from python/qemu/.flake8 rename to python/.flake8 diff --git a/python/Makefile.include b/python/Makefile.include new file mode 100644 index 0000000000..917808e2f1 --- /dev/null +++ b/python/Makefile.include @@ -0,0 +1,33 @@ +# -*- Mode: makefile -*- + +PYLIB_VENV_DIR=$(BUILD_DIR)/venv/cqa +PYLIB_VENV_REQ=$(SRC_PATH)/python/requirements.cqa.txt + +$(PYLIB_VENV_DIR): $(PYLIB_VENV_REQ) + $(call quiet-command, \ + $(PYTHON) -m venv $@, \ + VENV, $@) + $(call quiet-command, \ + $(PYLIB_VENV_DIR)/bin/python3 -m pip -q install -r $(PYLIB_VENV_REQ), \ + PIP, $(PYLIB_VENV_REQ)) + $(call quiet-command, touch $@) + +pylib-venv: $(PYLIB_VENV_DIR) + +check-python: pylib-venv + $(call quiet-command, cd $(SRC_PATH)/python && \ + $(PYLIB_VENV_DIR)/bin/python3 -m flake8 qemu, \ + FLAKE8, \ + $(SRC_PATH)/python/qemu \ + ) + $(call quiet-command, cd $(SRC_PATH)/python && \ + $(PYLIB_VENV_DIR)/bin/python3 -m pylint qemu, \ + PYLINT, \ + $(SRC_PATH)/python/qemu \ + ) + $(call quiet-command, cd $(SRC_PATH)/python && \ + $(PYLIB_VENV_DIR)/bin/python3 -m mypy \ + --strict --no-error-summary qemu, \ + MYPY, \ + "--strict $(SRC_PATH)/python/qemu" \ + ) diff --git a/python/qemu/pylintrc b/python/pylintrc similarity index 99% rename from python/qemu/pylintrc rename to python/pylintrc index 5d6ae7367d..65b4545a6b 100644 --- a/python/qemu/pylintrc +++ b/python/pylintrc @@ -16,6 +16,7 @@ disable=too-many-arguments, too-many-public-methods, [REPORTS] +score=no [REFACTORING] diff --git a/python/requirements.cqa.txt b/python/requirements.cqa.txt new file mode 100644 index 0000000000..dbf45984bc --- /dev/null +++ b/python/requirements.cqa.txt @@ -0,0 +1,3 @@ +pylint==2.5.3 +mypy==0.782 +flake8==3.8.3
Move pylintrc and flake8 up to the root of the python folder where they're the most useful. Add a requirements.cqa.txt file to house the requirements necessary to build a venv sufficient for running code quality analysis on the python folder. Add a makefile that will build the venv and run the tests. Signed-off-by: John Snow <jsnow@redhat.com> --- Makefile | 1 + python/{qemu => }/.flake8 | 0 python/Makefile.include | 33 +++++++++++++++++++++++++++++++++ python/{qemu => }/pylintrc | 1 + python/requirements.cqa.txt | 3 +++ 5 files changed, 38 insertions(+) rename python/{qemu => }/.flake8 (100%) create mode 100644 python/Makefile.include rename python/{qemu => }/pylintrc (99%) create mode 100644 python/requirements.cqa.txt