Re: [PULL 04/10] build: fix device module builds

[Claudio Fontana]

On 7/3/20 12:25 PM, Gerd Hoffmann wrote:
> On Fri, Jul 03, 2020 at 11:01:43AM +0200, Claudio Fontana wrote:
>> On 7/2/20 2:20 PM, Gerd Hoffmann wrote:
>>> See comment.  Feels quite hackish.  Better ideas anyone?
>>
>> A better idea could be to investigate what and why gets into the variable.
> 
> I see paths prefixed by "../", which seems to come from recursing into
> target directories and not properly handling -m there.
> 
> I think this stop-gap will do the job fine as long as we don't have
> target-specific modules.
> 
> With the pending switch to a meson-based build system which I hope
> simplifies all this I didn't feel like investing too much effort into
> finding the real root cause.  Debugging the Makefiles is a PITA ...
> 
> I'm open to any hints though.
> 
> take care,
>   Gerd
> 

Hi, if I recall correctly I encountered this.

It has to do with the convoluted rules in rules.mak for modules.

There are multiple specific problems there, beyond design issues.

one is

%$(DSOSUF): %.mo

this breaks and causes all sorts of misbehavior for make rules that do not 
require ./configure.

it can be worked around by providing a default definition in rules.mak for 
DSOSUF.

One other issue is the corresponding recipe

$(if $(findstring /,$@),$(call quiet-command,cp $@ $(subst 
/,-,$@),"CP","$(subst /,-,$@)"))

This causes more than one problem (for example, it is the reason qemu can 
"rebuild" stuff for no apparent reason after doing a full make).

In addition to that, this does not work well when called from the target dir.

If I remember correctly I solved this only building modules from a recursive 
build, ie my Makefile had:

commit 250cb13eb1fb4d4b29552acb0b768616321886ef
Author: Claudio Fontana <cfontana@suse.de>
Date:   Wed May 13 19:13:15 2020 +0200

    Makefile: add recurse-modules
    
    Signed-off-by: Claudio Fontana <cfontana@suse.de>

diff --git a/Makefile b/Makefile
index 34275f57c9..5ec1ff2c2f 100644
--- a/Makefile
+++ b/Makefile
@@ -479,7 +479,7 @@ dummy := $(call unnest-vars,, \
 
 include $(SRC_PATH)/tests/Makefile.include
 
-all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) recurse-all 
modules $(vhost-user-json-y)
+all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) recurse-all 
recurse-modules $(vhost-user-json-y)
 
 qemu-version.h: FORCE
        $(call quiet-command, \
@@ -497,7 +497,7 @@ config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
        $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > 
$@,"GEN","$@")
 
-TARGET_DIRS_RULES := $(foreach t, all fuzz clean install, $(addsuffix /$(t), 
$(TARGET_DIRS)))
+TARGET_DIRS_RULES := $(foreach t, all fuzz clean install modules, $(addsuffix 
/$(t), $(TARGET_DIRS)))
 
 SOFTMMU_ALL_RULES=$(filter %-softmmu/all, $(TARGET_DIRS_RULES))
 $(SOFTMMU_ALL_RULES): $(authz-obj-y)
@@ -580,8 +580,9 @@ ROM_DIRS_RULES=$(foreach t, all clean, $(addsuffix /$(t), 
$(ROM_DIRS)))
 $(ROM_DIRS_RULES):
        $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" 
TARGET_DIR="$(dir $@)" CFLAGS="$(filter -O% -g%,$(CFLAGS))" $
(notdir $@),)

-.PHONY: recurse-all recurse-clean recurse-install
+.PHONY: recurse-all recurse-modules recurse-clean recurse-install
 recurse-all: $(addsuffix /all, $(TARGET_DIRS) $(ROM_DIRS))
+recurse-modules: $(addsuffix /modules, $(TARGET_DIRS))
 recurse-clean: $(addsuffix /clean, $(TARGET_DIRS) $(ROM_DIRS))
 recurse-install: $(addsuffix /install, $(TARGET_DIRS))
 $(addsuffix /install, $(TARGET_DIRS)): all
@@ -757,6 +758,8 @@ clean-coverage:
                "CLEAN", "coverage files")
 endif
 
+modules: recurse-modules
+
 clean: recurse-clean
 # avoid old build problems by removing potentially incorrect old files
        rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h


-----------------

and then my rules.mak had:

@@ -106,11 +111,12 @@ LINK = $(call quiet-command, $(LINKPROG) $(CFLAGS) 
$(QEMU_LDFLAGS) -o $@ \
 DSO_OBJ_CFLAGS := -fPIC -DBUILD_DSO
 module-common.o: CFLAGS += $(DSO_OBJ_CFLAGS)
 %$(DSOSUF): QEMU_LDFLAGS += $(LDFLAGS_SHARED)
-%$(DSOSUF): %.mo
+../%$(DSOSUF): DEST=$(subst /,-,$(subst ../,,$@))
+../%$(DSOSUF): ../%.mo
+       @# Link non-accelerator, non-target-specific modules
        $(call LINK,$^)
        @# Copy to build root so modules can be loaded when program started 
without install
-       $(if $(findstring /,$@),$(call quiet-command,cp $@ $(subst 
/,-,$@),"CP","$(subst /,-,$@)"))
-
+       $(call quiet-command,cp $@ ../$(DEST),"CP","$(DEST)")
 
 LD_REL := $(CC) -nostdlib $(LD_REL_FLAGS)
 
@@ -364,7 +370,7 @@ define unnest-vars
                    $(eval $($o-objs): CFLAGS += $(DSO_OBJ_CFLAGS))
                    $(eval $o: $($o-objs)))
              $(eval $(patsubst %-m,%-y,$v) += $($v))
-             $(eval modules: $($v:%.mo=%$(DSOSUF))),
+             $(if $(findstring accel-,$(v)),,$(eval modules: 
$($v:%.mo=%$(DSOSUF)))),
              $(eval $(patsubst %-m,%-y,$v) += $(call expand-objs, $($v)))))
 
     # Post-process all the unnested vars