Re: [Minios-devel] [UNIKRAFT PATCH 1/9] build: support for sharing libraries across multiple platforms

["Haibo Xu (Arm Technology China)" <Haibo.Xu@xxxxxxx>]

Hi Simon,

To speed the review, I used a Unikraft version(a7e7ad07ad28b71d) which is very 
close to this patch.
The next step is to rebase the patch to the staging HEAD, and send out patch V2.

Regards,
Haibo

> -----Original Message-----
> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Sent: 2019年10月14日 17:19
> To: Haibo Xu (Arm Technology China) <Haibo.Xu@xxxxxxx>; Florian Schmidt
> <florian.schmidt@xxxxxxxxx>; minios-devel@xxxxxxxxxxxxx
> Subject: Re: [Minios-devel] [UNIKRAFT PATCH 1/9] build: support for sharing
> libraries across multiple platforms
>
> Hey Haibo,
>
> thanks a lot for taking this patch series over. It is quite helpful to
> get this functionality finally in.
> I am pretty sure that at least this patch does not apply to the latest
> staging branch. At least it would not have the desired effect anymore.
> Sharan introduced a change in Makefile.build which should be used in
> order to implement this functionality. Against which Unikraft version
> did you do your review?
>
> Thanks,
>
> Simon
>
> On 14.10.19 09:56, Haibo Xu (Arm Technology China) wrote:
> > This patch looks good.
> >
> > Reviewed-by: Haibo Xu <haibo.xu@xxxxxxx>
> >
> > On 2019/6/3 22:56, Florian Schmidt wrote:
> >> Currently, if a library is added to the list of libraries-to-be-built
> >> more than once, this will lead to problems when make rules are
> >> installed, as well as in the linking step (trying to link multiple
> >> versions in, resulting in duplicate symbols).
> >> This patch aggregates all libraries first, then removes duplicates (via
> >> make's $(sort) function), and so makes sure no library is built more
> >> than once.
> >>
> >> Why would this happen? The best example is having a platform library
> used
> >> by two platforms, for example, using libpci for a kvm and a baremetal
> >> platform, which requires two addplatlib calls, one for each platform it
> >> is registered with.
> >>
> >> The same applies to cleaning, respectively.
> >>
> >> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> >> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> >> ---
> >>   support/build/Makefile.build | 21 +++++++++++----------
> >>   support/build/Makefile.clean |  8 ++++++--
> >>   2 files changed, 17 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/support/build/Makefile.build b/support/build/Makefile.build
> >> index 5679741d..6a0091c2 100644
> >> --- a/support/build/Makefile.build
> >> +++ b/support/build/Makefile.build
> >> @@ -26,16 +26,12 @@
> >>   ifneq ($(call qstrip,$(UK_LIBS) $(UK_LIBS-y)),)
> >>   $(foreach L,$(UK_LIBS) $(UK_LIBS-y), \
> >>   $(if $(call qstrip,$($(call uc,$(L))_SRCS) $($(call uc,$(L))_SRCS-y)), \
> >> -$(foreach S,$($(call uc,$(L))_SRCS) $($(call uc,$(L))_SRCS-y), \
> >> -$(eval $(call buildrule_libobj_multitarget,$(L),$(S))) \
> >> -); \
> >> -$(eval $(call buildrule_olib,$(L))); \
> >>   $(eval UK_OLIBS-y += $(call libname2olib,$(L))); \
> >> +$(eval UKBUILD_LIBS += $(L)); \
> >>   ) \
> >>   )
> >>   endif
> >>
> >> -
> >>   #################################################
> >>   #
> >>   # Install build rules for each enabled platform
> >> @@ -47,13 +43,18 @@ $(foreach P,$(UK_PLATS) $(UK_PLATS-y), \
> >>   $(if $(call qstrip,$($(call uc,$(P))_LIBS) $($(call uc,$(P))_LIBS-y)), \
> >>   $(foreach L,$($(call uc,$(P))_LIBS) $($(call uc,$(P))_LIBS-y), \
> >>   $(if $(call qstrip,$($(call uc,$(L))_SRCS) $($(call uc,$(L))_SRCS-y)), \
> >> +$(eval $(call uc,$(P))_OLIBS-y += $(call libname2olib,$(L))); \
> >> +$(eval UKBUILD_LIBS += $(L)); \
> >> +) \
> >> +) \
> >> +) \
> >> +)
> >> +
> >> +endif
> >> +
> >> +$(foreach L,$(sort $(UKBUILD_LIBS)), \
> >>   $(foreach S,$($(call uc,$(L))_SRCS) $($(call uc,$(L))_SRCS-y), \
> >>   $(eval $(call buildrule_libobj_multitarget,$(L),$(S))) \
> >>   ); \
> >>   $(eval $(call buildrule_olib,$(L))); \
> >> -$(eval $(call uc,$(P))_OLIBS-y    += $(call libname2olib,$(L))); \
> >> -) \
> >> -) \
> >> -); \
> >>   )
> >> -endif
> >> diff --git a/support/build/Makefile.clean b/support/build/Makefile.clean
> >> index 0ad3d155..a131119e 100644
> >> --- a/support/build/Makefile.clean
> >> +++ b/support/build/Makefile.clean
> >> @@ -25,7 +25,7 @@
> >>
> >>   ifneq ($(call qstrip,$(UK_LIBS) $(UK_LIBS-y)),)
> >>   $(foreach L,$(UK_LIBS) $(UK_LIBS-y), \
> >> -$(eval $(call cleanrule_lib,$(L))); \
> >> +$(eval UKCLEAN_LIBS += $(L)); \
> >>   )
> >>   endif
> >>
> >> @@ -40,8 +40,12 @@ ifneq ($(call qstrip,$(UK_PLATS) $(UK_PLATS-y)),)
> >>   $(foreach P,$(UK_PLATS) $(UK_PLATS-y), \
> >>   $(if $(call qstrip,$($(call uc,$(P))_LIBS) $($(call uc,$(P))_LIBS-y)), \
> >>   $(foreach L,$($(call uc,$(P))_LIBS) $($(call uc,$(P))_LIBS-y), \
> >> -$(eval $(call cleanrule_lib,$(L))); \
> >> +$(eval UKCLEAN_LIBS += $(L)); \
> >>   ) \
> >>   ) \
> >>   )
> >>   endif
> >> +
> >> +$(foreach L,$(sort $(UKCLEAN_LIBS)), \
> >> +$(eval $(call cleanrule_lib,$(L))); \
> >> +)
> >>
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended 
> recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
> >
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel