Re: [Qemu-devel] [PATCH v2] rules.mak: Rewrite unnest-vars

[Paolo Bonzini <pbonzini@redhat.com>]

Just one question:

> +# fix-paths
> +# Usage: $(call fix-paths, obj_path, src_path, vars)
> +# Add prefix @obj_path to all objects in @vars, and add prefix @src_path to all
> +# directories in @vars.
> +define fix-paths
> +    $(foreach v,$3,
> +        $(foreach o,$($v),
> +            $(if $($o-libs),
> +                $(eval $1$o-libs := $(value $o-libs)))
> +            $(if $($o-cflags),
> +                $(eval $1$o-cflags := $(value $o-cflags)))
> +            $(if $($o-objs),
> +                $(eval $1$o-objs := $(addprefix $1,$(value $o-objs)))))
> +        $(eval $v := $(addprefix $1,$(filter-out %/,$(value $v))) \
> +                     $(addprefix $2,$(filter %/,$(value $v)))))

Why $(value $v) here in?  I think you need to expand the variable, 
especially in the last assignment but possibly also in the others.  For 
example if you have

     OBJECTS = foo1.o foo2.o
     foo.mo-objs = $(OBJECTS)

the addprefix would set

     foo.mo-objs = dir/$(OBJECTS)

which is wrong.

Paolo

