On Tue, May 25, 2021 at 01:14:50PM -0400, John Snow wrote: > On 5/24/21 10:33 PM, Cleber Rosa wrote: > > On Wed, May 12, 2021 at 07:12:22PM -0400, John Snow wrote: > > > Add short readmes to python/, python/qemu/, python/qemu/machine, > > > python/qemu/qmp, and python/qemu/utils that explain the directory > > > hierarchy. These readmes are visible when browsing the source on > > > e.g. gitlab/github and are designed to help new developers/users quickly > > > make sense of the source tree. > > > > > > They are not designed for inclusion in a published manual. > > > > > > Signed-off-by: John Snow > > > --- > > > python/README.rst | 41 ++++++++++++++++++++++++++++++++++ > > > python/qemu/README.rst | 8 +++++++ > > > python/qemu/machine/README.rst | 9 ++++++++ > > > python/qemu/qmp/README.rst | 9 ++++++++ > > > python/qemu/utils/README.rst | 7 ++++++ > > > 5 files changed, 74 insertions(+) > > > create mode 100644 python/README.rst > > > create mode 100644 python/qemu/README.rst > > > create mode 100644 python/qemu/machine/README.rst > > > create mode 100644 python/qemu/qmp/README.rst > > > create mode 100644 python/qemu/utils/README.rst > > > > > > diff --git a/python/README.rst b/python/README.rst > > > new file mode 100644 > > > index 00000000000..7a0dc5dff4a > > > --- /dev/null > > > +++ b/python/README.rst > > > @@ -0,0 +1,41 @@ > > > +QEMU Python Tooling > > > +=================== > > > + > > > +This directory houses Python tooling used by the QEMU project to build, > > > +configure, and test QEMU. It is organized by namespace (``qemu``), and > > > +then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc). > > > + > > > +``setup.py`` is used by ``pip`` to install this tooling to the current > > > +environment. ``setup.cfg`` provides the packaging configuration used by > > > +setup.py in a setuptools specific format. You will generally invoke it > > > > For consistency, ``setup.py`` here? Also, I guess ``setuptools`` as it > > falls in the same category of ``pip``. > > > > Kinda-sorta, but `pip` is a command line executable and setuptools isn't. > Along those lines I'll fix setup.py but leave setuptools as-is. > OK, fair enough. > > > +by doing one of the following: > > > + > > > +1. ``pip3 install .`` will install these packages to your current > > > + environment. If you are inside a virtual environment, they will > > > + install there. If you are not, it will attempt to install to the > > > + global environment, which is not recommended. > > > > Maybe some **emphasis** on **not**? > > > > Sure :) > > > > + > > > +2. ``pip3 install --user .`` will install these packages to your user's > > > + local python packages. If you are inside of a virtual environment, > > > + this will fail. > > > + > > > > Maybe note that, if you are inside of a virtual environment, option #1 > > will probably be what users doing "--user" in a venv actually want. > > > > Yes. It's frequently annoying how this works, because it's hard to relay > succinctly :) > > I think at least newer versions of pip give you good warnings when you use > --user for virtual environments at least. > True. > > > +If you append the ``-e`` argument, pip will install in "editable" mode; > > > +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. > > > + > > > +See `Installing packages using pip and virtual environments > > > +`_ > > > +for more information. > > > + > > > + > > > +Files in this directory > > > +----------------------- > > > + > > > +- ``qemu/`` Python package source directory. > > > +- ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org. > > > +- ``README.rst`` you are here! > > > +- ``VERSION`` contains the PEP-440 compliant version used to describe > > > + this package; it is referenced by ``setup.cfg``. > > > +- ``setup.cfg`` houses setuptools package configuration. > > > +- ``setup.py`` is the setuptools installer used by pip; See above. > > > > Not only used by pip... but I understand the reason for limiting the > > amount of information given here. > > > > Yes ... suggesting broadly that I don't really support using > setuptools/setup.py alone to install the package, but instead expect and > consider 'pip' to be the canonical/supported interface. > > There are sometimes minor differences between how they handle things, so I > wanted less emphasis on setuptools et al. > Sure, I can definitely side with you here. > > > diff --git a/python/qemu/README.rst b/python/qemu/README.rst > > > new file mode 100644 > > > index 00000000000..d04943f526c > > > --- /dev/null > > > +++ b/python/qemu/README.rst > > > @@ -0,0 +1,8 @@ > > > +QEMU Python Namespace > > > +===================== > > > + > > > +This directory serves as the root of a `Python PEP 420 implicit > > > +namespace package `_. > > > + > > > +Each directory below is assumed to be an installable Python package that > > > +is available under the ``qemu.`` namespace. > > > diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst > > > new file mode 100644 > > > index 00000000000..ac2b4fffb42 > > > --- /dev/null > > > +++ b/python/qemu/machine/README.rst > > > @@ -0,0 +1,9 @@ > > > +qemu.machine package > > > +==================== > > > + > > > +This package provides core utilities used for testing and debugging > > > +QEMU. It is used by the iotests, vm tests, acceptance tests, and several > > > +other utilities in the ./scripts directory. It is not a fully-fledged > > > +SDK and it is subject to change at any time. > > > + > > > +See the documentation in ``__init__.py`` for more information. > > > diff --git a/python/qemu/qmp/README.rst b/python/qemu/qmp/README.rst > > > new file mode 100644 > > > index 00000000000..c21951491cf > > > --- /dev/null > > > +++ b/python/qemu/qmp/README.rst > > > @@ -0,0 +1,9 @@ > > > +qemu.qmp package > > > +================ > > > + > > > +This package provides a library used for connecting to and communicating > > > +with QMP servers. It is used extensively by iotests, vm tests, > > > +acceptance tests, and other utilities in the ./scripts directory. It is > > > +not a fully-fledged SDK and is subject to change at any time. > > > + > > > +See the documentation in ``__init__.py`` for more information. > > > diff --git a/python/qemu/utils/README.rst b/python/qemu/utils/README.rst > > > new file mode 100644 > > > index 00000000000..975fbf4d7de > > > --- /dev/null > > > +++ b/python/qemu/utils/README.rst > > > @@ -0,0 +1,7 @@ > > > +qemu.utils package > > > +================== > > > + > > > +This package provides miscellaneous utilities used for testing and > > > +debugging QEMU. It is used primarily by the vm and acceptance tests. > > > + > > > +See the documentation in ``__init__.py`` for more information. > > > -- > > > 2.30.2 > > > > > > > > > > With the ``setup.py`` and ``setuptools`` for consistency sake > > mentioned in my first comment, all other comments are suggestions, so: > > > > I took one of those two review comments, and acted on one of two of your > suggestions. > > > Reviewed-by: Cleber Rosa > > > > So technically I didn't meet your criteria for taking this RB :) Let me know > if I can still apply it: > > diff --git a/python/README.rst b/python/README.rst > index 7a0dc5dff4a..38b0c83f321 100644 > --- a/python/README.rst > +++ b/python/README.rst > @@ -7,17 +7,17 @@ then by package (e.g. ``qemu/machine``, ``qemu/qmp``, > etc). > > ``setup.py`` is used by ``pip`` to install this tooling to the current > environment. ``setup.cfg`` provides the packaging configuration used by > -setup.py in a setuptools specific format. You will generally invoke it > -by doing one of the following: > +``setup.py`` in a setuptools specific format. You will generally invoke > +it by doing one of the following: > OK. > 1. ``pip3 install .`` will install these packages to your current > environment. If you are inside a virtual environment, they will > install there. If you are not, it will attempt to install to the > - global environment, which is not recommended. > + global environment, which is **not recommended**. > Nice :) > 2. ``pip3 install --user .`` will install these packages to your user's > local python packages. If you are inside of a virtual environment, > - this will fail. > + this will fail; you likely want the first invocation above. > > If you append the ``-e`` argument, pip will install in "editable" mode; > which installs a version of the package that installs a forwarder Cool! And, just to make sure, I'll give you not one but two: Reviewed-by: Cleber Rosa Reviewed-by: Cleber Rosa :)