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

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hey Felipe,

thanks a lot for looking into this. Just to make sure: What this patch 
fixes is the case that you assign a single library to multiple 
platforms. It enables the case that platforms can share the same library 
build (imagine a kvm and a bare-metal platform could share a driver 
library for PCI bus). If I understood you right, you tested the case 
where you include a single library multiple times to the _same_ platform 
(here: kvm). This is is still something else and I would prefer keeping 
this to fail. I can't image how useful correcting the behaviour to make 
work would be. Maybe we should make the patch description a bit more 
clear about this.
Thanks,

Simon

On 23.05.19 15:36, Felipe Huici wrote:
> Hi Florian, Simon,
>
> If I duplicate a line in say plat/kvm/Makefile.uk such as $(eval $(call 
> addplatlib_s,kvm,libkvmpci,$(CONFIG_KVM_PCI))) I still get multiple definition 
> errors on the final link step. Perhaps some sort()s are missing in the linker 
> script(s)?
>
> Thanks,
>
> -- Felipe
>
> On 23.05.19, 11:16, "Minios-devel on behalf of Florian Schmidt" 
> <minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx on behalf of florian.schmidt@xxxxxxxxx> 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))); \
>      +)
>      --
>      2.21.0
>      
>      
>      _______________________________________________
>      Minios-devel mailing list
>      Minios-devel@xxxxxxxxxxxxxxxxxxxx
>      https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel