| Message ID | 20200702122048.27798-5-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,01/10] module: qom module support | expand |
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 guess at this point we will need to revisit this later on. CLaudio > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Message-id: 20200624131045.14512-5-kraxel@redhat.com > --- > Makefile.target | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Makefile.target b/Makefile.target > index 8ed1eba95b9c..c70325df5796 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -179,6 +179,13 @@ endif # CONFIG_SOFTMMU > dummy := $(call unnest-vars,,obj-y) > all-obj-y := $(obj-y) > > +# > +# common-obj-m has some crap here, probably as side effect from > +# filling obj-y. Clear it. Fixes suspious dependency errors when > +# building devices as modules. > +# > +common-obj-m := > +> include $(SRC_PATH)/Makefile.objs > dummy := $(call unnest-vars,.., \ > authz-obj-y \ >
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
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
Hi, > 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. I guess you just figured why there is a plan to replace the current build system with a meson based one ;) take care, Gerd
diff --git a/Makefile.target b/Makefile.target index 8ed1eba95b9c..c70325df5796 100644 --- a/Makefile.target +++ b/Makefile.target @@ -179,6 +179,13 @@ endif # CONFIG_SOFTMMU dummy := $(call unnest-vars,,obj-y) all-obj-y := $(obj-y) +# +# common-obj-m has some crap here, probably as side effect from +# filling obj-y. Clear it. Fixes suspious dependency errors when +# building devices as modules. +# +common-obj-m := + include $(SRC_PATH)/Makefile.objs dummy := $(call unnest-vars,.., \ authz-obj-y \
See comment. Feels quite hackish. Better ideas anyone? Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200624131045.14512-5-kraxel@redhat.com --- Makefile.target | 7 +++++++ 1 file changed, 7 insertions(+)