Re: [PATCH v3 02/15] python: add qemu package installer

[John Snow <jsnow@redhat.com>]

On 10/28/20 3:46 PM, Cleber Rosa wrote:
> On Wed, Oct 28, 2020 at 01:02:52PM -0400, John Snow wrote:
>> On 10/28/20 11:10 AM, Cleber Rosa wrote:
>>>> +mirror <https://gitlab.com/jsnow/qemu/-/tree/python>`_, but
>>>> +contributions must be sent to the list for inclusion.
>>>
>>> IMO it's not clear if this branch/mirror is your development work, a
>>> staging area, etc.
>>>
>>
>> Fair enough. jsnow/qemu/python is intended as a staging area for patches
>> that have been vetted on-list.
>>
>> jsnow/qemu/master is a lazily-updated mirror. (I tend to update it every day
>> as part of my development process, but there are days I don't write code.)
>>
>> jsnow/qemu/python-* is development work; review branches, etc.
>>
>> I'll try to rephrase this a bit. What I want to communicate:
>>
>> - This package exists as a subfolder of a larger project
>> - I am responsible for maintaining this package, but not for the larger
>> project
>> - Please contact *me* for problems with this package
>> - Contributions should go through qemu-devel (I will gently redirect
>> contributors who may send pull requests to the qemu devel list.)
>>
> 
> OK, sounds good.  I'll look at the exact rewording on v+1.
> 
>>>> diff --git a/python/setup.cfg b/python/setup.cfg
>>>> new file mode 100755
>>>> index 0000000000..12b99a796e
>>>> --- /dev/null
>>>> +++ b/python/setup.cfg
>>>> @@ -0,0 +1,18 @@
>>>> +[metadata]
>>>> +name = qemu
>>>> +maintainer = QEMU Developer Team
>>>> +maintainer_email = qemu-devel@nongnu.org
>>>> +url = https://www.qemu.org/
>>>> +download_url = https://www.qemu.org/download/
>>>> +description = QEMU Python Build, Debug and SDK tooling.
>>>> +long_description = file:PACKAGE.rst
>>>> +long_description_content_type = text/x-rst
>>>> +classifiers =
>>>> +    Development Status :: 3 - Alpha
>>>> +    License :: OSI Approved :: GNU General Public License v2 (GPLv2)
>>>> +    Natural Language :: English
>>>> +    Operating System :: OS Independent
>>>> +
>>>
>>
>> Also ... licensing question, do we need *L*GPLv2, or does Python not have a
>> "linking exception" problem?
>>
>> I guess we can't really re-license these files anyway, so nevermind.
>>
>> (I immediately regret asking this.)
>>
> 
> I'll just pretend you never did.
> 
>>> I know the sky is the limit, but I miss the Python version classifier,
>>> at least:
>>>
>>>     Programming Language :: Python :: 3 :: Only
>>>
>>
>> Sure.
>>
>> Wait, why can you specify Python as a language? Is it possible to have
>> non-Python packages on PyPI?
>>
>> *CONCERNED*
>>
> 
> I guess it has to do with packages that can interact or serve other
> languages.  Or, that are (partially) written in another language?
> 
>>> And optionally those:
>>>
>>>     Programming Language :: Python :: 3.6
>>>     Programming Language :: Python :: 3.7
>>>     Programming Language :: Python :: 3.8
>>>     Programming Language :: Python :: 3.9
>>>
>>> Although it may be a good idea to add them along test jobs on those
>>> specific Python versions.
>>>
>>
>> Are these worth adding? I've got python_requires >= 3.6 down below. From my
>> test of a blank package upload to PyPI, it seems to display prominently:
>>
>> https://pypi.org/project/qemu/
>>
>> Is there a tangible benefit that you are aware of?
>>
> 
> AFAICT, the classifiers are about letting people search for packages
> that match a given criteria.  It's all metadata, so the benefits are
> not super tangible.  I've used those to keep track / document the
> Python versions that I know the project has been actively tested on,
> and that's the reason of my comment about (CI) test jobs.
> 

OK, let's add them alongside a tox/pytest configuration or something in 
the future when we add those versions as being supported.

I guess I can add the 3.6 version for starters, since it's explicitly 
supported?

>>>> +[options]
>>>> +python_requires = >= 3.6
>>>> +packages = find_namespace:
>>>> diff --git a/python/setup.py b/python/setup.py
>>>> new file mode 100755
>>>> index 0000000000..e93d0075d1
>>>> --- /dev/null
>>>> +++ b/python/setup.py
>>>> @@ -0,0 +1,23 @@
>>>> +#!/usr/bin/env python3
>>>> +"""
>>>> +QEMU tooling installer script
>>>> +Copyright (c) 2020 John Snow for Red Hat, Inc.
>>>> +"""
>>>> +
>>>> +import setuptools
>>>> +import pkg_resources
>>>> +
>>>> +
>>>> +def main():
>>>> +    """
>>>> +    QEMU tooling installer
>>>> +    """
>>>> +
>>>> +    # https://medium.com/@daveshawley/safely-using-setup-cfg-for-metadata-1babbe54c108
>>>> +    pkg_resources.require('setuptools>=39.2')
>>>
>>> Getting back to the "test jobs on those specific Python versions" I
>>> was really anxious that environments with Python 3.6 will fail to
>>> have such a "recent" setuptools version.
>>>
>>
>> Reasonable doubt. However, this isn't *required* to use the library (the
>> QEMU code can continue to just set PYTHONPATH or sys.path as necessary) and
>> bypasses the installer entirely.
>>
> 
> Right, but I had the impression that activating it in develop mode (at
> least) was the intention down the line.
> 

