On 12/01/2023 7:46 am, Jan Beulich wrote: > On 11.01.2023 23:29, Andrew Cooper wrote: >> 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. > I'm afraid I can't see the connection between use of -iquote and the bug > here.
So I had concluded (wrongly) that it was to do with an asymmetry of include paths, but it's not. <../$x> would behave the same, even if it is a bit more obviously wrong. The actual problem is the use of ./ or ../ because, despite how they read, they are never relative to the current file. The contents between the "" or <> is treated as a string literal and not interpreted by CPP. So furthermore, the public headers are buggy in their use of ../ because it is an implicit dependency on -I/path/to/xen/headers/dir/ being earlier on the include path than other dirs with these fairly generic and not-xen-prefixed file names. I still think that include path asymmetry is bad idea and wants reverting, but I agree it's not the source of this bug. >> 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. > There are no public API headers which are generated. The compat headers > are generate solely for Xen's internal purposes (and hence there's also > no public ABI breakage). Wow, I really was decaffeinated when working on this... Yeah, it's not that either. ~Andrew
