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

[Cleber Rosa <crosa@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 6369 bytes --]

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.

> > > +[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.

> 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.

- Cleber.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]