Hi, this is ... a fairly incomplete series about trying to get iotests
to run out of the configure-time venv. I'm looking for some feedback, so
out to the list it goes.

Primarily, I'm having doubts about these points:

1) I think I need something like "mkvenv install" in the first patch,
   but mkvenv.py is getting pretty long...

2) Is there a way to optimize the speed for patch #2? Maybe installing
   this package can be skipped until it's needed, but that means that
   things like iotest's ./check might get complicated to support that.

3) I cheated quite a bit in patch 4 to use the correct Python to launch
   iotests, but I'm wondering if there's a nicer way to solve this
   more *completely*.

John Snow (6):
  experiment: add mkvenv install
  build, tests: Add qemu in-tree packages to pyvenv at configure time.
  iotests: get rid of '..' in path environment output
  iotests: use the correct python to run linters
  iotests: use pyvenv/bin/python3 to launch child test processes
  iotests: don't add qemu.git/python to PYTHONPATH

 configure                     | 31 +++++++++++++++++++++++++++
 python/scripts/mkvenv.py      | 40 +++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/linters.py |  2 +-
 tests/qemu-iotests/testenv.py | 21 ++++++++++++------
 4 files changed, 87 insertions(+), 7 deletions(-)

-- 
2.40.1

Il mer 21 giu 2023, 02:21 John Snow <jsnow@redhat.com> ha scritto:

> Hi, this is ... a fairly incomplete series about trying to get iotests
> to run out of the configure-time venv. I'm looking for some feedback, so
> out to the list it goes.
>
> Primarily, I'm having doubts about these points:
>
> 1) I think I need something like "mkvenv install" in the first patch,
>    but mkvenv.py is getting pretty long...
>

It's not a lot of code, but using pip install from configure might also be
good enough, I don't know.

2) Is there a way to optimize the speed for patch #2? Maybe installing

   this package can be skipped until it's needed, but that means that
>    things like iotest's ./check might get complicated to support that.
>
> 3) I cheated quite a bit in patch 4 to use the correct Python to launch
>    iotests, but I'm wondering if there's a nicer way to solve this
>    more *completely*.
>

Maybe patch 4 can use distlib.scripts as well to create the check script in
the build directory? (Yes that's another mkvenv functionality...) On a
phone and don't have the docs at hand, so I am not sure. If not, your
solution is good enough.

Apart from this the only issue is the speed. IIRC having a prebuilt .whl
would fix it, I think for Meson we observed that the slow part was building
the wheel. Possibilities:

1) using --no-pep517 if that also speeds it up?

2) already removing the sources to qemu.qmp since that's the plan anyway;
and then, if you want editability you can install the package with --user
--editable, i.e. outside the venv

Paolo


> John Snow (6):
>   experiment: add mkvenv install
>   build, tests: Add qemu in-tree packages to pyvenv at configure time.
>   iotests: get rid of '..' in path environment output
>   iotests: use the correct python to run linters
>   iotests: use pyvenv/bin/python3 to launch child test processes
>   iotests: don't add qemu.git/python to PYTHONPATH
>
>  configure                     | 31 +++++++++++++++++++++++++++
>  python/scripts/mkvenv.py      | 40 +++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/linters.py |  2 +-
>  tests/qemu-iotests/testenv.py | 21 ++++++++++++------
>  4 files changed, 87 insertions(+), 7 deletions(-)
>
> --
> 2.40.1
>
>
>

On Wed, Jun 21, 2023 at 9:08 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Maybe patch 4 can use distlib.scripts as well to create the check script in the build directory? (Yes that's another mkvenv functionality...) On a phone and don't have the docs at hand, so I am not sure. If not, your solution is good enough.
>
> Apart from this the only issue is the speed. IIRC having a prebuilt .whl would fix it, I think for Meson we observed that the slow part was building the wheel. Possibilities:
>
> 1) using --no-pep517 if that also speeds it up?
>
> 2) already removing the sources to qemu.qmp since that's the plan anyway; and then, if you want editability you can install the package with --user --editable, i.e. outside the venv

Nope, it's 3 second always and 1.5 even with the wheel.

Maybe replace qemu.qmp with a wheel and leaving PYTHONPATH for the rest?

Paolo

> Paolo
>
>>
>> John Snow (6):
>>   experiment: add mkvenv install
>>   build, tests: Add qemu in-tree packages to pyvenv at configure time.
>>   iotests: get rid of '..' in path environment output
>>   iotests: use the correct python to run linters
>>   iotests: use pyvenv/bin/python3 to launch child test processes
>>   iotests: don't add qemu.git/python to PYTHONPATH
>>
>>  configure                     | 31 +++++++++++++++++++++++++++
>>  python/scripts/mkvenv.py      | 40 +++++++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/linters.py |  2 +-
>>  tests/qemu-iotests/testenv.py | 21 ++++++++++++------
>>  4 files changed, 87 insertions(+), 7 deletions(-)
>>
>> --
>> 2.40.1
>>
>>

