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

["Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com>]

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?

Sergio