diff mbox

[dpdk-dev,v2,3/4] Update library build process

Message ID 1412592755-3370-4-git-send-email-sergio.gonzalez.monroy@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Sergio Gonzalez Monroy Oct. 6, 2014, 10:52 a.m. UTC 
  Remove COMBINE_LIBS option and by default build:
- CONFIG_RTE_BUILD_SHARED_LIB=y : both individual and combined libraries
- CONFIG_RTE_BUILD_SHARED_LIB=n : single combined library

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 config/common_bsdapp   |  3 +--
 config/common_linuxapp |  3 +--
 mk/rte.lib.mk          | 73 ++++++++------------------------------------------
 mk/rte.sdkbuild.mk     |  2 +-
 mk/rte.sharelib.mk     | 41 +++++++++++-----------------
 mk/rte.vars.mk         |  4 ---
 6 files changed, 29 insertions(+), 97 deletions(-)

Comments

Matthew Hall Oct. 6, 2014, 8:46 p.m. UTC | #1 
On Mon, Oct 06, 2014 at 11:52:34AM +0100, Sergio Gonzalez Monroy wrote:
> Remove COMBINE_LIBS option and by default build:
> - CONFIG_RTE_BUILD_SHARED_LIB=y : both individual and combined libraries
> - CONFIG_RTE_BUILD_SHARED_LIB=n : single combined library

As previously discussed.,It would be better for backward-compatibility of 
people linking against DPDK, if the static lib could come out as both a 
combined library and separate individual libraries by default.

Otherwise everybody linking against DPDK has to change their code, and it 
won't be easy for them to move forward and backward against different DPDKs 
before and after this change.

Thanks,
Matthew.
Sergio Gonzalez Monroy Oct. 7, 2014, 9:55 a.m. UTC | #2 
On Mon, Oct 06, 2014 at 01:46:07PM -0700, Matthew Hall wrote:
> On Mon, Oct 06, 2014 at 11:52:34AM +0100, Sergio Gonzalez Monroy wrote:
> > Remove COMBINE_LIBS option and by default build:
> > - CONFIG_RTE_BUILD_SHARED_LIB=y : both individual and combined libraries
> > - CONFIG_RTE_BUILD_SHARED_LIB=n : single combined library
> 
> As previously discussed.,It would be better for backward-compatibility of 
> people linking against DPDK, if the static lib could come out as both a 
> combined library and separate individual libraries by default.
> 
> Otherwise everybody linking against DPDK has to change their code, and it 
> won't be easy for them to move forward and backward against different DPDKs 
> before and after this change.
> 
> Thanks,
> Matthew.

Hi Matthew,

My understanding from previous conversations is that backward-compatibility
is not being provided for the next release, as such it is unlikely that
backward-compatibility for compile/linking will be provided.

I don't see how this particular patch would force people to change their code,
though in all likelihood they will have to as a result of ABI changes in the
following release.
The only difference now would be when they link their applications against a
single library instead of multiple separated libraries.

Thanks,
Sergio
Thomas Monjalon Oct. 8, 2014, 3:38 p.m. UTC | #3 
Hi Sergio,

2014-10-06 11:52, Sergio Gonzalez Monroy:
> Remove COMBINE_LIBS option and by default build:
> - CONFIG_RTE_BUILD_SHARED_LIB=y : both individual and combined libraries
> - CONFIG_RTE_BUILD_SHARED_LIB=n : single combined library
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

I'd appreciate this hairy patch to be splitted and commented if possible.
It's not easy to review as is.

Thanks
Matthew Hall Oct. 8, 2014, 10:36 p.m. UTC | #4 
On Tue, Oct 07, 2014 at 10:55:11AM +0100, Sergio Gonzalez Monroy wrote:
> I don't see how this particular patch would force people to change their code,
> though in all likelihood they will have to as a result of ABI changes in the
> following release.
>
> The only difference now would be when they link their applications against a
> single library instead of multiple separated libraries.
> 
> Thanks,
> Sergio

Hi Sergio,

Changing all of your Makefiles does sound like changing code to me.

Matthew.
Sergio Gonzalez Monroy Oct. 9, 2014, 9:44 a.m. UTC | #5 
On Wed, Oct 08, 2014 at 03:36:55PM -0700, Matthew Hall wrote:
> On Tue, Oct 07, 2014 at 10:55:11AM +0100, Sergio Gonzalez Monroy wrote:
> > I don't see how this particular patch would force people to change their code,
> > though in all likelihood they will have to as a result of ABI changes in the
> > following release.
> >
> > The only difference now would be when they link their applications against a
> > single library instead of multiple separated libraries.
> > 
> > Thanks,
> > Sergio
> 
> Hi Sergio,
> 
> Changing all of your Makefiles does sound like changing code to me.
> 

Hi Matthew,

You are right, it is changing code.
What I was trying to say is that the impact of this patch is probrably going to
be one of the lowest as far as changing code for the next release.

Thanks,
Sergio

> Matthew.
diff mbox

Patch

diff --git a/config/common_bsdapp b/config/common_bsdapp
index eebd05b..65d2ecc 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -79,9 +79,8 @@  CONFIG_RTE_FORCE_INTRINSICS=n
 CONFIG_RTE_BUILD_SHARED_LIB=n
 
 #
-# Combine to one single library
+# Library name
 #
-CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_LIBNAME=intel_dpdk
 
 #
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 4713eb4..5bdcdf3 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -79,9 +79,8 @@  CONFIG_RTE_FORCE_INTRINSICS=n
 CONFIG_RTE_BUILD_SHARED_LIB=n
 
 #
-# Combine to one single library
+# Library name
 #
-CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_LIBNAME="intel_dpdk"
 
 #
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index f458258..e7420bf 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -66,48 +66,23 @@  LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
 CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
 endif
 
-O_TO_A = $(AR) crus $(LIB) $(OBJS-y)
-O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
-O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  AR $(@)")
-O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)"
-O_TO_A_DO = @set -e; \
-	echo $(O_TO_A_DISP); \
-	$(O_TO_A) && \
-	echo $(O_TO_A_CMD) > $(call exe2cmd,$(@))
-
 O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB)
-O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
+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; \
+	cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib; \
 	echo $(O_TO_S_DISP); \
 	$(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) $(LD_MULDEFS) -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
 
 #
 # Archive objects in .a file if needed
 #
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
 $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
+ifeq ($(RTE_BUILD_SHARED_LIB),y)
 	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
 	$(if $(D),\
 		@echo -n "$< -> $@ " ; \
@@ -116,51 +91,25 @@  $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
 		echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \
 		echo "depfile_newer=$(call boolean,$(depfile_newer)) ")
 	$(if $(or \
-		$(file_missing),\
-		$(call cmdline_changed,$(O_TO_S_STR)),\
-		$(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 $@)
-	$(if $(D),\
-	    @echo -n "$< -> $@ " ; \
-	    echo -n "file_missing=$(call boolean,$(file_missing)) " ; \
-	    echo -n "cmdline_changed=$(call boolean,$(call cmdline_changed,$(O_TO_A_STR))) " ; \
-	    echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \
-	    echo "depfile_newer=$(call boolean,$(depfile_newer)) ")
-	$(if $(or \
 	    $(file_missing),\
-	    $(call cmdline_changed,$(O_TO_A_STR)),\
+	    $(call cmdline_changed,$(O_TO_S_STR)),\
 	    $(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
+	    $(O_TO_S_DO))
+else
+	@set -e; \
+	cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib;
 endif
 
 #
 # install lib in $(RTE_OUTPUT)/lib
 #
 $(RTE_OUTPUT)/lib/$(LIB): $(LIB)
+ifeq ($(RTE_BUILD_SHARED_LIB),y)
 	@echo "  INSTALL-LIB $(LIB)"
 	@[ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib
 	$(Q)cp -f $(LIB) $(RTE_OUTPUT)/lib
+endif
 
 #
 # Clean all generated files
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index 3154457..f20ec1e 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -93,7 +93,7 @@  $(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 \
+	@if [ $@ = lib ]; then \
 		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
 	fi
 
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
index 73ce709..8fc6548 100644
--- a/mk/rte.sharelib.mk
+++ b/mk/rte.sharelib.mk
@@ -34,13 +34,11 @@  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
 
 ifeq ($(LINK_USING_CC),1)
 # Override the definition of LD here, since we're linking with CC
@@ -54,37 +52,28 @@  sharelib: $(LIB_ONE) FORCE
 
 OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS)\
+ifeq ($(RTE_BUILD_SHARED_LIB),y)
+O_TO_L = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS)\
 	-o $(RTE_OUTPUT)/lib/$(LIB_ONE)
-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)
+L_DISP=LD
+else
+O_TO_L = $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS)
+L_DISP=AR
+endif
+O_TO_L_STR = $(subst ','\'',$(O_TO_L)) #')# fix syntax highlight
+O_TO_L_DISP = $(if $(V),"$(O_TO_L_STR)","  $(L_DISP) $(@)")
+O_TO_L_CMD = "cmd_$@ = $(O_TO_L_STR)"
+O_TO_L_DO = @set -e; \
+	mkdir -p $(RTE_OUTPUT)/lib; \
+	echo $(O_TO_L_DISP); \
+	$(O_TO_L)
 
-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
+	$(O_TO_L_DO)
 
 #
 # Clean all generated files
diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
index 1e874ee..5bcc4a5 100644
--- a/mk/rte.vars.mk
+++ b/mk/rte.vars.mk
@@ -67,10 +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
   RTE_LIBNAME := $(CONFIG_RTE_LIBNAME:"%"=%)
   ifeq ($(RTE_LIBNAME),)
     RTE_LIBNAME := intel_dpdk