On Thu, Jun 22, 2023 at 5:24 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Wed, Jun 21, 2023 at 9:08 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Maybe patch 4 can use distlib.scripts as well to create the check script in the build directory? (Yes that's another mkvenv functionality...) On a phone and don't have the docs at hand, so I am not sure. If not, your solution is good enough.
> >

Yeah, that's a possibility... we could "install" the iotests script.
That might keep things simple. I'll investigate it.

> > Apart from this the only issue is the speed. IIRC having a prebuilt .whl would fix it, I think for Meson we observed that the slow part was building the wheel. Possibilities:
> >
> > 1) using --no-pep517 if that also speeds it up?
> >
> > 2) already removing the sources to qemu.qmp since that's the plan anyway; and then, if you want editability you can install the package with --user --editable, i.e. outside the venv
>
> Nope, it's 3 second always and 1.5 even with the wheel.
>
> Maybe replace qemu.qmp with a wheel and leaving PYTHONPATH for the rest?
>
> Paolo
>

Hm, I guess so. It's just disappointing because I was really hoping to
be able to use "pip install" to handle dependencies like a normal
package instead of trying to shoulder that burden with an increasing
amount of custom logic that's hard for anyone but me (or you, now) to
maintain.

It kind of defeats the point of having formatted it as a package to begin with.

Maybe there's a sane way to amortize the cost of installation by not
re-creating it after every call to configure instead -- the rest of
the script is fast enough, perhaps we could default clear to *False*
from now on and use the _get_version() bits to detect if the local
internal package is already installed or not -- and if it is, just
leave it alone.

If we always install it in editable mode, and the path where it is
"installed" is what we expect it to be, it shouldn't have any problems
with being out of date.... I think. We could conceivably use the
"faux" package version the internal package has to signal when the
script needs to re-install it.

Something like that?

--js

On Thu, Jun 22, 2023 at 11:03 PM John Snow <jsnow@redhat.com> wrote:
> If we always install it in editable mode, and the path where it is
> "installed" is what we expect it to be, it shouldn't have any problems
> with being out of date.... I think. We could conceivably use the
> "faux" package version the internal package has to signal when the
> script needs to re-install it.

Stupid question, why not treat it just like avocado?

On Thu, Jun 22, 2023 at 5:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Thu, Jun 22, 2023 at 11:03 PM John Snow <jsnow@redhat.com> wrote:
> > If we always install it in editable mode, and the path where it is
> > "installed" is what we expect it to be, it shouldn't have any problems
> > with being out of date.... I think. We could conceivably use the
> > "faux" package version the internal package has to signal when the
> > script needs to re-install it.
>
> Stupid question, why not treat it just like avocado?
>

How do you mean? (i.e. installing it on-demand in reaction to "make
check-avocado"?)

On Thu, Jun 22, 2023 at 11:08 PM John Snow <jsnow@redhat.com> wrote:
>
> On Thu, Jun 22, 2023 at 5:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On Thu, Jun 22, 2023 at 11:03 PM John Snow <jsnow@redhat.com> wrote:
> > > If we always install it in editable mode, and the path where it is
> > > "installed" is what we expect it to be, it shouldn't have any problems
> > > with being out of date.... I think. We could conceivably use the
> > > "faux" package version the internal package has to signal when the
> > > script needs to re-install it.
> >
> > Stupid question, why not treat it just like avocado?
> >
>
> How do you mean? (i.e. installing it on-demand in reaction to "make
> check-avocado"?)

Yes, installing it on-demand the first time "make check-iotests" is
run, using a "depend:" keyword argument in
tests/qemu-iotests/meson.build.

BTW,

from distlib.scripts import ScriptMaker
ScriptMaker('..', '.').make('foo.py')

Seems to do the right thing as long as foo.py includes a shebang (I
tested it inside a virtual environment).

Paolo

On Thu, Jun 22, 2023 at 5:12 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Thu, Jun 22, 2023 at 11:08 PM John Snow <jsnow@redhat.com> wrote:
> >
> > On Thu, Jun 22, 2023 at 5:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >
> > > On Thu, Jun 22, 2023 at 11:03 PM John Snow <jsnow@redhat.com> wrote:
> > > > If we always install it in editable mode, and the path where it is
> > > > "installed" is what we expect it to be, it shouldn't have any problems
> > > > with being out of date.... I think. We could conceivably use the
> > > > "faux" package version the internal package has to signal when the
> > > > script needs to re-install it.
> > >
> > > Stupid question, why not treat it just like avocado?
> > >
> >
> > How do you mean? (i.e. installing it on-demand in reaction to "make
> > check-avocado"?)
>
> Yes, installing it on-demand the first time "make check-iotests" is
> run, using a "depend:" keyword argument in
> tests/qemu-iotests/meson.build.
>
> BTW,
>
> from distlib.scripts import ScriptMaker
> ScriptMaker('..', '.').make('foo.py')
>
> Seems to do the right thing as long as foo.py includes a shebang (I
> tested it inside a virtual environment).
>
> Paolo

That's possible, but it means that it will break if you run configure
and then immediately go to invoke iotests, unless we have a way to
have iotests bootstrap itself. Which I think can't be done through the
makefile, because we don't know which "make" to run in order to get
that to happen. (Or at least, I don't!)

Possibly I could teach mkvenv a new trick, like "mkvenv init iotests"
and have the mkvenv script DTRT at that point, whatever that is --
ideally exiting very quickly without doing anything.

Il gio 22 giu 2023, 23:18 John Snow <jsnow@redhat.com> ha scritto:

> Possibly I could teach mkvenv a new trick, like "mkvenv init iotests"
> and have the mkvenv script DTRT at that point, whatever that is --
> ideally exiting very quickly without doing anything.
>

Or maybe check itself should do the bootstrap if it's invoked from the
venv?!?

Paolo

>