For builds at all?

I guess we could, yeah, if we wanted to start making a "build venv" and 
install it there. I think there's a lot of questions to work out there 
first, though. I am not really there yet, myself.

Right now, *right now*, all of this code is used only for testing and 
CI, so we've skirted around build requirements.

I did want to start picking up some other packages though, like 'qapi', 
for the purposes of applying the linter paradigm though ... and I 
figured I'd cross that bridge when I got there.

Right now, having a forwarder script with a sys.path hack works.

(Probably by the time we figure that out, setuptools 39 will be standard 
on all of our supported build platforms...)

>> That gives us some leeway apart from the usual version constraints; in order
>> to independently use this library outside of the QEMU tree we may impose
>> more modern setups -- as long as the minimum requirements for QEMU itself
>> don't break.
>>
> 
> OK.
> 
>> Having a modern setuptools in order to install seems like less of a problem
>> barrier; and it seemed like a good idea to make it explicitly fail instead
>> of silently doing something weird if it didn't see/understand setup.cfg.
>>
> 
> Agreed.
> 
>> (And it seems like good practice to update pip in your venv, so I think
>> we'll be OK except for the stodgiest of users, but sometimes you can't have
>> new things on old systems without learning some new tricks!)
>>
>>> CentOS 8 has that specific version, while Ubuntu 18.04 has version
>>> 39.0.  Ubuntu 20.04 has a recent enough version though.  Given that
>>> all GitLab CI moved to 20.04, this should be safe.
>>>
>>> - Cleber.
>>>
>>
>> FWIW, for the purposes of running the linters, I am using Fedora32 and the
>> python36 package.
>>
> 
> This is a minor suggestion: use CentOS 8 stock Python 3.6 packages,
> and then Fedora 33 with also stock Python 3.9.  Even though all
> tools are pinned, it's still a good idea to test at least min/max
> (if not all) Python versions.
> 

I can use CentOS, sure. I don't think it matters tremendously whose 
Python 3.6 we use. I opted for Fedora because we package old python 
interpreters on purpose, which makes it easy to say "I want Python3.6 
and not a drop older or newer."

I assume the same is true on CentOS. We can talk about this on the CI 
series; though I will likely merge these two series into one for future 
revisions.

I don't have a framework in mind for doing python version matrix testing 
yet. I guess tox is the canonical tool for that. We can cross that 
bridge when we get there, I guess. (We currently have: 0 tests for the 
qemu python library. oops!)

For the meantime, though, I think it's OK to only run the linter on 
Python 3.6: it's not a test of the package itself, it's just a specific 
environment that we use to enforce code quality. It so happens to be a 
Python 3.6 environment. Pinning it to a specific version of python there 
is useful because the linters *do* sometimes have different behavior 
depending on version; due largely in part to changes in the stdlib 
typing library.

> - Cleber.
> 
>>>> +
>>>> +    setuptools.setup()
>>>> +
>>>> +
>>>> +if __name__ == '__main__':
>>>> +    main()
>>>> -- 
>>>> 2.26.2
>>>>
>>