Re: [Qemu-devel] [RFC PATCH v3 3/5] Makefile: introduce common-obj-m and block-obj-m for DSO

[Paolo Bonzini]

Il 10/09/2013 11:42, Fam Zheng ha scritto:
> On Tue, 09/10 08:45, Paolo Bonzini wrote:
>> Il 10/09/2013 03:02, Fam Zheng ha scritto:
>>> -all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
>>> +# static linked mods are expanded to .o list
>>> +dummy := $(call expand-mod-obj,common-obj-y)
>>> +dummy := $(call expand-mod-obj,block-obj-y)
>>> +
>>> +modules-m = $(patsubst %.o,%$(DSOSUF),$(filter %.o,$(block-obj-m) 
>>> $(common-obj-m))) \
>>> +            $(patsubst %.mo,%$(DSOSUF),$(filter %.mo,$(block-obj-m) 
>>> $(common-obj-m)))
>>> +
>>> +all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all $(modules-m)
>>> +
>>> +# Generate rules for single file modules (%.so: %.o).
>>> +$(foreach o,$(filter %.o,$(block-obj-m) $(common-obj-m)),$(eval \
>>> +   $(patsubst %.o,%.so,$o): $o))
>>> +
>>> +# For multi file modules, dependencies should be listed explicitly in
>>> +# Makefile.objs as
>>> +#     foo.mo-objs := bar.o biz.o
>>> +$(foreach o,$(filter %.mo,$(block-obj-m) $(common-obj-m)),$(eval \
>>> +   $(patsubst %.mo,%.so,$o): $($o-objs)))
>>
>> I agree that this foo.mo-objs variable is homogeneous with how you
>> handle libraries and cflags.  I like it now.
>>
>> However, I don't like the many places in which you have to special-case
>> modules (expand-mod-obj, modules-m, etc.), and the duplication between
>> Makefile and Makefile.target.
>>
>> I would prefer if you try doing this patch along the lines I suggested
>> in my review of v2, using .mo files as a placeholder and then doing the
>> final link either into the .so or in the executable.  This should remove
>> the need for at least expand-mod-obj, and probably for more of the
>> duplicated constructs you have.
>>
> OK. I'll try.
> 
>> In particular, I would like modules-m to be simply "$(block-obj-m)
>> $(common-obj-m)".
> 
> There need to be some variable with %.o and %.mo subst to %.so, to become
> dependency of target "all".

I think you are putting too much weight on %.so, which complicates
things when handling both modular and non-modular builds.  Assuming you
have transformed block-obj-m and common-obj-m to only contain .mo files,
with something like this:

    define add-modules
    $(foreach o, $(filter-out %.o, $($1)), $(eval $o-objs := $o))
    $(eval modules-m += $(patsubst %.o,%.mo,$($1)))
    endif

    dummy := $(call add-modules,block-obj-m)
    dummy := $(call add-modules,common-obj-m)

then building modules can be just a bunch of extra targets:

    ifeq ($(CONFIG_MODULES),y)
    modules: $(patsubst %.mo,%$(DSOSUF),$(modules-m))
    all: modules
    endif

>> In the medium term, we need to find a way to avoid the duplication:
>>
>>      block-obj-y = block/
>>      block-obj-m = block/
>>
>> Perhaps by introducing a "dirs" variable that automatically triggers
>> recursion on all nested variables.  But this can be the topic of a
>> separate patch series, if you prefer.
>>
> Agree but prefer to do it in a separate series.

Sure.

Paolo