| Message ID | 7f0d8e16-c580-4dba-a81a-72d5334052dc@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: tool option handling adjustments | expand |
On Thu, Dec 07, 2023 at 11:48:12AM +0100, Jan Beulich wrote: > Passing in $(CFLAGS) means also requesting inclusion of certain headers > (via -include command line options). That's particularly xen/config.h, > which in turn requires generated/autoconf.h. This has not caused any > problems so far only because arch.mk is processed twice, and the missing The first pass is ignore because "include/config/auto.conf" is missing, and generating that will also generates "generated/autoconf.h" I think. So result don't matter, and make restart from scratch once "auto.conf" is generated. > header on the 1st pass would be there on the 2nd. Having added an > inclusion of asm/asm-macros.h to x86'es asm/config.h, the 2nd pass then I don't see asm-macros.h been included in asm/config.h, is this in a pending patch? > also fails on an initial, pristine build. > > As per dd40177c1bc8 ("x86-64/EFI: add CFLAGS to check compile") dropping > the use of $(CFLAGS) altogether isn't an option, though. Hence remove > the problematic options only. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> In any case, patch looks fine: Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> > --- > This is now the 3rd place where the -include needs dropping. I was half > decided to introduce a new lazy-expansion variable, yet it's not > consistently $(CFLAGS) that the options need purging from. Thoughts? Something like that I guess, we probably want to avoid the "-include" while testing the compiler. I guess introducing "-include config.h" once we have all CFLAGS might make more sense, that is at about the time where XEN_CFLAGS is currently introduced, but I haven't checked if that's fine to do. Cheers,
On 06.03.2024 15:57, Anthony PERARD wrote: > On Thu, Dec 07, 2023 at 11:48:12AM +0100, Jan Beulich wrote: >> Passing in $(CFLAGS) means also requesting inclusion of certain headers >> (via -include command line options). That's particularly xen/config.h, >> which in turn requires generated/autoconf.h. This has not caused any >> problems so far only because arch.mk is processed twice, and the missing > > The first pass is ignore because "include/config/auto.conf" is missing, > and generating that will also generates "generated/autoconf.h" I think. > So result don't matter, and make restart from scratch once "auto.conf" > is generated. > >> header on the 1st pass would be there on the 2nd. Having added an >> inclusion of asm/asm-macros.h to x86'es asm/config.h, the 2nd pass then > > I don't see asm-macros.h been included in asm/config.h, is this > in a pending patch? Yes. I hoped to get this across with "Having added ...". >> also fails on an initial, pristine build. >> >> As per dd40177c1bc8 ("x86-64/EFI: add CFLAGS to check compile") dropping >> the use of $(CFLAGS) altogether isn't an option, though. Hence remove >> the problematic options only. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > In any case, patch looks fine: > Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Thanks. Jan
--- a/xen/arch/x86/arch.mk +++ b/xen/arch/x86/arch.mk @@ -87,7 +87,8 @@ efi-check := arch/x86/efi/check $(shell mkdir -p $(dir $(efi-check))) # Check if the compiler supports the MS ABI. -XEN_BUILD_EFI := $(call if-success,$(CC) $(CFLAGS) -c $(srctree)/$(efi-check).c -o $(efi-check).o,y) +XEN_BUILD_EFI := $(call if-success,$(CC) $(filter-out -include %/include/xen/config.h,$(CFLAGS)) \ + -c $(srctree)/$(efi-check).c -o $(efi-check).o,y) # Check if the linker supports PE. EFI_LDFLAGS := $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10
Passing in $(CFLAGS) means also requesting inclusion of certain headers (via -include command line options). That's particularly xen/config.h, which in turn requires generated/autoconf.h. This has not caused any problems so far only because arch.mk is processed twice, and the missing header on the 1st pass would be there on the 2nd. Having added an inclusion of asm/asm-macros.h to x86'es asm/config.h, the 2nd pass then also fails on an initial, pristine build. As per dd40177c1bc8 ("x86-64/EFI: add CFLAGS to check compile") dropping the use of $(CFLAGS) altogether isn't an option, though. Hence remove the problematic options only. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- This is now the 3rd place where the -include needs dropping. I was half decided to introduce a new lazy-expansion variable, yet it's not consistently $(CFLAGS) that the options need purging from. Thoughts? There probably ought to be a Fixes: tag here, but it's quite hard to tell which change to actually blame. It's the interaction of various changes which has resulted in the (so far only latent) badness.