Re: [PATCH 4/5] python: add .gitignore

From: John Snow <jsnow@redhat.com>
To: Thomas Huth <thuth@redhat.com>, qemu-devel@nongnu.org
Cc: "Fam Zheng" <fam@euphon.net>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH 4/5] python: add .gitignore
Date: Wed, 28 Oct 2020 14:33:27 -0400	[thread overview]
Message-ID: <3c46ae21-c8fe-41ff-ac42-7e42524c857d@redhat.com> (raw)
In-Reply-To: <f9ea6ce1-f464-0376-29ac-6071f680e63b@redhat.com>

On 10/28/20 4:13 AM, Thomas Huth wrote:
> On 27/10/2020 23.38, John Snow wrote:
>> Ignore build and package output (build, dist, qemu.egg-info);
>> effectively these are "in-tree" builds of a kind.
> 
> Since we recently moved away from in-tree builds, could these also be made
> out-of-tree only?
> 
>   Thomas
> 

# Summary

This wound up being controversial, so let's go over exactly what 
artifacts I have ignored here, and why I have done so. This email gets 
long, but please stick with me for at least this first Summary section.

One thing I would like to immediately clear up is that these artifacts 
are not related to a "QEMU build" in any way, shape or form; i.e., 
building QEMU does not cause these artifacts to exist.

For clarity: neither "make" nor "make check" from the root nor the build 
root will cause any changes to the ./python source tree. These artifacts 
come from explicit actions taken during manual testing/development in 
the ./python directory.

I use a Makefile in the ./python directory to canonize the expected 
invocations for a few common operations I anticipate developers wanting 
to take; these actions occur outside of "the QEMU build".

Those actions, and the expected artifacts they create, are:

- Running 'make check' inside ./python; AKA
   running 'pytest' (no arguments.)
   (.mypy_cache, .pytest_cache, __pycache__, possibly *.pyc)

- Running 'make venv' inside ./python; AKA
   running PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
   (.venv, qemu.egg-info/, build/)

- Running 'make venv-check' inside ./python; AKA
   running 'make venv; pipenv run make check'
   (All of the above from both categories)

- Running various PyPI distribution commands; not in the Makefile
   (e.g. python3 setup.py sdist bdist_wheel)
   (build/, dist/)

- Using the Pycharm IDE to edit, run, or test python files:
   (.idea/)

While these do not happen during a (QEMU) make/make check, they DO 
happen as a result of my gitlab job, which simply does the dumbest 
possible thing and navigates to the source ./python directory and runs 
"make venv-check".

This was presumed "safe", because the gitlab output is not committed 
back into the container, does not affect the subsequent build, and has 
its output discarded after the job completes.

Read on below for more detailed information on what each file is, what 
creates it, what it's useful for, and why you might come to have these 
files in your source tree during development.

# make check

running "make check" here simply invokes "pytest". This causes 
.pytest_cache to exist where you ran it. The tests located in tests/ 
will run mypy (.mypy_cache), flake8, pylint and isort.

The act of running or importing files here may or may not cause 
__pycache__ and *.pyc files to exist based on the caller's environment.

All of the files created by running the linter are normal confetti you 
might expect to incur from running such programs; i.e. if you run mypy 
or pytest separately, of your own volition, you will encounter these 
files being made for you.

It is normal and expected for developers to want to run the linters 
during development to ensure adherence to the coding standards, so it is 
normal and expected to see these files being created in the source tree 
-- not during a build, but during development.

We ignore __pycache__ and *.pyc in the root tree too, this is just 
extending the concept to more python tooling that may litter the tree.

# make venv

This is an alias for "pipenv sync --dev", which uses the pipenv tool to 
actualize a very specific virtual environment based on the contents in 
Pipfile.lock.

It normally creates a virtual environment somewhere like:
  /home/jsnow/.local/share/virtualenvs/python-Z09Et8eW

but you can configure it to look in $CWD/.venv instead. There are no 
other options I am aware of! Your $CWD must include the Pipfile.lock file.

Let's assume that we will want to create a venv in two distinct contexts:

(1) For the purposes of iterative development; as an object we want to 
keep long-term (conjured explicitly by the developer), and

(2) For the purposes of a continuous integration check; a build check, 
or some other automatic invocation not explicitly conjured by the developer.

For the first, it's okay to store in ~/.local/share/ etc, and it's okay 
to store it in the source tree too -- in both cases, you'll have one 
venv per source tree. No problem. (And reminder, because this python 
virtual environment is tied to the python *source* and has no 
relationship whatsoever to a configuration of a QEMU build, a 1:1 
relationship from venv:source is OK. We do not need (or want) a 1:1 
relationship with configurations.)

For the second, it would be best if we didn't taint the user's 
environment. In this case, constraining it to where the the venv was 
requested is the best choice available. There are no hooks in this patch 
series that will invoke the creation of this venv in the source tree 
when a user types "make" or "make check" in their root or their build root.

So: ignoring .venv is for the sake of the python developer, but does not 
imply this file will exist for those not developing python.

Lastly, The qemu.egg-info file is an artifact of installing the source 
package itself into the virtual environment in "editable" mode. This is 
an expected artifact of development, but won't occur during a build.

A trick I explicitly support is `cd ~/src/qemu/python; pip install -e .` 
-- this installs a python package to your current environment (wherever 
it is; It can be one of your explicit creation) that simply installs 
symlinks to your git source tree -- this is a feature -- so that while 
updating the python source, your installed package always reflects the 
latest version without having to "re-install" the package to test it.

Therefore, it's reasonable and expected to have a 'qemu.egg-info/' 
folder in your source tree: it means you've installed this package 
somewhere. This doesn't happen by accident!

# make venv-check

This is a hook that runs both things: it creates a venv, then runs the 
tests inside that venv. It will create all of the confetti from both 
above processes.

If you made a build directory and configured QEMU, the configuration 
script will have copied the python directory into the build directory. 
You can run "make venv-check" from in there to avoid disturbing your 
source tree.

However, since running configure isn't necessary to check the python 
code, you can just run it straight from the source tree.

# .idea

This is the pycharm IDE folder for managing project settings. If you use 
pycharm to edit the Python code (and I encourage you to try if you wish 
to contribute Python code), you will see this directory created.

# build

This directory will be created as a result of engaging with pipenv.

It will show up under any of the following:

pipenv sync
pipenv lock
pipenv install

I believe this happens as a side-effect of installing the qemu package 
in editable mode, however:

`pip install -e .` does not create such a directory. I'm not sure which 
step is creating it and what it's used for, to be crassly honest. It 
appears safe to delete afterwards.

Actually, you can even create a read-only build directory owned by root, 
and pipenv will ... work the same, and never write into that directory. 
What's going on? Someone else's bug, I guess.

(It also shows up if you were to type "python3 setup.py bdist_wheel", 
which is a packaging step you would only type if you were preparing to 
build and package this code to upload it to PyPI.)

# dist

Only shows up if you type 'python3 setup.py sdist'. No reason to do this 
unless you're building a source distribution of the Python package alone 
to upload to somewhere else. Intentional developer action.

So ... long story short: I think these files are legitimate; I am sorry 
to have alarmed people by calling it an "in-tree build", they're not 
related to the QEMU build.

--js