Re: [PATCH v5 1/3] automation: Create Yocto docker images

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Nov 30, 2022 at 12:15:07PM +0000, Bertrand Marquis wrote:
> diff --git a/automation/build/Makefile b/automation/build/Makefile
> index a4b2b85178cf..72a5335baec1 100644
> --- a/automation/build/Makefile
> +++ b/automation/build/Makefile
> @@ -1,13 +1,18 @@
>  
>  # the base of where these containers will appear
>  REGISTRY := registry.gitlab.com/xen-project/xen
> -CONTAINERS = $(subst .dockerfile,,$(wildcard */*.dockerfile))
> +CONTAINERS = $(filter-out yocto/%,$(subst .dockerfile,,$(wildcard 
> */*.dockerfile)))

Nit: while there, could you use ":=" instead of "=" ? The value of
CONTAINERS is always going to be evaluated by make because it's used as
a prerequisite of "all", so we can at least tell make to evaluate the
value right away.

> +CONTAINERS_EXTRA =
>  DOCKER_CMD ?= docker
>  
> +include yocto/yocto.inc
> +
>  help:
>       @echo "Builds containers for building Xen based on different distros"
>       @echo "To build one run 'make DISTRO/VERSION'. Available containers:"
>       @$(foreach file,$(sort $(CONTAINERS)),echo ${file};)
> +     @echo "Extra containers (not built using make all):"
> +     @$(foreach file,$(sort $(CONTAINERS_EXTRA)),echo ${file};)

I wonder why the help syntax uses both ${} and $() for make variables, is
it to confuse people? :-)

You can write $(file) instead of ${file}, I think this would be less
confusing. I rarely see ${} been used in make, so seen ${} can be
confusing. I've learned (relearned?) this alternative syntax only a few
weeks ago as it's used by automake or autoconf.

>       @echo "To push container builds, set the env var PUSH"
>  
>  %: %.dockerfile ## Builds containers
> @@ -16,5 +21,10 @@ help:
>               $(DOCKER_CMD) push $(REGISTRY)/$(@D):$(@F); \
>       fi
>  
> -.PHONY: all
> +.PHONY: all clean
>  all: $(CONTAINERS)
> +
> +# Remove generated dockerfiles for yocto
> +clean:
> +     rm -f yocto/*.dockerfiles

There's an extra 's', I guess you want to remove "*.dockerfile" instead
of "*.dockerfiles".

You could also add those to a .gitignore, even if there are likely to be
removed by make.


Cheers,

-- 
Anthony PERARD