Re: [XEN PATCH v8 07/47] build: set ALL_OBJS to main Makefile; move prelink.o to main Makefile

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Dec 06, 2021 at 05:52:51PM +0100, Jan Beulich wrote:
> On 25.11.2021 14:39, Anthony PERARD wrote:
> 
> Nit: In the title, do you mean "set ALL_OBJS in main Makefile; ..."?
> 
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -285,8 +285,21 @@ CFLAGS += -flto
> >  LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
> >  endif
> >  
> > +# Note that link order matters!
> > +ALL_OBJS-y                := common/built_in.o
> > +ALL_OBJS-y                += drivers/built_in.o
> > +ALL_OBJS-y                += lib/built_in.o
> > +ALL_OBJS-y                += xsm/built_in.o
> > +ALL_OBJS-y                += arch/$(TARGET_ARCH)/built_in.o
> > +ALL_OBJS-$(CONFIG_CRYPTO) += crypto/built_in.o
> > +
> > +ALL_LIBS-y                := lib/lib.a
> > +
> >  include $(BASEDIR)/arch/$(TARGET_ARCH)/arch.mk
> >  
> > +export ALL_OBJS := $(ALL_OBJS-y)
> > +export ALL_LIBS := $(ALL_LIBS-y)
> 
> Who's the consumer of these exports? I ask because I don't consider the
> names very suitable for exporting, and hence I'd prefer to see their
> scope limited. If e.g. it's only a single make invocation where they
> need propagating, doing so on the command line might be better.

There seems to be only one consumer, "build.mk", and only the last
$(MAKE) call in the recipe "$(TARGET)". So, it's probably fine to set
both on the command line instead of using export. I'll have a look.

> > --- a/xen/arch/arm/Rules.mk
> > +++ b/xen/arch/arm/Rules.mk
> > @@ -0,0 +1,5 @@
> > +# head.o is built by descending into arch/arm/$(TARGET_SUBARCH), depends 
> > on the
> > +# part of $(ALL_OBJS) that will eventually recurse into $(TARGET_SUBARCH)/ 
> > and
> > +# build head.o
> > +arch/arm/$(TARGET_SUBARCH)/head.o: arch/arm/built_in.o
> > +arch/arm/$(TARGET_SUBARCH)/head.o: ;
> 
> Can't this be a single line:
> 
> arch/arm/$(TARGET_SUBARCH)/head.o: arch/arm/built_in.o ;

Sure.

> > @@ -235,7 +218,7 @@ $(TARGET).efi: FORCE
> >     echo '$(if $(filter y,$(XEN_BUILD_EFI)),xen.efi generation,EFI support) 
> > disabled'
> >  endif
> >  
> > -efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o
> > +# These should already have been rebuilt when building the prerequisite of 
> > "prelink.o"
> >  efi/buildid.o efi/relocs-dummy.o: ;
> 
> If the comment is true in all cases, do they really still need an empty
> rule?

Yes. Those two targets are unfortunately a prerequisite of "xen.efi", so
make will look for a rule to make them, and would use %.o:%.c without
this explicit rule.

Thanks,

-- 
Anthony PERARD