diff mbox series

[v5,2/3] automation: Add a clean rule for containers

Message ID f793cc70fdb6802b66156a8756bf676fbac0d809.1669810269.git.bertrand.marquis@arm.com (mailing list archive)
State Superseded
Headers show
Series Yocto Gitlab CI | expand

Commit Message

Bertrand Marquis Nov. 30, 2022, 12:15 p.m. UTC
Add make clean support to remove the containers from the local docker
registry.
make clean-<image_name> must be called to remove an image:
make clean-yocto/kirkstone-qemuarm: remove yocto kirkstone for qemuarm
image

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in v5:
- remove cleaning of all images using make clean
Changes in v4:
- also generate clean rule for CONTAINERS_EXTRA
Changes in v3:
- none
Changes in v2:
- none
Changes in v1:
- patch added
---
 automation/build/Makefile | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Michal Orzel Nov. 30, 2022, 1:03 p.m. UTC | #1
Hi Bertrand,

On 30/11/2022 13:15, Bertrand Marquis wrote:
> 
> 
> Add make clean support to remove the containers from the local docker
> registry.
> make clean-<image_name> must be called to remove an image:
> make clean-yocto/kirkstone-qemuarm: remove yocto kirkstone for qemuarm
> image
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal
Anthony PERARD Nov. 30, 2022, 3:48 p.m. UTC | #2
On Wed, Nov 30, 2022 at 12:15:08PM +0000, Bertrand Marquis wrote:
> --- a/automation/build/Makefile
> +++ b/automation/build/Makefile
> @@ -28,3 +28,13 @@ all: $(CONTAINERS)
>  clean:
>  	rm -f yocto/*.dockerfiles
>  
> +define CLEAN_RULE
> +.PHONY: clean-$(1)
> +clean-$(1):
> +ifneq ($$(shell docker image ls -q $(REGISTRY)/$(subst /,:,$(1))),)

Please, don't use "ifneq" in a rule's recipe, especially when running a
shell command. That shell command is evaluated every time make parse the
makefile, so we are going to run `docker image ls` 23 times!

Just write the call to `docker image ls` and evaluate the result in
shell. I guess something like:
    [ "$(docker image ls -q ...)" ] && docker image rm ...


Cheers,
Bertrand Marquis Nov. 30, 2022, 4:14 p.m. UTC | #3
Hi Anthony,

> On 30 Nov 2022, at 15:48, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> On Wed, Nov 30, 2022 at 12:15:08PM +0000, Bertrand Marquis wrote:
>> --- a/automation/build/Makefile
>> +++ b/automation/build/Makefile
>> @@ -28,3 +28,13 @@ all: $(CONTAINERS)
>> clean:
>> 	rm -f yocto/*.dockerfiles
>> 
>> +define CLEAN_RULE
>> +.PHONY: clean-$(1)
>> +clean-$(1):
>> +ifneq ($$(shell docker image ls -q $(REGISTRY)/$(subst /,:,$(1))),)
> 
> Please, don't use "ifneq" in a rule's recipe, especially when running a
> shell command. That shell command is evaluated every time make parse the
> makefile, so we are going to run `docker image ls` 23 times!
> 
> Just write the call to `docker image ls` and evaluate the result in
> shell. I guess something like:
>    [ "$(docker image ls -q ...)" ] && docker image rm ...

Very good point, I will fix that in v6.

Cheers
Bertrand

> 
> 
> Cheers,
> 
> -- 
> Anthony PERARD
diff mbox series

Patch

diff --git a/automation/build/Makefile b/automation/build/Makefile
index 72a5335baec1..4cbb1365f94e 100644
--- a/automation/build/Makefile
+++ b/automation/build/Makefile
@@ -28,3 +28,13 @@  all: $(CONTAINERS)
 clean:
 	rm -f yocto/*.dockerfiles
 
+define CLEAN_RULE
+.PHONY: clean-$(1)
+clean-$(1):
+ifneq ($$(shell docker image ls -q $(REGISTRY)/$(subst /,:,$(1))),)
+	docker image rm $(REGISTRY)/$(subst /,:,$(1))
+endif
+
+endef
+
+$(eval $(foreach img,$(CONTAINERS) $(CONTAINERS_EXTRA),$(call CLEAN_RULE,$(img))))