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

John Snow posted 9 patches 3 years, 8 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>
[PATCH 5/9] tests: add quiet-venv-pip macro
Posted by John Snow 3 years, 8 months ago
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):
-- 
2.34.1
Re: [PATCH 5/9] tests: add quiet-venv-pip macro
Posted by Paolo Bonzini 3 years, 8 months ago
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
Re: [PATCH 5/9] tests: add quiet-venv-pip macro
Posted by John Snow 3 years, 8 months ago
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

>
Re: [PATCH 5/9] tests: add quiet-venv-pip macro
Posted by Paolo Bonzini 3 years, 8 months ago
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
Re: [PATCH 5/9] tests: add quiet-venv-pip macro
Posted by Paolo Bonzini 3 years, 8 months ago
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>