On Thu, May 19, 2022, 4:25 AM Daniel P. Berrangé <berra...@redhat.com> wrote:
> On Thu, May 19, 2022 at 09:54:56AM +0200, Kevin Wolf wrote: > > Am 18.05.2022 um 20:21 hat John Snow geschrieben: > > > To wire it up to "make check" by *default*, I believe I need to expand > the > > > configure script to poll for certain requisites and then create some > > > wrapper script of some kind that only engages the python tests if the > > > requisites were met ... and I lose some control over the mypy/pylint > > > versioning windows. I have to tolerate a wider versioning, or it'll > never > > > get run in practice. > > > > > > I have some reluctance to doing this, because pylint and mypy change so > > > frequently that I don't want "make check" to fail spuriously in the > future. > > > > > > (In practice, these failures occur 100% of the time when I am on > vacation.) > > > > So we seem to agree that it's something that we do expect to fail from > > time to time. Maybe this is how I could express my point better: If it's > > a hard failure, it should fail as early as possible - i.e. ideally > > before the developer sends a patch, but certainly before failing a pull > > request. > > At least with pylint we can make an explicit list of which lint > checks we want to run, so we should not get new failures when a > new pylint is released. If there are rare cases where we none > the less see a new failure from a new release, then so be it, > whoever hits it first can send a patch. IOW, I think we should > just enable pylint all the time with a fixed list of tests we > care about. Over time we can enable more of its checks when > desired. > Yeh, this might help a bit. If we use system packages by default, we'll also generally avoid using bleeding edge packages and I'll (generally) catch those myself via check-tox before people run into them organically. > I don't know enough about mypy to know if it can provide similar > level of control. Possibly the answer for "should we run it by default" > will be different for pylint vs mypy. > Yeah, we can probably do different things. mypy is actually much more stable than pylint IMO, it's probably actually okay to just let that one behave as-is. (I know I have a fix for 0.950 in my recent rfc series, but anecdotally I feel mypy changes behavior a lot less often than pylint. isort and flake8 have basically never ever broken on update for me, either.) Still, none of this is all that different from the case where > new GCC or CLang are released and developers find new warnings > have arrived. People just send patches when they hit this. > Given python is a core part of QEMU's dev tooling, I think it > is reasonable to expect developers to cope with this for python > too, as long as the frequency of problems is not unreasonably > high. > To some extent, though it's still a bummer to get warnings and errors that have nothing to do with your changes. I have made sure I test a wide matrix to the best of my ability, so it should be fine. I guess I'm just super conservative about it ... (Well, and even when I had the check-tox test set to allow failure, the yellow exclamation mark still annoyed people. I'm just keen to avoid more nastygrams.) > > > That said ... maybe I can add a controlled venv version of > "check-python" > > > and just have a --disable-check-python or something that spec files > can opt > > > into. Maybe that will work well enough? > > > > > > i.e. maybe configure can check for the presence of pip, the python venv > > > module (debian doesnt ship it standard...), and PyPI connectivity and > if > > > so, enables the test. Otherwise, we skip it. > > > > I think this should work. If detecting the right environment is hard, I > > don't think there is even a requirement to do so. You can make > > --enable-check-python the default and if people don't want it, they can > > explicitly disable it. (I understand that until you run 'make check', it > > doesn't make a difference anyway, so pure users would never have to > > change the option, right?) > > I think it should just be the default too. Contributors have to accept > that python is a core part of our project and we expect such code to > pass various python quality control tests, on the wide variety of OS > platforms we run on. > I meant that I'd have the default be "auto", but if you're arguing for the default to be "on", I suppose I could. I have a weak preference for keeping the min requisites for a no-option configure set small. This should be trivial to change in either direction, though. The requisites aren't steep: you just need python and the venv stdlib module. If you have python, you meet that requisite on every platform except debian/ubuntu, which ships venv separately. In practice, it probably will be enabled for most people by default. > > > Got it. I'll see what I can come up with that checks the boxes for > > > everyone, thanks for clarifying yours. > > > > > > I want to make everything "just work" but I'm also afraid of writing > too > > > much magic crap that could break and frustrate people who have no > desire to > > > understand python packaging junk, so I'm trying to balance that. > > > > Yes, sounds like we need to find some balance there. Test infrastructure > > breaking locally for no obvious reason can be quite frustrating. But > > sending a patch and getting it queued, only to be notified that it's > > dropped again because of a mypy problem two weeks later when the > > maintainer sends the pull request, can be equally (if not even more) > > frustrating. > > The other benefit to having the checks turned on by default for everyone > is that it removes John as the centralized point responsible for finding, > investigating and addressing python style issues, and distributes it > amongst all the QEMU contributors. > Haha. I won't hold my breath for anyone else to have patience enough to track it down, but I appreciate the sentiment :) Thanks for the feedback. I'm still working on my RFC for the actual unit testing venv but ran into some difficulties with vm tests and Avocado tests being a little flaky, and testing has been slow. So: v2 RFC coming up soon and I'll put code and test results to all these worrrrrrrrrds. --js
