Re: [PATCH 16/37] qapi: establish mypy type-checking baseline

[John Snow <jsnow@redhat.com>]

On 9/21/20 4:05 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 9/18/20 7:55 AM, Markus Armbruster wrote:
>>> Ignorant question: why does this come after PATCH 13 "qapi/common.py:
>>> add notational type hints", but before all the other patches adding type
>>> hints?
>>> John Snow <jsnow@redhat.com> writes:
>>>
>>>> Fix two very minor issues, and then establish a mypy type-checking
>>>> baseline.
>>>>
>>>> Like pylint, this should be run from the folder above:
>>>>
>>>>    > mypy --config-file=qapi/mypy.ini qapi/
>>> I get:
>>>       $ mypy --config-file qapi/mypy.ini qapi
>>>       qapi/mypy.ini: [mypy]: Strict mode is not supported in configuration files: specify individual flags instead (see 'mypy -h' for the list of flags enabled in strict mode)
>>>       qapi/types.py:29: error: Need type annotation for 'objects_seen' (hint: "objects_seen: Set[<type>] = ...")
>>>       Found 1 error in 1 file (checked 18 source files)
>>> Is this expected?
>>> In case it matters:
>>>       $ mypy --version
>>>       mypy 0.761
>>>
>>
>> (Warning; FiSH and stgit ahead)
>>
>> cd ~/src/qemu/scripts
>> pipenv --python 3.6
>> pipenv shell
>> pip install pylint flake8
>>
>> ### Testing mypy 0.770
>>
>> pip install mypy==0.770
>> stg goto qapi-establish-mypy-type
>>
>> while true; and flake8 qapi/; and pylint --rcfile=qapi/pylintrc qapi/;
>> and mypy --config-file=qapi/mypy.ini qapi/; and stg push; end
>>
>> pip uninstall mypy
>>
>> ###
>>
>>
>>
>> 0.782 - OK
>> 0.770 - OK
>> 0.760 - FAIL, Fixable*
>> 0.750 - OK*
>> 0.740 - OK*
>> 0.730 - OK*
>> 0.720 - OK*
>> 0.710 - OK** (Does not recognize 'no_implicit_reexport' option)
>> 0.700 - OK*** (Not compatible with bleeding edge pylint/flake8)
>> 0.670 - OK***
>> 0.660 - OK***
>> 0.650 - OK***
>> 0.641 - OK***
>> 0.630 - Fails again.
>>
>>
>>
>> 0.760 doesn't support strict in the config file (It needs component
>> options), and it wants a few extra annotations where its inference
>> power is weaker. Well, easy enough to fix up.
>>
>> 0.630 fails again and insists that __init__ should have a return type
>> annotation of None. Modern mypy is smart enough to know that's what
>> that type is supposed to be. Arbitrarily, this is my cutoff for what
>> seems reasonable to even want to support.
>>
>> I still find the lack of "strict=true" in the config file irritating
>> and might ask to target 0.770 or newer. There should be no reason we
>> can't install that in a venv for CI to chew on.
>>
>> Humbly ask I take the lazy way out and document that we support mypy
>>> = 0.770.
> 
> Our "supported build platforms" policy puts hard limits on the minimum
> versions for tools the build requires.
> 
> Mypy is not such a tool.  I hope we get to the point where we can have
> "make check" run it, but skipping the test when we don't have a
> sufficiently modern mypy feels okay to me, as long as our gating CI
> still guards the master branch.
> 

Yes, that's been my view. These tools *are* new and they *do* change 
often. Chasing a wide compatibility window with them is prohibitively 
difficult. Luckily, they're not needed for running the packages at all.

If I do add linting to 'make check', it is going to be through a virtual 
environment that deploys *specific versions* of these tools, and it will 
happen transparently. The VM tests already do this.

--js