Re: [Qemu-devel] [PATCH v3 3/6] tests/docker: add podman support

[Markus Armbruster]

Just saw this land in master, awesome.  Suggestion inline.

Marc-André Lureau <address@hidden> writes:

> Allow to specify the container engine to run with ENGINE variable.
>
> By default, ENGINE=auto and will select either podman or docker.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  Makefile                      | 2 +-
>  tests/docker/Makefile.include | 8 ++++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1fcbaed62c..7863bb0cf5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1153,7 +1153,7 @@ endif
>       @echo  ''
>       @echo  'Test targets:'
>       @echo  '  check           - Run all tests (check-help for details)'
> -     @echo  '  docker          - Help about targets running tests inside 
> Docker containers'
> +     @echo  '  docker          - Help about targets running tests inside 
> containers'
>       @echo  '  vm-help         - Help about targets running tests inside VM'

Let's rename target docker to something like container-help, for
symmetry with vm-help, and because calling the target to get help on X X
is in poor taste.

See also
commit 4f2f62762f8119886fbb65920931613cd87840b3
Author: Philippe Mathieu-Daudé <address@hidden>
Date:   Fri May 31 08:43:41 2019 +0200

    Makefile: Rename the 'vm-test' target as 'vm-help'
    
    We already have 'make check-help', use the 'make vm-help' form
    to display helps about VM testing. Keep the old target to not
    bother old customs.
    
    Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
    Reviewed-by: Stefano Garzarella <address@hidden>
    Message-Id: <address@hidden>
    Signed-off-by: Alex Bennée <address@hidden>

>       @echo  ''
>       @echo  'Documentation targets:'
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index f4226b79d0..3c1aec862a 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -17,7 +17,9 @@ DOCKER_TESTS := $(notdir $(shell \
>  
>  DOCKER_TOOLS := travis
>  
> -DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py
> +ENGINE := auto
> +
> +DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(ENGINE)
>  
>  TESTS ?= %
>  IMAGES ?= %
> @@ -146,7 +148,7 @@ $(foreach i,$(filter-out 
> $(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES) $(DOCKER_DEPR
>  )
>  
>  docker:
> -     @echo 'Build QEMU and run tests inside Docker containers'
> +     @echo 'Build QEMU and run tests inside Docker or Podman containers'
>       @echo
>       @echo 'Available targets:'
>       @echo
> @@ -193,6 +195,8 @@ endif
>       @echo '    EXECUTABLE=<path>    Include executable in image.'
>       @echo '    EXTRA_FILES="<path> [... <path>]"'
>       @echo '                         Include extra files in image.'
> +     @echo '    ENGINE=auto/docker/podman'

Your | are listing.

> +     @echo '                         Specify which container engine to run.'
>  
>  # This rule if for directly running against an arbitrary docker target.
>  # It is called by the expanded docker targets (e.g. make