Re: [PATCH 4/4] stubdom: add fine grained library config items to Mini-OS configs

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Sat, Oct 05, 2024 at 05:15:48PM +0200, Juergen Gross wrote:
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index 8c503c2bf8..3b501a0710 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -340,6 +340,14 @@ endef
>  
>  $(foreach lib,$(STUB_LIBS),$(eval $(call BUILD_lib,$(lib))))
>  
> +define BUILD_config
> + cp $< $@
> + for i in $(sort $(APP_LIBS) $(call xenlibs-dependencies,$(APP_LIBS))); do \
> +   u=`echo $$i | tr a-z A-Z`; \
> +   echo "CONFIG_LIBXEN$$u=y"; \
> + done >> $@
> +endef

I don't think I like having a recipe hidden like that in a variable,
maybe if it was a full rule it would be a bit less annoying to me. But
how about something slightly different:

First, the name, "GEN_config" would be a bit better, then we could have
it only do the output and not writing any file:

define GEN_config
 (cat '$<' && \
 for i in $(sort $(APP_LIBS) $(call xenlibs-dependencies,$(APP_LIBS))); do \
   u=`echo $$i | tr a-z A-Z`; \
   echo "CONFIG_LIBXEN$$u=y"; \
 done)
endef

The this can be used in rules as:
    $(GEN_config) > $@

Would that be ok?

(It might be better to have the macro not depends on the environment
have take parameter explicitly which could be used as $(call
GEN_config,$<,evtchn gnttab) > $@ or take a variable if it's useful
elsewhere, but I'm already fine if $@ is taken out of the macro.)

> +
>  xenstore/stamp: $(XEN_ROOT)/tools/xenstored/Makefile.common
>       $(do_links)
>  
> @@ -373,8 +381,12 @@ $(TARGETS_MINIOS): mini-os-%:
>  # ioemu
>  #######
>  
> -ioemu-minios-config.mk: $(CURDIR)/ioemu-minios.cfg
> -     MINIOS_CONFIG="$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C 
> $(MINI_OS) config
> +ioemu-minios.out.cfg: APP_LIBS = evtchn gnttab ctrl guest
> +ioemu-minios.out.cfg: $(CURDIR)/ioemu-minios.cfg Makefile

Could you change the suffix to ".gen.cfg"? ".out.cfg" is a bit generic
while "generated" is more common for the kind of file that are
automatically generated by the build system for it's own use.

BTW, in the first prerequisite, $(CURDIR) isn't necessary anymore, it
was only to be used in "MINIOS_CONFIG" just below.

> +     $(BUILD_config)
> +
> +ioemu-minios-config.mk: ioemu-minios.out.cfg
> +     MINIOS_CONFIG="$(CURDIR)/$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) 
> DESTDIR= -C $(MINI_OS) config

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech