| Message ID | Zu4063fjfHC5hHUl@humpty.home.comstyle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | contrib/plugins: ensure build does not pick up a system copy of plugin header | expand |
Brad Smith <brad@comstyle.com> writes: > contrib/plugins: ensure build does not pick up a system copy of plugin > header I'm confused because this changes the ordering of the GLIB inclusion. We shouldn't be including the whole QEMU include path. How does this fail? > With the ordering of the header path if a copy of QEMU is installed it > will pickup the system copy of the header before the build paths copy > and the build will fail. > > Signed-off-by: Brad Smith <brad@comstyle.com> > --- > contrib/plugins/Makefile | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile > index 05a2a45c5c..52fc390376 100644 > --- a/contrib/plugins/Makefile > +++ b/contrib/plugins/Makefile > @@ -41,9 +41,10 @@ SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES))) > > # The main QEMU uses Glib extensively so it is perfectly fine to use it > # in plugins (which many example do). > -PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) > -PLUGIN_CFLAGS += -fPIC -Wall > +GLIB_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) > PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu Not withstanding the fact I've just borrowed bswap.h for a test plugin maybe we should actually copy qemu-plugin.h to an entirely new location during the build and then include from there to avoid any other potential pollutions? > +PLUGIN_CFLAGS += $(GLIB_CFLAGS) > +PLUGIN_CFLAGS += -fPIC -Wall > > # Helper that honours V=1 so we get some output when compiling > quiet-@ = $(if $(V),,@$(if $1,printf " %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))
On 2024-09-21 8:55 a.m., Alex Bennée wrote: > Brad Smith <brad@comstyle.com> writes: > >> contrib/plugins: ensure build does not pick up a system copy of plugin >> header > I'm confused because this changes the ordering of the GLIB inclusion. We > shouldn't be including the whole QEMU include path. That's intentional. The GLIB header paths cannot come before the header path for the plugin header otherwise it pulls in the older plugin header from the installed copy of QEMU and breaks. The QEMU include path is necessary for the plugin header. cc -O2 -g -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -fPIC -Wall -I/home/brad/tmp/qemu/contrib/plugins/../../include/qemu -c -o execlog.o /home/brad/tmp/qemu/contrib/plugins/execlog.c /home/brad/tmp/qemu/contrib/plugins/execlog.c:262:41: error: too many arguments to function call, expected single argument 'insn', have 3 arguments qemu_plugin_insn_data(insn, &insn_opcode, sizeof(insn_opcode)); ~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/include/qemu-plugin.h:407:13: note: 'qemu_plugin_insn_data' declared here const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn); ^ 1 error generated. > How does this fail? > >> With the ordering of the header path if a copy of QEMU is installed it >> will pickup the system copy of the header before the build paths copy >> and the build will fail. >> >> Signed-off-by: Brad Smith <brad@comstyle.com> >> --- >> contrib/plugins/Makefile | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile >> index 05a2a45c5c..52fc390376 100644 >> --- a/contrib/plugins/Makefile >> +++ b/contrib/plugins/Makefile >> @@ -41,9 +41,10 @@ SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES))) >> >> # The main QEMU uses Glib extensively so it is perfectly fine to use it >> # in plugins (which many example do). >> -PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) >> -PLUGIN_CFLAGS += -fPIC -Wall >> +GLIB_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) >> PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu > Not withstanding the fact I've just borrowed bswap.h for a test plugin > maybe we should actually copy qemu-plugin.h to an entirely new location > during the build and then include from there to avoid any other > potential pollutions? I don't see how that would make any difference, but either way as long as the header path ordering is corrected so this new path is not passed last on the command line getting the ordering wrong. > >> +PLUGIN_CFLAGS += $(GLIB_CFLAGS) >> +PLUGIN_CFLAGS += -fPIC -Wall >> >> # Helper that honours V=1 so we get some output when compiling >> quiet-@ = $(if $(V),,@$(if $1,printf " %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))
Brad Smith <brad@comstyle.com> writes: > On 2024-09-21 8:55 a.m., Alex Bennée wrote: >> Brad Smith <brad@comstyle.com> writes: >> >>> contrib/plugins: ensure build does not pick up a system copy of plugin >>> header >> I'm confused because this changes the ordering of the GLIB inclusion. We >> shouldn't be including the whole QEMU include path. > > That's intentional. The GLIB header paths cannot come before the header path > for the plugin header otherwise it pulls in the older plugin header from the > installed copy of QEMU and breaks. The QEMU include path is necessary > for the plugin header. > > > cc -O2 -g -I/usr/local/include/glib-2.0 > -I/usr/local/lib/glib-2.0/include -I/usr/local/include -fPIC -Wall > -I/home/brad/tmp/qemu/contrib/plugins/../../include/qemu -c -o > execlog.o /home/brad/tmp/qemu/contrib/plugins/execlog.c > /home/brad/tmp/qemu/contrib/plugins/execlog.c:262:41: error: too many > arguments to function call, expected single argument 'insn', have 3 > arguments > qemu_plugin_insn_data(insn, &insn_opcode, sizeof(insn_opcode)); > ~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > /usr/local/include/qemu-plugin.h:407:13: note: 'qemu_plugin_insn_data' > declared here > const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn); > ^ > 1 error generated. Ahh gotcha, I confused myself thinking this was QEMU's internal API header. Queued to plugins/next, thanks. > >> How does this fail? >> >>> With the ordering of the header path if a copy of QEMU is installed it >>> will pickup the system copy of the header before the build paths copy >>> and the build will fail. >>> >>> Signed-off-by: Brad Smith <brad@comstyle.com> >>> --- >>> contrib/plugins/Makefile | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile >>> index 05a2a45c5c..52fc390376 100644 >>> --- a/contrib/plugins/Makefile >>> +++ b/contrib/plugins/Makefile >>> @@ -41,9 +41,10 @@ SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES))) >>> # The main QEMU uses Glib extensively so it is perfectly fine >>> to use it >>> # in plugins (which many example do). >>> -PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) >>> -PLUGIN_CFLAGS += -fPIC -Wall >>> +GLIB_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) >>> PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu >> Not withstanding the fact I've just borrowed bswap.h for a test plugin >> maybe we should actually copy qemu-plugin.h to an entirely new location >> during the build and then include from there to avoid any other >> potential pollutions? > > I don't see how that would make any difference, but either way as long > as the header > path ordering is corrected so this new path is not passed last on the > command line > getting the ordering wrong. > >> >>> +PLUGIN_CFLAGS += $(GLIB_CFLAGS) >>> +PLUGIN_CFLAGS += -fPIC -Wall >>> # Helper that honours V=1 so we get some output when compiling >>> quiet-@ = $(if $(V),,@$(if $1,printf " %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile index 05a2a45c5c..52fc390376 100644 --- a/contrib/plugins/Makefile +++ b/contrib/plugins/Makefile @@ -41,9 +41,10 @@ SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES))) # The main QEMU uses Glib extensively so it is perfectly fine to use it # in plugins (which many example do). -PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) -PLUGIN_CFLAGS += -fPIC -Wall +GLIB_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0) PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu +PLUGIN_CFLAGS += $(GLIB_CFLAGS) +PLUGIN_CFLAGS += -fPIC -Wall # Helper that honours V=1 so we get some output when compiling quiet-@ = $(if $(V),,@$(if $1,printf " %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))
contrib/plugins: ensure build does not pick up a system copy of plugin header With the ordering of the header path if a copy of QEMU is installed it will pickup the system copy of the header before the build paths copy and the build will fail. Signed-off-by: Brad Smith <brad@comstyle.com> --- contrib/plugins/Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)