On Tue, May 25, 2021 at 01:43:42PM -0400, John Snow wrote: > On 5/25/21 12:13 PM, Cleber Rosa wrote: > > On Wed, May 12, 2021 at 07:12:36PM -0400, John Snow wrote: > > > setuptools doesn't have a formal understanding of development requires, > > > but it has an optional feataures section. Fine; add a "devel" feature > > > and add the requirements to it. > > > > > > To avoid duplication, we can modify pipenv to install qemu[devel] > > > instead. This enables us to run invocations like "pip install -e > > > .[devel]" and test the package on bleeding-edge packages beyond those > > > specified in Pipfile.lock. > > > > > > Importantly, this also allows us to install the qemu development > > > packages in a non-networked mode: `pip3 install --no-index -e .[devel]` > > > will now fail if the proper development dependencies are not already > > > met. This can be useful for automated build scripts where fetching > > > network packages may be undesirable. > > > > > > > This is a fairly exotic feature of setuptools, with very very few > > packages that I know about using it. With most users (I believe) > > relying on pipenv to get the exact packages, the setuptools/pip use > > case may fall into obscurity IMO. > > > > Fair enough. > > The intent is: > > - Pipenv is more for CI, to deploy a consistent set of frozen packages that > are known to behave in an extremely stable manner. My hope is to avoid > breaking changes introduced unknowingly by pylint et al. > > - pip install qemu[devel] is intended more for external/normal use by > developers. It grabs the latest and greatest and it may indeed break as > dependencies change beyond my awareness. > > > Some packages like aiohttp use that optional dependency feature to install > optional modules -- `pip install aiohttp[speedups]` installs optional > dependencies that allow that module to work much faster, but aren't > required. > > Since these linting tools aren't *required* just to *use* the package, I am > doing users a courtesy by listing them as optional. That way, they aren't > pulled in when using "pip install qemu", and if I have to pin on specific > sub-versions etc, it won't include conflict dependencies for people using > other projects that DO declare a hard requirement on those packages. > > I can amend the PACKAGE.rst file to mention this usage, though it's only > useful for folks developing the package. > > (Still, part of the ploy here is to attract outside help on developing the > QEMU SDK, pull requests welcome etc, so it's worth a documentation blurb for > now.) >
Yep, I agree with your reasoning here. I just felt like an extra bit of documentation would do the trick. > > So my suggestion is: consider better exposing the fact that this is > > available (a documentation section perhaps). > > > > > Signed-off-by: John Snow <js...@redhat.com> > > > --- > > > python/Pipfile | 5 +---- > > > python/Pipfile.lock | 14 +++++++++----- > > > python/setup.cfg | 9 +++++++++ > > > 3 files changed, 19 insertions(+), 9 deletions(-) > > > > > > > Either way, > > > > Reviewed-by: Cleber Rosa <cr...@redhat.com> > > > > Thanks! I am taking your R-B and I have applied the following diff. > > Note that the PACKAGE.rst blurb references qemu[devel] instead because the > PACKAGE.rst file is what is displayed theoretically on PyPI. That exact > invocation will fail currently, because it's not on PyPI yet. > > A little weird, but I *think* it's correct. > > > diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst > index 1bbfe1b58e2..05ea7789fc1 100644 > --- a/python/PACKAGE.rst > +++ b/python/PACKAGE.rst > @@ -31,3 +31,7 @@ official `GitLab mirror > <https://gitlab.com/qemu-project/qemu>`_. > Please report bugs on the `QEMU issue tracker > <https://gitlab.com/qemu-project/qemu/-/issues>`_ and tag ``@jsnow`` in > the report. > + > +Optional packages necessary for running code quality analysis for this > +package can be installed with the optional dependency group "devel": > +``pip install qemu[devel]``. > diff --git a/python/README.rst b/python/README.rst > index bf9bbca979a..954870973d0 100644 > --- a/python/README.rst > +++ b/python/README.rst > @@ -24,6 +24,10 @@ which installs a version of the package that installs a > forwarder > pointing to these files, such that the package always reflects the > latest version in your git tree. > > +Installing ".[devel]" instead of "." will additionally pull in required > +packages for testing this package. They are not runtime requirements, > +and are not needed to simply use these libraries. > + > See `Installing packages using pip and virtual environments > > <https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/>`_ > for more information. Looks great to me. And, let me be clear about it: Reviewed-by: Cleber Rosa <cr...@redhat.com>
signature.asc
Description: PGP signature