>  endef
>
> -define process-modules
> -$(foreach o,$(filter %.o,$($1)),
> -	$(eval $(patsubst %.o,%.mo,$o): $o) \
> -	$(eval $(patsubst %.o,%.mo,$o)-objs := $o))
> -$(foreach o,$(filter-out $(modules-m), $(patsubst %.o,%.mo,$($1))), \
> -    $(eval $o-objs += module-common.o)
> -    $(eval $o: $($o-objs))
> -    $(eval modules-objs-m += $($o-objs))
> -    $(eval modules-m += $o)
> -    $(eval $o:; $$(call quiet-command,touch $$@,"  GEN   $$(TARGET_DIR)$$@"))
> -    $(if $(CONFIG_MODULES),$(eval modules: $(patsubst %.mo,%$(DSOSUF),$o)))) \
> -$(eval modules-objs-m := $(sort $(modules-objs-m)))
> -$(foreach o,$(modules-objs-m), \
> -    $(if $(CONFIG_MODULES),$(eval $o-cflags := $(call maybe-add, $(DSO_CFLAGS), $($o-cflags)))))
> -$(eval $(patsubst %-m,%-$(call lnot,$(CONFIG_MODULES)),$1) += $($1))
> +# unnest-var-recursive
> +# Usage: $(call unnest-var-recursive, obj_prefix, vars, var)
> +#
> +# Unnest @var by including subdir Makefile.objs, while protect others in @vars
> +# unchanged.
> +#
> +# @obj_prefix is the starting point of object path prefix.
> +#
> +define unnest-var-recursive
> +	$(eval dirs := $(sort $(filter %/,$($3))))
> +	$(eval $3 := $(filter-out %/,$($3)))
> +	$(foreach d,$(dirs:%/=%),
> +			$(call save-vars,$2)
> +			$(eval obj := $(if $1,$1/)$d)
> +			$(eval -include $(SRC_PATH)/$d/Makefile.objs)
> +			$(call fix-paths,$(if $1,$1/)$d/,$d/,$2)
> +			$(call load-vars,$2,$3)
> +			$(call unnest-var-recursive,$1,$2,$3))
>  endef
>
> +# unnest-vars
> +# Usage: $(call unnest-vars, obj_prefix, vars)
> +#
> +# @obj_prefix: object path prefix, can be empty, or '..', etc. Don't include
> +# ending '/'.
> +#
> +# @vars: the list of variable names to unnest.
> +#
> +# This macro will scan subdirectories's Makefile.objs, include them, to build
> +# up each variable listed in @vars.
> +#
> +# Per object and per module cflags and libs are saved with relative path fixed
> +# as well, those variables include -libs, -cflags and -objs. Items in -objs are
> +# also fixed to relative path against SRC_PATH plus the prefix @obj_prefix.
> +#
> +# All nested variables postfixed by -m in names are treated as DSO variables,
> +# and will be built as modules, if enabled.
> +#
> +# A simple example of the unnest:
> +#
> +#     obj_prefix = ..
> +#     vars = hot cold
> +#     hot  = fire.o sun.o season/
> +#     cold = snow.o water/ season/
> +#
> +# Unnest through a faked source directory structure:
> +#
> +#     SRC_PATH
> +#        ├── water
> +#        │   └── Makefile.objs──────────────────┐
> +#        │       │ hot += steam.o               │
> +#        │       │ cold += ice.mo               │
> +#        │       │ ice.mo-libs := -licemaker    │
> +#        │       │ ice.mo-objs := ice1.o ice2.o │
> +#        │       └──────────────────────────────┘
> +#        │
> +#        └── season
> +#            └── Makefile.objs──────┐
> +#                │ hot += summer.o  │
> +#                │ cold += winter.o │
> +#                └──────────────────┘
> +#
> +# In the end, the result will be:
> +#
> +#     hot  = ../fire.o ../sun.o ../season/summer.o
> +#     cold = ../snow.o ../water/ice.mo ../season/winter.o
> +#     ../water/ice.mo-libs = -licemaker
> +#     ../water/ice.mo-objs = ../water/ice1.o ../water/ice2.o
> +#
> +# Note that 'hot' didn't include 'season/' in the input, so 'summer.o' is not
> +# included.
> +#
>  define unnest-vars
> -$(eval obj := $1)
> -$(eval nested-vars := $2)
> -$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))
> -$(eval old-nested-dirs := )
> -$(call unnest-vars-1)
> -$(if $1,$(foreach v,$(nested-vars),$(eval \
> -	$v := $(addprefix $1/,$($v)))))
> -$(foreach var,$(nested-vars),$(eval $(var) := $(filter-out %/, $($(var)))))
> -$(shell mkdir -p $(sort $(foreach var,$(nested-vars),$(dir $($(var))))))
> -$(foreach var,$(nested-vars), $(eval \
> -  -include $(addsuffix *.d, $(sort $(dir $($(var)))))))
> -$(foreach v,$(filter %-m,$(nested-vars)), \
> -    $(call process-modules,$v))
> +    # In the case of target build (i.e. $1 == ..), fix path for top level
> +    # Makefile.objs objects
> +    $(if $1,$(call fix-paths,$1/,,$2))
> +
> +    # Descend and include every subdir Makefile.objs
> +    $(foreach v, $2, $(call unnest-var-recursive,$1,$2,$v))
> +
> +    $(foreach v,$(filter %-m,$2),
> +        # All .o found in *-m variables are single object modules, create .mo
> +        # for them
> +        $(foreach o,$(filter %.o,$($v)),
> +            $(eval $(o:%.o=%.mo)-objs := $o))
> +        # Now unify .o in -m variable to .mo
> +        $(eval $v := $($v:%.o=%.mo))
> +
> +        $(eval modules: $($v:%.mo=%$(DSOSUF)))
> +        $(eval modules-m += $($v:%.mo=%$(DSOSUF)))
> +
> +        # For non-module build, add -m to -y
> +        $(if $(CONFIG_MODULES),,$(eval $(patsubst %-m,%-y,$v) += $($v))))
> +
> +    # .mo's are .PHONY targets
> +    $(eval .PHONY: $(modules-m))
> +    $(eval $(modules-m): module-common.o)
> +
> +    # Post-process all the unnested vars
> +    $(foreach v,$2,
> +        $(foreach o, $(filter %.mo,$($v)),
> +            # Find all the .mo objects in variables and add dependency rules
> +            # according to .mo-objs. Report error if not set
> +            $(if $($o-objs),
> +                $(eval $o: $($o-objs)),
> +                $(error $o added in $v but $o-objs is not set))
> +            # Pass the .mo-cflags along to member objects
> +            $(if $($o-cflags),
> +                $(foreach p,$($o-objs),
> +                    $(eval $p-cflags := $($o-cflags)))))
> +        $(shell mkdir -p ./ $(sort $(dir $($v))))
> +        # Include all the .d files
> +        $(eval -include $(addsuffix *.d, $(sort $(dir $($v)))))
> +        $(eval $v := $(filter-out %/,$($v))))
>  endef
>