On 11/01/2023 6:17 pm, Anthony PERARD wrote: > Some compat headers depends on other compat headers that may not have > been generated due to config option. > > This would be a generic way to deal with deps, instead of > headers-$(call or $(CONFIG_TRACEBUFFER),$(CONFIG_HVM)) += compat/trace.h > > This is just an RFC, as it only deals with "hvm_op.h" and nothing is > done to have make regenerate the new file when needed. > > Signed-off-by: Anthony PERARD <anthony.per...@citrix.com> > --- > xen/include/Makefile | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/xen/include/Makefile b/xen/include/Makefile > index 65be310eca..5e6de97841 100644 > --- a/xen/include/Makefile > +++ b/xen/include/Makefile > @@ -34,6 +34,29 @@ headers-$(CONFIG_TRACEBUFFER) += compat/trace.h > headers-$(CONFIG_XENOPROF) += compat/xenoprof.h > headers-$(CONFIG_XSM_FLASK) += compat/xsm/flask_op.h > > +# Find dependencies of compat headers. > +# e.g. hvm/hvm_op.h needs trace.h; but if CONFIG_TRACEBUFFER=n, then trace.h > would be missing. > +# > +# Using sed to remove ".." from path because unsure if something else is > available > +# There's `realpath`, but maynot be available > +# realpath --relative-to=. -mL compat/hvm/../trace.h -> compat/trace.h > +# `make` also have macro for that $(abspath), only recent version. > +# > +# The $(CC) line to gen deps is derived from $(cmd_compat_i) > +include $(obj)/.compat-header-deps.d > +$(obj)/.compat-header-deps.d: include/public/hvm/hvm_op.h > + $(CC) -MM -MF $@.tmp $(filter-out -Wa$(comma)% -include > %/include/xen/config.h,$(XEN_CFLAGS)) $< > + for f in $$(cat $@.tmp | sed -r '1s/^[^:]*: //; s/ \\$$//'); do \ > + echo $$f; \ > + done | sed -r \ > + -e 's#.*/public#compat#; : p; s#/[^/]+/../#/#; t p; s#$$# \\#' \ > + -e '1i headers-y-deps := \\' -e '$$a \ ' \ > + > $@ > + > +headers-y-deps := $(filter-out compat/xen-compat.h,$(headers-y-deps)) > +# Add compat header dependencies and eliminates duplicates > +headers-y := $(sort $(headers-y) $(headers-y-deps)) > + > cppflags-y := -include public/xen-compat.h > -DXEN_GENERATING_COMPAT_HEADERS > cppflags-$(CONFIG_X86) += -m32 >
For posterity, https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/3585379553 is the issue in question. In file included from arch/x86/hvm/hvm.c:82: ./include/compat/hvm/hvm_op.h:6:10: fatal error: ../trace.h: No such file or directory 6 | #include "../trace.h" | ^~~~~~~~~~~~ compilation terminated. make[4]: *** [Rules.mk:246: arch/x86/hvm/hvm.o] Error 1 make[3]: *** [Rules.mk:320: arch/x86/hvm] Error 2 make[3]: *** Waiting for unfinished jobs.... All public headers use "../" relative includes for traversing the public/ hierarchy. This cannot feasibly change given our "copy this into your project" stance, but it means the compat headers have the same structure under compat/. This include is supposed to be including compat/trace.h but it was actually picking up x86's asm/trace.h, hence the build failure now that I've deleted the file. This demonstrates that trying to be clever with -iquote is a mistake. Nothing good can possibly come of having the <> and "" include paths being different. Therefore we must revert all uses of -iquote. But, that isn't the only bug. The real hvm_op.h legitimately includes the real trace.h, therefore the compat hvm_op.h legitimately includes the compat trace.h too. But generation of compat trace.h was made asymmetric because of 2c8fabb223. In hindsight, that's a public ABI breakage. The current configuration of this build of the hypervisor has no legitimate bearing on the headers needing to be installed to /usr/include/xen. Or put another way, it is a breakage to require Xen to have CONFIG_COMPAT+CONFIG_TRACEBUFFER enabled in the build simply to get the public API headers generated properly. So 2c8fabb223 needs reverting too. ~Andrew
