diff mbox series

[5/9] tests: add quiet-venv-pip macro

Message ID 20220526000921.1581503-6-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series tests, python: prepare to expand usage of test venv | expand

Commit Message

John Snow May 26, 2022, 12:09 a.m. UTC
Factor out the "test venv pip" macro; rewrite the "check-venv" rule to
be a little more compact. Replace the "PIP" pseudo-command output with
"VENVPIP" to make it 1% more clear that we are talking about using pip
to install something into a venv.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile.include | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Paolo Bonzini May 26, 2022, 12:15 p.m. UTC | #1
On 5/26/22 02:09, John Snow wrote:
> Factor out the "test venv pip" macro; rewrite the "check-venv" rule to
> be a little more compact. Replace the "PIP" pseudo-command output with
> "VENVPIP" to make it 1% more clear that we are talking about using pip
> to install something into a venv.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/Makefile.include | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 839ffde876a..052d7f56e9a 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -104,13 +104,13 @@ else
>   	AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
>   endif
>   
> +quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
> +    $(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
> +    "VENVPIP","$1")
> +
>   $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
> -	$(call quiet-command, \
> -            $(PYTHON) -m venv $@, \
> -            VENV, $@)
> -	$(call quiet-command, \
> -            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
> -            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
> +	$(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
> +	$(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
>   	$(call quiet-command, touch $@)
>   
>   $(TESTS_RESULTS_DIR):

Sooner or later I'd like quiet-command to be changed to English 
descriptions like the ones currently emitted during the ninja build, but 
stuff for later.

Paolo
Paolo Bonzini May 26, 2022, 12:16 p.m. UTC | #2
On 5/26/22 02:09, John Snow wrote:
> Factor out the "test venv pip" macro; rewrite the "check-venv" rule to
> be a little more compact. Replace the "PIP" pseudo-command output with
> "VENVPIP" to make it 1% more clear that we are talking about using pip
> to install something into a venv.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/Makefile.include | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 839ffde876a..052d7f56e9a 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -104,13 +104,13 @@ else
>   	AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
>   endif
>   
> +quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
> +    $(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
> +    "VENVPIP","$1")
> +
>   $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
> -	$(call quiet-command, \
> -            $(PYTHON) -m venv $@, \
> -            VENV, $@)
> -	$(call quiet-command, \
> -            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
> -            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
> +	$(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
> +	$(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
>   	$(call quiet-command, touch $@)
>   
>   $(TESTS_RESULTS_DIR):

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
John Snow May 26, 2022, 2:17 p.m. UTC | #3
On Thu, May 26, 2022, 8:16 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/26/22 02:09, John Snow wrote:
> > Factor out the "test venv pip" macro; rewrite the "check-venv" rule to
> > be a little more compact. Replace the "PIP" pseudo-command output with
> > "VENVPIP" to make it 1% more clear that we are talking about using pip
> > to install something into a venv.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   tests/Makefile.include | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 839ffde876a..052d7f56e9a 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -104,13 +104,13 @@ else
> >       AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
> >   endif
> >
> > +quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
> > +    $(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
> > +    "VENVPIP","$1")
> > +
> >   $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
> > -     $(call quiet-command, \
> > -            $(PYTHON) -m venv $@, \
> > -            VENV, $@)
> > -     $(call quiet-command, \
> > -            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check
> install \
> > -            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
> > +     $(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
> > +     $(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
> >       $(call quiet-command, touch $@)
> >
> >   $(TESTS_RESULTS_DIR):
>
> Sooner or later I'd like quiet-command to be changed to English
> descriptions like the ones currently emitted during the ninja build, but
> stuff for later.
>
> Paolo
>

If it helps, this is a bit of a stopgap on the way to the configure-driven
version; ideally this goes away by the end of this little project.

(I just thought it made the recipes read nicer and reduced the chance for
anyone else getting the pip flags wrong in the interim.)

--js

>
Paolo Bonzini May 26, 2022, 7:54 p.m. UTC | #4
On Thu, May 26, 2022 at 4:17 PM John Snow <jsnow@redhat.com> wrote:
>> > -     $(call quiet-command, \
>> > -            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
>> > -            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
>> > +     $(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
>> > +     $(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
>> >       $(call quiet-command, touch $@)
>> >
>> >   $(TESTS_RESULTS_DIR):
>>
>> Sooner or later I'd like quiet-command to be changed to English
>> descriptions like the ones currently emitted during the ninja build, but
>> stuff for later.
>
> If it helps, this is a bit of a stopgap on the way to the configure-driven version; ideally this goes away by the end of this little project.
>
> (I just thought it made the recipes read nicer and reduced the chance for anyone else getting the pip flags wrong in the interim.)

Don't worry, you're at least consistent with the current way the macros work.

Paolo
diff mbox series

Patch

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 839ffde876a..052d7f56e9a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -104,13 +104,13 @@  else
 	AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
 endif
 
+quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
+    $(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
+    "VENVPIP","$1")
+
 $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
-	$(call quiet-command, \
-            $(PYTHON) -m venv $@, \
-            VENV, $@)
-	$(call quiet-command, \
-            $(TESTS_PYTHON) -m pip -q --disable-pip-version-check install \
-            -r $(TESTS_VENV_REQ), PIP, $(TESTS_VENV_REQ))
+	$(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
+	$(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
 	$(call quiet-command, touch $@)
 
 $(TESTS_RESULTS_DIR):