Re: [Xen-devel] [XEN PATCH v3 15/23] xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Mar 04, 2020 at 04:00:52PM +0100, Jan Beulich wrote:
> On 26.02.2020 12:33, Anthony PERARD wrote:
> > @@ -113,6 +115,64 @@ $(KCONFIG_CONFIG):
> > +AFLAGS += -D__ASSEMBLY__
> > +
> > +CFLAGS += $(CFLAGS-y)
> 
> I can't seem to be able to spot a similar line for AFLAGS.

I didn't add any because it wasn't necessary.

> > @@ -107,7 +65,7 @@ $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) 
> > $(extra-y)),$(eval $(call gend
> >  subdir-y := $(subdir-y) $(filter %/, $(obj-y))
> >  obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
> >  
> > -$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += 
> > -DINIT_SECTIONS_ONLY
> > +$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += 
> > -DINIT_SECTIONS_ONLY
> 
> While in the description you say "We can't use CFLAGS in
> subdirectories to add flags to particular targets, ...", it
> remains unclear there why that is, and hence why changes like
> this one are necessary. If this is a restriction that's going to
> remain, this also needs writing down in a prominent place. After

Yes, I should probably start writing some documentation, probably
following the kind of documentation that Linux has for kbuild, and I
could put that in docs/misc, along side the kconfig doc. That probably
not a prominent place, but I don't know if there's a better place.

> all if (for example) special compiler options are needed, CFLAGS
> would be the natural thing one would want to alter. (Even better
> if wrong playing with CFLAGS could be detected and at least
> warned about, but I'm completely unclear on how feasible this
> would be.)

The issue with the CFLAGS variable is that it is still set by Config.mk
which Rules.mk includes.
I guess I could reset the CFLAGS variable in Rules.mk after loading
Config.mk, and allow it to be used. I want to stop including Config.mk
in Rules.mk at some point anyway.

I don't think it's possible to check if a variable has been altered,
because it could be changed for a specific target where it's not easy to
check.

> > diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> > index 022a3a6f82ba..e69de29bb2d1 100644
> > --- a/xen/arch/arm/Rules.mk
> > +++ b/xen/arch/arm/Rules.mk
> > @@ -1,93 +0,0 @@
> 
> As per the header here you're using git. Can you please arrange for
> this file movement (to arch.mk, and also for x86) to actually be
> expressed here as a rename, i.e. such that one can see what - if
> anything - changes?

That should be possible, I'll try to remember to add the right options.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel