Re: [dpdk-dev] [PATCH v2 1/4] mk: Remove combined library and related options

[Neil Horman <nhorman@tuxdriver.com>]

On Wed, Mar 18, 2015 at 12:11:50PM +0000, Gonzalez Monroy, Sergio wrote:
> On 12/03/2015 16:27, Sergio Gonzalez Monroy wrote:
> >Remove CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LIBNAME.
> >
> >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >---
> >  config/common_bsdapp                        |   6 --
> >  config/common_linuxapp                      |   6 --
> >  config/defconfig_ppc_64-power8-linuxapp-gcc |   2 -
> >  lib/Makefile                                |   1 -
> >  mk/rte.app.mk                               |  12 ----
> >  mk/rte.lib.mk                               |  35 ----------
> >  mk/rte.sdkbuild.mk                          |   3 -
> >  mk/rte.sharelib.mk                          | 101 ----------------------------
> >  mk/rte.vars.mk                              |   9 ---
> >  9 files changed, 175 deletions(-)
> >  delete mode 100644 mk/rte.sharelib.mk
> >
> >diff --git a/config/common_bsdapp b/config/common_bsdapp
> >index 8ff4dc2..7ee5ecf 100644
> >--- a/config/common_bsdapp
> >+++ b/config/common_bsdapp
> >@@ -79,12 +79,6 @@ CONFIG_RTE_FORCE_INTRINSICS=n
> >  CONFIG_RTE_BUILD_SHARED_LIB=n
> >  #
> >-# Combine to one single library
> >-#
> >-CONFIG_RTE_BUILD_COMBINE_LIBS=n
> >-CONFIG_RTE_LIBNAME=intel_dpdk
> >-
> >-#
> >  # Compile Environment Abstraction Layer
> >  #
> >  CONFIG_RTE_LIBRTE_EAL=y
> >diff --git a/config/common_linuxapp b/config/common_linuxapp
> >index 97f1c9e..ae13805 100644
> >--- a/config/common_linuxapp
> >+++ b/config/common_linuxapp
> >@@ -79,12 +79,6 @@ CONFIG_RTE_FORCE_INTRINSICS=n
> >  CONFIG_RTE_BUILD_SHARED_LIB=n
> >  #
> >-# Combine to one single library
> >-#
> >-CONFIG_RTE_BUILD_COMBINE_LIBS=n
> >-CONFIG_RTE_LIBNAME="intel_dpdk"
> >-
> >-#
> >  # Compile Environment Abstraction Layer
> >  #
> >  CONFIG_RTE_LIBRTE_EAL=y
> >diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-linuxapp-gcc
> >index d97a885..f1af518 100644
> >--- a/config/defconfig_ppc_64-power8-linuxapp-gcc
> >+++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
> >@@ -39,8 +39,6 @@ CONFIG_RTE_ARCH_64=y
> >  CONFIG_RTE_TOOLCHAIN="gcc"
> >  CONFIG_RTE_TOOLCHAIN_GCC=y
> >-CONFIG_RTE_LIBNAME="powerpc_dpdk"
> >-
> >  # Note: Power doesn't have this support
> >  CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
> >diff --git a/lib/Makefile b/lib/Makefile
> >index d94355d..c34cf2f 100644
> >--- a/lib/Makefile
> >+++ b/lib/Makefile
> >@@ -77,5 +77,4 @@ DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
> >  DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += librte_ivshmem
> >  endif
> >-include $(RTE_SDK)/mk/rte.sharelib.mk
> >  include $(RTE_SDK)/mk/rte.subdir.mk
> >diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> >index 63a41e2..e2baa49 100644
> >--- a/mk/rte.app.mk
> >+++ b/mk/rte.app.mk
> >@@ -61,12 +61,6 @@ ifeq ($(NO_AUTOLIBS),)
> >  LDLIBS += --whole-archive
> >-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
> >-LDLIBS += -l$(RTE_LIBNAME)
> >-endif
> >-
> >-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
> >-
> >  ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
> >  LDLIBS += -lrte_distributor
> >  endif
> >@@ -137,8 +131,6 @@ ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
> >  LDLIBS += -lrte_vhost
> >  endif
> >-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
> >-
> >  ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> >  LDLIBS += -lpcap
> >  endif
> >@@ -153,8 +145,6 @@ endif
> >  LDLIBS += --start-group
> >-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
> >-
> >  ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> >  LDLIBS += -lrte_kvargs
> >  endif
> >@@ -253,8 +243,6 @@ endif
> >  endif # plugins
> >-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
> >-
> >  LDLIBS += $(EXECENV_LDLIBS)
> >  LDLIBS += --end-group
> >diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> >index 0d7482d..d96101a 100644
> >--- a/mk/rte.lib.mk
> >+++ b/mk/rte.lib.mk
> >@@ -87,24 +87,6 @@ O_TO_S_DO = @set -e; \
> >  	$(O_TO_S) && \
> >  	echo $(O_TO_S_CMD) > $(call exe2cmd,$(@))
> >-ifeq ($(RTE_BUILD_SHARED_LIB),n)
> >-O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y)
> >-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
> >-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  AR_C $(@)")
> >-O_TO_C_DO = @set -e; \
> >-	$(lib_dir) \
> >-	$(copy_obj)
> >-else
> >-O_TO_C = $(LD) -shared $(OBJS-y) -o $(LIB_ONE)
> >-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
> >-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
> >-O_TO_C_DO = @set -e; \
> >-	$(lib_dir) \
> >-	$(copy_obj)
> >-endif
> >-
> >-copy_obj = cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib;
> >-lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib;
> >  -include .$(LIB).cmd
> >  #
> >@@ -129,15 +111,6 @@ endif
> >  		$(depfile_missing),\
> >  		$(depfile_newer)),\
> >  		$(O_TO_S_DO))
> >-
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-	$(if $(or \
> >-        $(file_missing),\
> >-        $(call cmdline_changed,$(O_TO_C_STR)),\
> >-        $(depfile_missing),\
> >-        $(depfile_newer)),\
> >-        $(O_TO_C_DO))
> >-endif
> >  else
> >  $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
> >  	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> >@@ -153,14 +126,6 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
> >  	    $(depfile_missing),\
> >  	    $(depfile_newer)),\
> >  	    $(O_TO_A_DO))
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-	$(if $(or \
> >-        $(file_missing),\
> >-        $(call cmdline_changed,$(O_TO_C_STR)),\
> >-        $(depfile_missing),\
> >-        $(depfile_newer)),\
> >-        $(O_TO_C_DO))
> >-endif
> >  endif
> >  #
> >diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
> >index 3154457..2b24e74 100644
> >--- a/mk/rte.sdkbuild.mk
> >+++ b/mk/rte.sdkbuild.mk
> >@@ -93,9 +93,6 @@ $(ROOTDIRS-y):
> >  	@[ -d $(BUILDDIR)/$@ ] || mkdir -p $(BUILDDIR)/$@
> >  	@echo "== Build $@"
> >  	$(Q)$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile -C $(BUILDDIR)/$@ all
> >-	@if [ $@ = lib -a $(RTE_BUILD_COMBINE_LIBS) = y ]; then \
> >-		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
> >-	fi
> >  %_clean:
> >  	@echo "== Clean $*"
> >diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
> >deleted file mode 100644
> >index de53558..0000000
> >--- a/mk/rte.sharelib.mk
> >+++ /dev/null
> >@@ -1,101 +0,0 @@
> >-#   BSD LICENSE
> >-#
> >-#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> >-#   All rights reserved.
> >-#
> >-#   Redistribution and use in source and binary forms, with or without
> >-#   modification, are permitted provided that the following conditions
> >-#   are met:
> >-#
> >-#     * Redistributions of source code must retain the above copyright
> >-#       notice, this list of conditions and the following disclaimer.
> >-#     * Redistributions in binary form must reproduce the above copyright
> >-#       notice, this list of conditions and the following disclaimer in
> >-#       the documentation and/or other materials provided with the
> >-#       distribution.
> >-#     * Neither the name of Intel Corporation nor the names of its
> >-#       contributors may be used to endorse or promote products derived
> >-#       from this software without specific prior written permission.
> >-#
> >-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> >-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> >-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> >-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> >-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> >-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >-
> >-include $(RTE_SDK)/mk/internal/rte.build-pre.mk
> >-
> >-# VPATH contains at least SRCDIR
> >-VPATH += $(SRCDIR)
> >-
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-ifeq ($(RTE_BUILD_SHARED_LIB),y)
> >-LIB_ONE := lib$(RTE_LIBNAME).so
> >-else
> >-LIB_ONE := lib$(RTE_LIBNAME).a
> >-endif
> >-endif
> >-
> >-.PHONY:sharelib
> >-sharelib: $(LIB_ONE) FORCE
> >-
> >-OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
> >-
> >-ifeq ($(LINK_USING_CC),1)
> >-# Override the definition of LD here, since we're linking with CC
> >-LD := $(CC) $(CPU_CFLAGS)
> >-O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
> >-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> >-else
> >-O_TO_S = $(LD) $(CPU_LDFLAGS) \
> >-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> >-endif
> >-
> >-O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
> >-O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
> >-O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)"
> >-O_TO_S_DO = @set -e; \
> >-    echo $(O_TO_S_DISP); \
> >-    $(O_TO_S)
> >-
> >-O_TO_A =  $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS)
> >-O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
> >-O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  LD $(@)")
> >-O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)"
> >-O_TO_A_DO = @set -e; \
> >-    echo $(O_TO_A_DISP); \
> >-    $(O_TO_A)
> >-#
> >-# Archive objects to share library
> >-#
> >-
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-ifeq ($(RTE_BUILD_SHARED_LIB),y)
> >-$(LIB_ONE): FORCE
> >-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> >-	$(O_TO_S_DO)
> >-else
> >-$(LIB_ONE): FORCE
> >-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> >-	$(O_TO_A_DO)
> >-endif
> >-endif
> >-
> >-#
> >-# Clean all generated files
> >-#
> >-.PHONY: clean
> >-clean: _postclean
> >-
> >-.PHONY: doclean
> >-doclean:
> >-	$(Q)rm -rf $(LIB_ONE)
> >-
> >-.PHONY: FORCE
> >-FORCE:
> >diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
> >index d2f01b6..7b6f53d 100644
> >--- a/mk/rte.vars.mk
> >+++ b/mk/rte.vars.mk
> >@@ -67,15 +67,6 @@ ifneq ($(BUILDING_RTE_SDK),)
> >    ifeq ($(RTE_BUILD_SHARED_LIB),)
> >      RTE_BUILD_SHARED_LIB := n
> >    endif
> >-  RTE_BUILD_COMBINE_LIBS := $(CONFIG_RTE_BUILD_COMBINE_LIBS:"%"=%)
> >-  ifeq ($(RTE_BUILD_COMBINE_LIBS),)
> >-    RTE_BUILD_COMBINE_LIBS := n
> >-  endif
> >-endif
> >-
> >-RTE_LIBNAME := $(CONFIG_RTE_LIBNAME:"%"=%)
> >-ifeq ($(RTE_LIBNAME),)
> >-RTE_LIBNAME := intel_dpdk
> >  endif
> >  # RTE_TARGET is deducted from config when we are building the SDK.
> Given that the patch to remove combined libraries is not welcome, I'll try
> to explain the current situation so we can agree on the way forward.
> 
> Currently we have build config option for shared libraries and combined
> libraries. Thus, this results in four possible combinations when building
> dpdk:
> - not combined static
> - not combined shared
> - combined static
> - combined shared
> 
> The makefile rules/targets for combined are different than for not combined.
> Thus, we currently have two different files for archive/linking (rte.lib.mk
> and rte.sharelib.mk).
> 
> Since having versioning, combined shared libraries build will be broken the
> moment we add a versioned API, as we do not have a global version map that
> we use when linking such library.
> Also in my opinion, we would want to prevent users linking against a
> combined libdpdk.so that may have different features built-in, with the
> corresponding debugging difficulties when users
> report different problems/errors. I think this would defeat many of the
> advantages of using shared libraries.
> 
> By removing the combined library build option, we would simplify the build
> system with only two possible choices:
> - static
> - shared
> 
> This would allow us to remove one file (rte.sharelib.mk) and have a single
> file with archive/linking rules.
> 
> For the convenience of linking against a single library instead of the
> multiple dpdk libraries, there are a few ways to go around it:
>  - for combined static lib, we can either have a script to re-archive all
> libraries into a single/combined library (ie. extract all archives into one
> directory, the re-archive all objects into a combined library),
>   or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
> - for combined shared lib, we can use a linker script (ie. INPUT ( -lrte_eal
> -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a global
> version map (either somehow merging all independent version maps
> or maintaining a global version map).
> 
> My preference would be to remove the combined libs as a build config option,
> then either add scripts to create those linker scripts or document it so
> users know how to create their own linker scripts.
> This would simplify the build process and still be able to provide the
> convenience of the combined library by using a linker script.
> 
> Comments?
> 
I agree, the combined libs config option creates significant build environment
complications that don't need to be there (given that in both the static and
shared cases, post processing gives us the same convieniences).

The comments on this thread have suggested that the combined libraries are
mostly seen as adventageous for their convienience, so if we can provide that
convienience in another way that solves other problems we have (versioning of a
combined library), and reduce build complexity, we should do it.

Neil

> Sergio
>