Message ID | 20201027223815.159802-5-jsnow@redhat.com |
---|---|
State | New |
Headers | show |
Series | python: add linters to gitlab CI | expand |
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
Thomas Huth <thuth@redhat.com> writes: > 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? The build should only write to the per-build spaces: the build tree, per-build scratch in /tmp, ... Writing to shared space such as the source tree can break parallel independent builds. I consider that a bug.
On Wed, Oct 28, 2020 at 10:16:33AM +0100, Markus Armbruster wrote: > Thomas Huth <thuth@redhat.com> writes: > > > 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? > > The build should only write to the per-build spaces: the build tree, > per-build scratch in /tmp, ... Writing to shared space such as the > source tree can break parallel independent builds. I consider that a > bug. Or worse it will simply fail when contributors have the source tree as a read-only filesystem. Regards, Daniel
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 > I'm not sure to be really honest with you. For "developer installs", I think the answer is *no*, it has to be in-tree. Basically you are installing this directory as a living package, as the live copy. It adds some metadata to the folder to do that. No way around it. I'll investigate, but I have doubts. --js
On 10/28/20 5:16 AM, Markus Armbruster wrote: > Thomas Huth <thuth@redhat.com> writes: > >> 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? > > The build should only write to the per-build spaces: the build tree, > per-build scratch in /tmp, ... Writing to shared space such as the > source tree can break parallel independent builds. I consider that a > bug. > It's not really a "build" in that traditional sense, but if you were to execute "make venv-check" in parallel, I'm not confident it would work right. Don't do that, I guess. This has nothing to do with QEMU's build step. We don't need to "build" or "install" this package to use it during QEMU builds or (most) testing. We *do* need to install it to a virtual environment to test it with an explicit set of linter packages, though. See also: why do we ignore *.pyc and __pycache__ files in the whole tree? These are in effect build artifacts too. I'm not sure I would know how to avoid those being created. Maybe it's possible? but... I don't think this is a problem that we have to solve, actually. OK, all that whining aside, I will give it a legitimate try. I just wanted to prepare you for disappointment. I might be able to move build/ and dist/, but I have doubts that anything can reasonably be done about qemu.egg-info, __pycache__, .mypy_cache, or the like. --js
On Wed, Oct 28, 2020 at 09:39:07AM -0400, John Snow wrote: > On 10/28/20 5:16 AM, Markus Armbruster wrote: > > Thomas Huth <thuth@redhat.com> writes: > > > > > 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? > > > > The build should only write to the per-build spaces: the build tree, > > per-build scratch in /tmp, ... Writing to shared space such as the > > source tree can break parallel independent builds. I consider that a > > bug. > > > > It's not really a "build" in that traditional sense, but if you were to > execute "make venv-check" in parallel, I'm not confident it would work > right. Don't do that, I guess. > > This has nothing to do with QEMU's build step. We don't need to "build" or > "install" this package to use it during QEMU builds or (most) testing. We > *do* need to install it to a virtual environment to test it with an explicit > set of linter packages, though. > > See also: why do we ignore *.pyc and __pycache__ files in the whole tree? > These are in effect build artifacts too. I'm not sure I would know how to > avoid those being created. Maybe it's possible? but... I don't think this is > a problem that we have to solve, actually. You can disable pyc files with export PYTHONDONTWRITEBYTECODE=dontmesswithmysourcedir https://docs.python.org/3/using/cmdline.html#envvar-PYTHONDONTWRITEBYTECODE Regards, Daniel
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
diff --git a/python/.gitignore b/python/.gitignore new file mode 100644 index 000000000000..78c522768bc1 --- /dev/null +++ b/python/.gitignore @@ -0,0 +1,9 @@ +*.pyc +.idea/ +.mypy_cache/ +.pytest_cache/ +.venv/ +__pycache__/ +build/ +dist/ +qemu.egg-info/
Ignore build and package output (build, dist, qemu.egg-info); effectively these are "in-tree" builds of a kind. Ignore miscellaneous cached python confetti (__pycache__, *.pyc, .mypy_cache). Ignore .idea (pycharm) and .venv (pipenv et al). Signed-off-by: John Snow <jsnow@redhat.com> --- python/.gitignore | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 python/.gitignore