| Message ID | 20210512231241.2816122-21-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | python: create installable package | expand |
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. So my suggestion is: consider better exposing the fact that this is available (a documentation section perhaps). > Signed-off-by: John Snow <jsnow@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 <crosa@redhat.com>
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.) > So my suggestion is: consider better exposing the fact that this is > available (a documentation section perhaps). > >> Signed-off-by: John Snow <jsnow@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 <crosa@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.
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 <jsnow@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 <crosa@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 <crosa@redhat.com>
diff --git a/python/Pipfile b/python/Pipfile index fb23455eadd..e7acb8cefa4 100644 --- a/python/Pipfile +++ b/python/Pipfile @@ -4,10 +4,7 @@ url = "https://pypi.org/simple" verify_ssl = true [dev-packages] -flake8 = ">=3.6.0" -isort = ">=5.1.2" -mypy = ">=0.770" -pylint = ">=2.7.0" +qemu = {editable = true, extras = ["devel"], path = "."} [packages] qemu = {editable = true,path = "."} diff --git a/python/Pipfile.lock b/python/Pipfile.lock index 5d3de43609d..18f3bba08f2 100644 --- a/python/Pipfile.lock +++ b/python/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "986164b4c690953890066f288b48c3d84c63df86fc8fa30a26e9001d5b0968e0" + "sha256": "eff562a688ebc6f3ffe67494dbb804b883e2159ad81c4d55d96da9f7aec13e91" }, "pipfile-spec": 6, "requires": { @@ -35,7 +35,7 @@ "sha256:12d05ab02614b6aee8df7c36b97d1a3b2372761222b19b58621355e82acddcff", "sha256:78873e372b12b093da7b5e5ed302e8ad9e988b38b063b61ad937f26ca58fc5f0" ], - "index": "pypi", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'", "version": "==3.9.0" }, "importlib-metadata": { @@ -51,7 +51,7 @@ "sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6", "sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d" ], - "index": "pypi", + "markers": "python_version >= '3.6' and python_version < '4.0'", "version": "==5.8.0" }, "lazy-object-proxy": { @@ -114,7 +114,7 @@ "sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c", "sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb" ], - "index": "pypi", + "markers": "python_version >= '3.5'", "version": "==0.812" }, "mypy-extensions": { @@ -145,9 +145,13 @@ "sha256:0e21d3b80b96740909d77206d741aa3ce0b06b41be375d92e1f3244a274c1f8a", "sha256:d09b0b07ba06bcdff463958f53f23df25e740ecd81895f7d2699ec04bbd8dc3b" ], - "index": "pypi", + "markers": "python_version ~= '3.6'", "version": "==2.7.2" }, + "qemu": { + "editable": true, + "path": "." + }, "toml": { "hashes": [ "sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b", diff --git a/python/setup.cfg b/python/setup.cfg index 8a2c200a581..9d941386921 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -19,6 +19,15 @@ classifiers = python_requires = >= 3.6 packages = find_namespace: +[options.extras_require] +# Run `pipenv lock` when changing these requirements. +devel = + flake8 >= 3.6.0 + isort >= 5.1.2 + mypy >= 0.770 + pylint >= 2.7.0 + + [flake8] extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's exclude = __pycache__,
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. Signed-off-by: John Snow <jsnow@redhat.com> --- python/Pipfile | 5 +---- python/Pipfile.lock | 14 +++++++++----- python/setup.cfg | 9 +++++++++ 3 files changed, 19 insertions(+), 9 deletions(-)