* [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. @ 2015-04-29 15:25 Keith Wiles 2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Keith Wiles @ 2015-04-29 15:25 UTC (permalink / raw) To: dev Trying to simplify the ifdefs in rte.app.mk to make the code more readable and maintainable by moving LDLIBS variable to use the same style as LDLIBS-y being used in the rest of the code. Added a new variable called EXTRA_LDLIBS to be used by example apps instead of using LDLIBS directly. Signed-off-by: Keith Wiles <keith.wiles@intel.com> --- examples/dpdk_qat/Makefile | 4 +- examples/vm_power_manager/Makefile | 2 +- mk/rte.app.mk | 254 ++++++++++--------------------------- mk/rte.hostapp.mk | 4 +- mk/rte.shared.mk | 12 +- 5 files changed, 77 insertions(+), 199 deletions(-) diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile index f1e06a1..90ca1d3 100644 --- a/examples/dpdk_qat/Makefile +++ b/examples/dpdk_qat/Makefile @@ -77,8 +77,8 @@ else ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a endif -LDLIBS += -L$(ICP_ROOT)/build -LDLIBS += $(ICP_LIBRARY_PATH) \ +EXTRA_LDLIBS += -L$(ICP_ROOT)/build +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ -lz \ -losal \ -ladf_proxy \ diff --git a/examples/vm_power_manager/Makefile b/examples/vm_power_manager/Makefile index 113dbc4..8fb78d4 100644 --- a/examples/vm_power_manager/Makefile +++ b/examples/vm_power_manager/Makefile @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ CFLAGS += $(WERROR_FLAGS) -LDLIBS += -lvirt +EXTRA_LDLIBS += -lvirt # workaround for a gcc bug with noreturn attribute # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 62a76ae..c41de82 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -1,7 +1,7 @@ # BSD LICENSE # -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. -# Copyright(c) 2014 6WIND S.A. +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. +# Copyright(c) 2015 6WIND S.A. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) endif # default path for libs -LDLIBS += -L$(RTE_SDK_BIN)/lib +LDLIBS-y += -L$(RTE_SDK_BIN)/lib # # Include libraries depending on config if NO_AUTOLIBS is not set @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib # ifeq ($(NO_AUTOLIBS),) -LDLIBS += --whole-archive +LDLIBS-y += --whole-archive -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) -LDLIBS += -l$(RTE_LIBNAME) -endif +LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS) += -l$(RTE_LIBNAME) ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) -LDLIBS += -lrte_distributor -endif - -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) -LDLIBS += -lrte_reorder -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor +LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) -LDLIBS += -lrte_kni -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) += -lrte_kni +LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += -lrte_ivshmem endif -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) -LDLIBS += -lrte_ivshmem -endif -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline +LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table +LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port +LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer +LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash +LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats +LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm +LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power +LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl +LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) -LDLIBS += -lrte_pipeline -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) -LDLIBS += -lrte_table -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) -LDLIBS += -lrte_port -endif - -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y) -LDLIBS += -lrte_timer -endif - -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y) -LDLIBS += -lrte_hash -endif - -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y) -LDLIBS += -lrte_jobstats -endif - -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) -LDLIBS += -lrte_lpm -endif - -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y) -LDLIBS += -lrte_power -endif - -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y) -LDLIBS += -lrte_acl -endif - -ifeq ($(CONFIG_RTE_LIBRTE_METER),y) -LDLIBS += -lrte_meter -endif - -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) -LDLIBS += -lrte_sched -LDLIBS += -lm -LDLIBS += -lrt -endif - -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y) -LDLIBS += -lrte_vhost -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) -LDLIBS += -lpcap -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lpcap -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn) -LDLIBS += -lfuse +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n) +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse endif -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) -LDLIBS += -libverbs -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -libverbs -LDLIBS += --start-group +LDLIBS-y += --start-group ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y) -LDLIBS += -lrte_kvargs -endif - -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y) -LDLIBS += -lrte_mbuf -endif - -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y) -LDLIBS += -lrte_ip_frag -endif - -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y) -LDLIBS += -lethdev -endif - -ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) -LDLIBS += -lrte_malloc -endif - -ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y) -LDLIBS += -lrte_mempool -endif - -ifeq ($(CONFIG_RTE_LIBRTE_RING),y) -LDLIBS += -lrte_ring -endif - -ifeq ($(CONFIG_RTE_LIBRTE_EAL),y) -LDLIBS += -lrte_eal -endif - -ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y) -LDLIBS += -lrte_cmdline -endif - -ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y) -LDLIBS += -lrte_cfgfile -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y) -LDLIBS += -lrte_pmd_bond -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y) -LDLIBS += -lrte_pmd_xenvirt -LDLIBS += -lxenstore -endif +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER) += -lethdev +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC) += -lrte_malloc +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += -lrte_mempool +LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond + +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lxenstore ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) # plugins (link only if static libraries) -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) -LDLIBS += -lrte_pmd_vmxnet3_uio -endif - -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) -LDLIBS += -lrte_pmd_virtio -endif - -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y) -LDLIBS += -lrte_pmd_enic -endif - -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) -LDLIBS += -lrte_pmd_i40e -endif - -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y) -LDLIBS += -lrte_pmd_fm10k -endif - -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) -LDLIBS += -lrte_pmd_ixgbe -endif - -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y) -LDLIBS += -lrte_pmd_e1000 -endif - -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) -LDLIBS += -lrte_pmd_mlx4 -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) -LDLIBS += -lrte_pmd_ring -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) -LDLIBS += -lrte_pmd_pcap -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y) -LDLIBS += -lrte_pmd_af_packet -endif - -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y) -LDLIBS += -lrte_pmd_null -endif - -endif # plugins +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null + +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS -LDLIBS += $(EXECENV_LDLIBS) - -LDLIBS += --end-group - -LDLIBS += --no-whole-archive +LDLIBS-y += $(EXECENV_LDLIBS) +LDLIBS-y += --end-group +LDLIBS-y += --no-whole-archive endif # ifeq ($(NO_AUTOLIBS),) -LDLIBS += $(CPU_LDLIBS) +LDLIBS-y += $(EXTRA_LDLIBS) .PHONY: all all: install @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1) override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \ -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call linkerprefix,$(LDFLAGS)) \ - $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS)) + $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y)) else O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ - -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS) + -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y) endif O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," LD $(@)") @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \ -include .$(APP).cmd # path where libraries are retrieved -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS))) -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y))) +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) # list of .a files that are linked to this application -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS))) +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y))) # list of found libraries files (useful for deps). If not found, the # library is silently ignored and dep won't be checked diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk index c44d0f8..51e8c1d 100644 --- a/mk/rte.hostapp.mk +++ b/mk/rte.hostapp.mk @@ -58,7 +58,7 @@ build: _postbuild exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \ - $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS) + $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y) O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@)") O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)" @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \ # list of .a files that are linked to this application LDLIBS_FILES := $(wildcard \ $(addprefix $(RTE_OUTPUT)/lib/, \ - $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))))) + $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))))) # # Compile executable file if needed diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk index fc6b0b4..56f20bb 100644 --- a/mk/rte.shared.mk +++ b/mk/rte.shared.mk @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) ifeq ($(LINK_USING_CC),1) override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ - -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS)) + -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y)) else O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ - -shared -o $@ $(OBJS-y) $(LDLIBS) + -shared -o $@ $(OBJS-y) $(LDLIBS-y) endif O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \ -include .$(SHARED).cmd # path where libraries are retrieved -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS))) -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y))) +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) # list of .a files that are linked to this application -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS))) +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y))) # list of found libraries files (useful for deps). If not found, the # library is silently ignored and dep won't be checked -- 2.3.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable 2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles @ 2015-04-29 15:25 ` Keith Wiles 2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon 2015-04-30 9:45 ` Olivier MATZ 2 siblings, 0 replies; 12+ messages in thread From: Keith Wiles @ 2015-04-29 15:25 UTC (permalink / raw) To: dev Signed-off-by: Keith Wiles <keith.wiles@intel.com> --- doc/build-sdk-quick.txt | 1 + doc/guides/prog_guide/build_app.rst | 2 +- doc/guides/prog_guide/dev_kit_build_system.rst | 2 ++ doc/guides/prog_guide/dev_kit_root_make_help.rst | 2 +- 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/build-sdk-quick.txt b/doc/build-sdk-quick.txt index 041a40e..26d5442 100644 --- a/doc/build-sdk-quick.txt +++ b/doc/build-sdk-quick.txt @@ -13,6 +13,7 @@ Build variables EXTRA_CPPFLAGS preprocessor options EXTRA_CFLAGS compiler options EXTRA_LDFLAGS linker options + EXTRA_LDLIBS linker libary options RTE_KERNELDIR linux headers path CROSS toolchain prefix V verbose diff --git a/doc/guides/prog_guide/build_app.rst b/doc/guides/prog_guide/build_app.rst index d4a3261..0680dee 100644 --- a/doc/guides/prog_guide/build_app.rst +++ b/doc/guides/prog_guide/build_app.rst @@ -123,6 +123,6 @@ chapter for details. * CPPFLAGS: The flags to use to provide flags to the C preprocessor (only useful when assembling .S files) -* LDLIBS: A list of libraries to link with (for example, -L /path/to/libfoo - lfoo) +* LDLIBS-y: A list of libraries to link with (for example, -L /path/to/libfoo - lfoo) Use EXTRA_LDLIBS to add more options. * NO_AUTOLIBS: If set, the libraries provided by the framework will not be included in the LDLIBS variable automatically. diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst b/doc/guides/prog_guide/dev_kit_build_system.rst index cf5c96f..b8ef167 100644 --- a/doc/guides/prog_guide/dev_kit_build_system.rst +++ b/doc/guides/prog_guide/dev_kit_build_system.rst @@ -413,6 +413,8 @@ Variables that Can be Set/Overridden by the User in a Makefile or Command Line * EXTRA_LDFLAGS: The content of this variable is appended after LDFLAGS when linking. +* EXTRA_LDLIBS: The content of this variable is appended after LDLIBS when linking. + * EXTRA_ASFLAGS: The content of this variable is appended after ASFLAGS when assembling. * EXTRA_CPPFLAGS: The content of this variable is appended after CPPFLAGS when using a C preprocessor on assembly files. diff --git a/doc/guides/prog_guide/dev_kit_root_make_help.rst b/doc/guides/prog_guide/dev_kit_root_make_help.rst index 4f30192..fdc5fea 100644 --- a/doc/guides/prog_guide/dev_kit_root_make_help.rst +++ b/doc/guides/prog_guide/dev_kit_root_make_help.rst @@ -205,7 +205,7 @@ The following variables can be specified on the command line: Enable dependency debugging. This provides some useful information about why a target is built or not. -* EXTRA_CFLAGS=, EXTRA_LDFLAGS=, EXTRA_ASFLAGS=, EXTRA_CPPFLAGS= +* EXTRA_CFLAGS=, EXTRA_LDFLAGS=, EXTRA_LDLIBS=, EXTRA_ASFLAGS=, EXTRA_CPPFLAGS= Append specific compilation, link or asm flags. -- 2.3.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles 2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles @ 2015-04-29 17:17 ` Thomas Monjalon 2015-04-29 17:49 ` Wiles, Keith 2015-04-30 9:45 ` Olivier MATZ 2 siblings, 1 reply; 12+ messages in thread From: Thomas Monjalon @ 2015-04-29 17:17 UTC (permalink / raw) To: Keith Wiles; +Cc: dev 2015-04-29 10:25, Keith Wiles: > -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > -# Copyright(c) 2014 6WIND S.A. > +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. > +# Copyright(c) 2015 6WIND S.A. A correct update of copyright dates should be - 2014 + 2014-2015 Please do not send a new version only for this change. 4 versions of the same patch in 1 day should be enough :) Let's wait for more feedback on the idea of your patch, especially from the maintainer (Olivier). ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon @ 2015-04-29 17:49 ` Wiles, Keith 0 siblings, 0 replies; 12+ messages in thread From: Wiles, Keith @ 2015-04-29 17:49 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev On 4/29/15, 12:17 PM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote: >2015-04-29 10:25, Keith Wiles: >> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >> -# Copyright(c) 2014 6WIND S.A. >> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >> +# Copyright(c) 2015 6WIND S.A. > >A correct update of copyright dates should be >- 2014 >+ 2014-2015 If I have to send a new patch for other stuff I will update the copyright, if not does that mean I have send a patch at some point or can it be handled. > >Please do not send a new version only for this change. >4 versions of the same patch in 1 day should be enough :) Multiple versions in one day I was trying to get the record for the most updates in one day. :-) >Let's wait for more feedback on the idea of your patch, especially from >the maintainer (Olivier). ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles 2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles 2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon @ 2015-04-30 9:45 ` Olivier MATZ 2015-04-30 13:24 ` Wiles, Keith 2 siblings, 1 reply; 12+ messages in thread From: Olivier MATZ @ 2015-04-30 9:45 UTC (permalink / raw) To: Keith Wiles, dev Hi Keith, Thank you for submitting a clean-up. Please see some comments below. On 04/29/2015 05:25 PM, Keith Wiles wrote: > Trying to simplify the ifdefs in rte.app.mk to make the code > more readable and maintainable by moving LDLIBS variable to use > the same style as LDLIBS-y being used in the rest of the code. > > Added a new variable called EXTRA_LDLIBS to be used by example apps > instead of using LDLIBS directly. If I understand well, the goal of this patch is only a cleanup in rte.app.mk, but at the end, it changes the makefile user "API", which could probably be a problem for applications using the dpdk makefile framework. Why not just having an temporary internal variable (let's say _LDLIBS-y) that would allow to do the clean-up without modifying the user interface? Also, with your patch, the approach for EXTRA_LDLIBS would be different than CFLAGS or LDFLAGS: - CFLAGS/LDFLAGS are in Makefiles only - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line to add flags to the default ones I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, keeping a compatibility with existing application Makefiles. What do you think? Regards, Olivier > > Signed-off-by: Keith Wiles <keith.wiles@intel.com> > --- > examples/dpdk_qat/Makefile | 4 +- > examples/vm_power_manager/Makefile | 2 +- > mk/rte.app.mk | 254 ++++++++++--------------------------- > mk/rte.hostapp.mk | 4 +- > mk/rte.shared.mk | 12 +- > 5 files changed, 77 insertions(+), 199 deletions(-) > > diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile > index f1e06a1..90ca1d3 100644 > --- a/examples/dpdk_qat/Makefile > +++ b/examples/dpdk_qat/Makefile > @@ -77,8 +77,8 @@ else > ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a > endif > > -LDLIBS += -L$(ICP_ROOT)/build > -LDLIBS += $(ICP_LIBRARY_PATH) \ > +EXTRA_LDLIBS += -L$(ICP_ROOT)/build > +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ > -lz \ > -losal \ > -ladf_proxy \ > diff --git a/examples/vm_power_manager/Makefile b/examples/vm_power_manager/Makefile > index 113dbc4..8fb78d4 100644 > --- a/examples/vm_power_manager/Makefile > +++ b/examples/vm_power_manager/Makefile > @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c > CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ > CFLAGS += $(WERROR_FLAGS) > > -LDLIBS += -lvirt > +EXTRA_LDLIBS += -lvirt > > # workaround for a gcc bug with noreturn attribute > # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > index 62a76ae..c41de82 100644 > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -1,7 +1,7 @@ > # BSD LICENSE > # > -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > -# Copyright(c) 2014 6WIND S.A. > +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. > +# Copyright(c) 2015 6WIND S.A. > # All rights reserved. > # > # Redistribution and use in source and binary forms, with or without > @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) > endif > > # default path for libs > -LDLIBS += -L$(RTE_SDK_BIN)/lib > +LDLIBS-y += -L$(RTE_SDK_BIN)/lib > > # > # Include libraries depending on config if NO_AUTOLIBS is not set > @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib > # > ifeq ($(NO_AUTOLIBS),) > > -LDLIBS += --whole-archive > +LDLIBS-y += --whole-archive > > -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) > -LDLIBS += -l$(RTE_LIBNAME) > -endif > +LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS) += -l$(RTE_LIBNAME) > > ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) > > -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) > -LDLIBS += -lrte_distributor > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) > -LDLIBS += -lrte_reorder > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor > +LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder > > -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > -LDLIBS += -lrte_kni > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) += -lrte_kni > +LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += -lrte_ivshmem > endif > > -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) > -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > -LDLIBS += -lrte_ivshmem > -endif > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline > +LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table > +LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port > +LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer > +LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash > +LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats > +LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm > +LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power > +LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > +LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter > > -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) > -LDLIBS += -lrte_pipeline > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched > +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm > +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt > > -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) > -LDLIBS += -lrte_table > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) > -LDLIBS += -lrte_port > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y) > -LDLIBS += -lrte_timer > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y) > -LDLIBS += -lrte_hash > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y) > -LDLIBS += -lrte_jobstats > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) > -LDLIBS += -lrte_lpm > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y) > -LDLIBS += -lrte_power > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y) > -LDLIBS += -lrte_acl > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_METER),y) > -LDLIBS += -lrte_meter > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) > -LDLIBS += -lrte_sched > -LDLIBS += -lm > -LDLIBS += -lrt > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y) > -LDLIBS += -lrte_vhost > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost > > endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS > > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) > -LDLIBS += -lpcap > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lpcap > > -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn) > -LDLIBS += -lfuse > +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n) > +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse > endif > > -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) > -LDLIBS += -libverbs > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -libverbs > > -LDLIBS += --start-group > +LDLIBS-y += --start-group > > ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) > > -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y) > -LDLIBS += -lrte_kvargs > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y) > -LDLIBS += -lrte_mbuf > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y) > -LDLIBS += -lrte_ip_frag > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y) > -LDLIBS += -lethdev > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) > -LDLIBS += -lrte_malloc > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y) > -LDLIBS += -lrte_mempool > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_RING),y) > -LDLIBS += -lrte_ring > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_EAL),y) > -LDLIBS += -lrte_eal > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y) > -LDLIBS += -lrte_cmdline > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y) > -LDLIBS += -lrte_cfgfile > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y) > -LDLIBS += -lrte_pmd_bond > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y) > -LDLIBS += -lrte_pmd_xenvirt > -LDLIBS += -lxenstore > -endif > +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs > +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf > +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag > +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER) += -lethdev > +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC) += -lrte_malloc > +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += -lrte_mempool > +LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring > +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal > +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline > +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond > + > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lxenstore > > ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) > # plugins (link only if static libraries) > > -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) > -LDLIBS += -lrte_pmd_vmxnet3_uio > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) > -LDLIBS += -lrte_pmd_virtio > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y) > -LDLIBS += -lrte_pmd_enic > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) > -LDLIBS += -lrte_pmd_i40e > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y) > -LDLIBS += -lrte_pmd_fm10k > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) > -LDLIBS += -lrte_pmd_ixgbe > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y) > -LDLIBS += -lrte_pmd_e1000 > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) > -LDLIBS += -lrte_pmd_mlx4 > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) > -LDLIBS += -lrte_pmd_ring > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) > -LDLIBS += -lrte_pmd_pcap > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y) > -LDLIBS += -lrte_pmd_af_packet > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y) > -LDLIBS += -lrte_pmd_null > -endif > - > -endif # plugins > +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio > +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio > +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic > +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e > +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k > +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe > +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 > +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet > +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null > + > +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) > > endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS > > -LDLIBS += $(EXECENV_LDLIBS) > - > -LDLIBS += --end-group > - > -LDLIBS += --no-whole-archive > +LDLIBS-y += $(EXECENV_LDLIBS) > +LDLIBS-y += --end-group > +LDLIBS-y += --no-whole-archive > > endif # ifeq ($(NO_AUTOLIBS),) > > -LDLIBS += $(CPU_LDLIBS) > +LDLIBS-y += $(EXTRA_LDLIBS) > > .PHONY: all > all: install > @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1) > override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) > O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \ > -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call linkerprefix,$(LDFLAGS)) \ > - $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS)) > + $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y)) > else > O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ > - -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS) > + -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y) > endif > O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight > O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," LD $(@)") > @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \ > -include .$(APP).cmd > > # path where libraries are retrieved > -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS))) > -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) > +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y))) > +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) > > # list of .a files that are linked to this application > -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) > -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS))) > +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) > +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y))) > > # list of found libraries files (useful for deps). If not found, the > # library is silently ignored and dep won't be checked > diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk > index c44d0f8..51e8c1d 100644 > --- a/mk/rte.hostapp.mk > +++ b/mk/rte.hostapp.mk > @@ -58,7 +58,7 @@ build: _postbuild > exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) > > O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \ > - $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS) > + $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y) > O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight > O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@)") > O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)" > @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \ > # list of .a files that are linked to this application > LDLIBS_FILES := $(wildcard \ > $(addprefix $(RTE_OUTPUT)/lib/, \ > - $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))))) > + $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))))) > > # > # Compile executable file if needed > diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk > index fc6b0b4..56f20bb 100644 > --- a/mk/rte.shared.mk > +++ b/mk/rte.shared.mk > @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) > ifeq ($(LINK_USING_CC),1) > override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) > O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ > - -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS)) > + -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y)) > else > O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ > - -shared -o $@ $(OBJS-y) $(LDLIBS) > + -shared -o $@ $(OBJS-y) $(LDLIBS-y) > endif > > O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight > @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \ > -include .$(SHARED).cmd > > # path where libraries are retrieved > -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS))) > -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) > +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y))) > +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) > > # list of .a files that are linked to this application > -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) > -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS))) > +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) > +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y))) > > # list of found libraries files (useful for deps). If not found, the > # library is silently ignored and dep won't be checked > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-30 9:45 ` Olivier MATZ @ 2015-04-30 13:24 ` Wiles, Keith 2015-04-30 13:38 ` Olivier MATZ 0 siblings, 1 reply; 12+ messages in thread From: Wiles, Keith @ 2015-04-30 13:24 UTC (permalink / raw) To: Olivier MATZ, dev On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: >Hi Keith, > >Thank you for submitting a clean-up. Please see some comments below. > >On 04/29/2015 05:25 PM, Keith Wiles wrote: >> Trying to simplify the ifdefs in rte.app.mk to make the code >> more readable and maintainable by moving LDLIBS variable to use >> the same style as LDLIBS-y being used in the rest of the code. >> >> Added a new variable called EXTRA_LDLIBS to be used by example apps >> instead of using LDLIBS directly. > >If I understand well, the goal of this patch is only a cleanup in >rte.app.mk, but at the end, it changes the makefile user "API", >which could probably be a problem for applications using the >dpdk makefile framework. > >Why not just having an temporary internal variable (let's say >_LDLIBS-y) that would allow to do the clean-up without modifying >the user interface? > >Also, with your patch, the approach for EXTRA_LDLIBS would be >different than CFLAGS or LDFLAGS: >- CFLAGS/LDFLAGS are in Makefiles only >- EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line > to add flags to the default ones > >I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, >keeping a compatibility with existing application Makefiles. The docs for DPDK 28.3.6 states they can be used for both command line and Makefile, so I think I like the current solution unless everyone wants it as you suggested. http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.html > >What do you think? > >Regards, >Olivier > > > >> >> Signed-off-by: Keith Wiles <keith.wiles@intel.com> >> --- >> examples/dpdk_qat/Makefile | 4 +- >> examples/vm_power_manager/Makefile | 2 +- >> mk/rte.app.mk | 254 >>++++++++++--------------------------- >> mk/rte.hostapp.mk | 4 +- >> mk/rte.shared.mk | 12 +- >> 5 files changed, 77 insertions(+), 199 deletions(-) >> >> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >> index f1e06a1..90ca1d3 100644 >> --- a/examples/dpdk_qat/Makefile >> +++ b/examples/dpdk_qat/Makefile >> @@ -77,8 +77,8 @@ else >> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >> endif >> >> -LDLIBS += -L$(ICP_ROOT)/build >> -LDLIBS += $(ICP_LIBRARY_PATH) \ >> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >> -lz \ >> -losal \ >> -ladf_proxy \ >> diff --git a/examples/vm_power_manager/Makefile >>b/examples/vm_power_manager/Makefile >> index 113dbc4..8fb78d4 100644 >> --- a/examples/vm_power_manager/Makefile >> +++ b/examples/vm_power_manager/Makefile >> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >> CFLAGS += $(WERROR_FLAGS) >> >> -LDLIBS += -lvirt >> +EXTRA_LDLIBS += -lvirt >> >> # workaround for a gcc bug with noreturn attribute >> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >> index 62a76ae..c41de82 100644 >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -1,7 +1,7 @@ >> # BSD LICENSE >> # >> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >> -# Copyright(c) 2014 6WIND S.A. >> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >> +# Copyright(c) 2015 6WIND S.A. >> # All rights reserved. >> # >> # Redistribution and use in source and binary forms, with or without >> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >> endif >> >> # default path for libs >> -LDLIBS += -L$(RTE_SDK_BIN)/lib >> +LDLIBS-y += -L$(RTE_SDK_BIN)/lib >> >> # >> # Include libraries depending on config if NO_AUTOLIBS is not set >> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >> # >> ifeq ($(NO_AUTOLIBS),) >> >> -LDLIBS += --whole-archive >> +LDLIBS-y += --whole-archive >> >> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >> -LDLIBS += -l$(RTE_LIBNAME) >> -endif >> +LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS) += -l$(RTE_LIBNAME) >> >> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >> >> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >> -LDLIBS += -lrte_distributor >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >> -LDLIBS += -lrte_reorder >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor >> +LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder >> >> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >> -LDLIBS += -lrte_kni >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) += -lrte_kni >> +LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += -lrte_ivshmem >> endif >> >> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >> -LDLIBS += -lrte_ivshmem >> -endif >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >> +LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port >> +LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >> +LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >> +LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >> +LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm >> +LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >> +LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >> +LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >> >> -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) >> -LDLIBS += -lrte_pipeline >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched >> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm >> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt >> >> -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) >> -LDLIBS += -lrte_table >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) >> -LDLIBS += -lrte_port >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y) >> -LDLIBS += -lrte_timer >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y) >> -LDLIBS += -lrte_hash >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y) >> -LDLIBS += -lrte_jobstats >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) >> -LDLIBS += -lrte_lpm >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y) >> -LDLIBS += -lrte_power >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y) >> -LDLIBS += -lrte_acl >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_METER),y) >> -LDLIBS += -lrte_meter >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) >> -LDLIBS += -lrte_sched >> -LDLIBS += -lm >> -LDLIBS += -lrt >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y) >> -LDLIBS += -lrte_vhost >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost >> >> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >> >> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >> -LDLIBS += -lpcap >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lpcap >> >> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn) >> -LDLIBS += -lfuse >> +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n) >> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse >> endif >> >> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >> -LDLIBS += -libverbs >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -libverbs >> >> -LDLIBS += --start-group >> +LDLIBS-y += --start-group >> >> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >> >> -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y) >> -LDLIBS += -lrte_kvargs >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y) >> -LDLIBS += -lrte_mbuf >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y) >> -LDLIBS += -lrte_ip_frag >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y) >> -LDLIBS += -lethdev >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) >> -LDLIBS += -lrte_malloc >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y) >> -LDLIBS += -lrte_mempool >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_RING),y) >> -LDLIBS += -lrte_ring >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_EAL),y) >> -LDLIBS += -lrte_eal >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y) >> -LDLIBS += -lrte_cmdline >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y) >> -LDLIBS += -lrte_cfgfile >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y) >> -LDLIBS += -lrte_pmd_bond >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y) >> -LDLIBS += -lrte_pmd_xenvirt >> -LDLIBS += -lxenstore >> -endif >> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs >> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf >> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER) += -lethdev >> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC) += -lrte_malloc >> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += -lrte_mempool >> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring >> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal >> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline >> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond >> + >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lxenstore >> >> ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) >> # plugins (link only if static libraries) >> >> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) >> -LDLIBS += -lrte_pmd_vmxnet3_uio >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) >> -LDLIBS += -lrte_pmd_virtio >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y) >> -LDLIBS += -lrte_pmd_enic >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) >> -LDLIBS += -lrte_pmd_i40e >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y) >> -LDLIBS += -lrte_pmd_fm10k >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) >> -LDLIBS += -lrte_pmd_ixgbe >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y) >> -LDLIBS += -lrte_pmd_e1000 >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >> -LDLIBS += -lrte_pmd_mlx4 >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) >> -LDLIBS += -lrte_pmd_ring >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >> -LDLIBS += -lrte_pmd_pcap >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y) >> -LDLIBS += -lrte_pmd_af_packet >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y) >> -LDLIBS += -lrte_pmd_null >> -endif >> - >> -endif # plugins >> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio >> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio >> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic >> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e >> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k >> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe >> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 >> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet >> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null >> + >> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) >> >> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >> >> -LDLIBS += $(EXECENV_LDLIBS) >> - >> -LDLIBS += --end-group >> - >> -LDLIBS += --no-whole-archive >> +LDLIBS-y += $(EXECENV_LDLIBS) >> +LDLIBS-y += --end-group >> +LDLIBS-y += --no-whole-archive >> >> endif # ifeq ($(NO_AUTOLIBS),) >> >> -LDLIBS += $(CPU_LDLIBS) >> +LDLIBS-y += $(EXTRA_LDLIBS) >> >> .PHONY: all >> all: install >> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1) >> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >> O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \ >> -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call >>linkerprefix,$(LDFLAGS)) \ >> - $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS)) >> + $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y)) >> else >> O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >> - -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS) >> + -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y) >> endif >> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," LD $(@)") >> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \ >> -include .$(APP).cmd >> >> # path where libraries are retrieved >> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>-Wl$(comma)-L%,$(LDLIBS))) >> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>-Wl$(comma)-L%,$(LDLIBS-y))) >> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >> >> # list of .a files that are linked to this application >> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>-Wl$(comma)-l%,$(LDLIBS))) >> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>-Wl$(comma)-l%,$(LDLIBS-y))) >> >> # list of found libraries files (useful for deps). If not found, the >> # library is silently ignored and dep won't be checked >> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk >> index c44d0f8..51e8c1d 100644 >> --- a/mk/rte.hostapp.mk >> +++ b/mk/rte.hostapp.mk >> @@ -58,7 +58,7 @@ build: _postbuild >> exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) >> >> O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \ >> - $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS) >> + $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y) >> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@)") >> O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)" >> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \ >> # list of .a files that are linked to this application >> LDLIBS_FILES := $(wildcard \ >> $(addprefix $(RTE_OUTPUT)/lib/, \ >> - $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))))) >> + $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))))) >> >> # >> # Compile executable file if needed >> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk >> index fc6b0b4..56f20bb 100644 >> --- a/mk/rte.shared.mk >> +++ b/mk/rte.shared.mk >> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst >>%,%.cmd,$(1)))) >> ifeq ($(LINK_USING_CC),1) >> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >> O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) >>$(EXTRA_LDFLAGS) \ >> - -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS)) >> + -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y)) >> else >> O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >> - -shared -o $@ $(OBJS-y) $(LDLIBS) >> + -shared -o $@ $(OBJS-y) $(LDLIBS-y) >> endif >> >> O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight >> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \ >> -include .$(SHARED).cmd >> >> # path where libraries are retrieved >> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>-Wl$(comma)-L%,$(LDLIBS))) >> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>-Wl$(comma)-L%,$(LDLIBS-y))) >> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >> >> # list of .a files that are linked to this application >> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>-Wl$(comma)-l%,$(LDLIBS))) >> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>-Wl$(comma)-l%,$(LDLIBS-y))) >> >> # list of found libraries files (useful for deps). If not found, the >> # library is silently ignored and dep won't be checked >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-30 13:24 ` Wiles, Keith @ 2015-04-30 13:38 ` Olivier MATZ 2015-04-30 14:31 ` Wiles, Keith 0 siblings, 1 reply; 12+ messages in thread From: Olivier MATZ @ 2015-04-30 13:38 UTC (permalink / raw) To: Wiles, Keith, dev Hi Keith, On 04/30/2015 03:24 PM, Wiles, Keith wrote: > > > On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: > >> Hi Keith, >> >> Thank you for submitting a clean-up. Please see some comments below. >> >> On 04/29/2015 05:25 PM, Keith Wiles wrote: >>> Trying to simplify the ifdefs in rte.app.mk to make the code >>> more readable and maintainable by moving LDLIBS variable to use >>> the same style as LDLIBS-y being used in the rest of the code. >>> >>> Added a new variable called EXTRA_LDLIBS to be used by example apps >>> instead of using LDLIBS directly. >> >> If I understand well, the goal of this patch is only a cleanup in >> rte.app.mk, but at the end, it changes the makefile user "API", >> which could probably be a problem for applications using the >> dpdk makefile framework. >> >> Why not just having an temporary internal variable (let's say >> _LDLIBS-y) that would allow to do the clean-up without modifying >> the user interface? >> >> Also, with your patch, the approach for EXTRA_LDLIBS would be >> different than CFLAGS or LDFLAGS: >> - CFLAGS/LDFLAGS are in Makefiles only >> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line >> to add flags to the default ones >> >> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, >> keeping a compatibility with existing application Makefiles. > > The docs for DPDK 28.3.6 states they can be used for both command line and > Makefile, so I think I like the current solution unless everyone wants it > as you suggested. > > http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.html From the link you have sent: - About CFLAGS: "28.3.4. Variables that Can be Set/Overridden in a Makefile Only [...] CFLAGS: Flags to use for C compilation. The user should use += to append data in this variable." nothing in 28.3.6 - About EXTRA_CFLAGS: nothing in 28.3.4 "28.3.6. Variables that Can be Set/Overridden by the User in a Makefile or Command Line [...] EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when compiling." It seems it's the description I have made above. Also, you don't answer to the Makefile API issue. From what I understand, your patch would break external Makefiles using LDLIBS just for doing an internal clean-up. Regards, Olivier > > >> >> What do you think? >> >> Regards, >> Olivier >> >> >> >>> >>> Signed-off-by: Keith Wiles <keith.wiles@intel.com> >>> --- >>> examples/dpdk_qat/Makefile | 4 +- >>> examples/vm_power_manager/Makefile | 2 +- >>> mk/rte.app.mk | 254 >>> ++++++++++--------------------------- >>> mk/rte.hostapp.mk | 4 +- >>> mk/rte.shared.mk | 12 +- >>> 5 files changed, 77 insertions(+), 199 deletions(-) >>> >>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >>> index f1e06a1..90ca1d3 100644 >>> --- a/examples/dpdk_qat/Makefile >>> +++ b/examples/dpdk_qat/Makefile >>> @@ -77,8 +77,8 @@ else >>> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >>> endif >>> >>> -LDLIBS += -L$(ICP_ROOT)/build >>> -LDLIBS += $(ICP_LIBRARY_PATH) \ >>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >>> -lz \ >>> -losal \ >>> -ladf_proxy \ >>> diff --git a/examples/vm_power_manager/Makefile >>> b/examples/vm_power_manager/Makefile >>> index 113dbc4..8fb78d4 100644 >>> --- a/examples/vm_power_manager/Makefile >>> +++ b/examples/vm_power_manager/Makefile >>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >>> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >>> CFLAGS += $(WERROR_FLAGS) >>> >>> -LDLIBS += -lvirt >>> +EXTRA_LDLIBS += -lvirt >>> >>> # workaround for a gcc bug with noreturn attribute >>> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >>> index 62a76ae..c41de82 100644 >>> --- a/mk/rte.app.mk >>> +++ b/mk/rte.app.mk >>> @@ -1,7 +1,7 @@ >>> # BSD LICENSE >>> # >>> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >>> -# Copyright(c) 2014 6WIND S.A. >>> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >>> +# Copyright(c) 2015 6WIND S.A. >>> # All rights reserved. >>> # >>> # Redistribution and use in source and binary forms, with or without >>> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >>> endif >>> >>> # default path for libs >>> -LDLIBS += -L$(RTE_SDK_BIN)/lib >>> +LDLIBS-y += -L$(RTE_SDK_BIN)/lib >>> >>> # >>> # Include libraries depending on config if NO_AUTOLIBS is not set >>> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >>> # >>> ifeq ($(NO_AUTOLIBS),) >>> >>> -LDLIBS += --whole-archive >>> +LDLIBS-y += --whole-archive >>> >>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >>> -LDLIBS += -l$(RTE_LIBNAME) >>> -endif >>> +LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS) += -l$(RTE_LIBNAME) >>> >>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >>> -LDLIBS += -lrte_distributor >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >>> -LDLIBS += -lrte_reorder >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >>> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>> -LDLIBS += -lrte_kni >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) += -lrte_kni >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += -lrte_ivshmem >>> endif >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >>> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>> -LDLIBS += -lrte_ivshmem >>> -endif >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) >>> -LDLIBS += -lrte_pipeline >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) >>> -LDLIBS += -lrte_table >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) >>> -LDLIBS += -lrte_port >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y) >>> -LDLIBS += -lrte_timer >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y) >>> -LDLIBS += -lrte_hash >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y) >>> -LDLIBS += -lrte_jobstats >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) >>> -LDLIBS += -lrte_lpm >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y) >>> -LDLIBS += -lrte_power >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y) >>> -LDLIBS += -lrte_acl >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_METER),y) >>> -LDLIBS += -lrte_meter >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) >>> -LDLIBS += -lrte_sched >>> -LDLIBS += -lm >>> -LDLIBS += -lrt >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y) >>> -LDLIBS += -lrte_vhost >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost >>> >>> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >>> -LDLIBS += -lpcap >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lpcap >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn) >>> -LDLIBS += -lfuse >>> +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n) >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse >>> endif >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >>> -LDLIBS += -libverbs >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -libverbs >>> >>> -LDLIBS += --start-group >>> +LDLIBS-y += --start-group >>> >>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y) >>> -LDLIBS += -lrte_kvargs >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y) >>> -LDLIBS += -lrte_mbuf >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y) >>> -LDLIBS += -lrte_ip_frag >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y) >>> -LDLIBS += -lethdev >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) >>> -LDLIBS += -lrte_malloc >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y) >>> -LDLIBS += -lrte_mempool >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_RING),y) >>> -LDLIBS += -lrte_ring >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_EAL),y) >>> -LDLIBS += -lrte_eal >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y) >>> -LDLIBS += -lrte_cmdline >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y) >>> -LDLIBS += -lrte_cfgfile >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y) >>> -LDLIBS += -lrte_pmd_bond >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y) >>> -LDLIBS += -lrte_pmd_xenvirt >>> -LDLIBS += -lxenstore >>> -endif >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER) += -lethdev >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC) += -lrte_malloc >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += -lrte_mempool >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond >>> + >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lxenstore >>> >>> ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) >>> # plugins (link only if static libraries) >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) >>> -LDLIBS += -lrte_pmd_vmxnet3_uio >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) >>> -LDLIBS += -lrte_pmd_virtio >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y) >>> -LDLIBS += -lrte_pmd_enic >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) >>> -LDLIBS += -lrte_pmd_i40e >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y) >>> -LDLIBS += -lrte_pmd_fm10k >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) >>> -LDLIBS += -lrte_pmd_ixgbe >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y) >>> -LDLIBS += -lrte_pmd_e1000 >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >>> -LDLIBS += -lrte_pmd_mlx4 >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) >>> -LDLIBS += -lrte_pmd_ring >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >>> -LDLIBS += -lrte_pmd_pcap >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y) >>> -LDLIBS += -lrte_pmd_af_packet >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y) >>> -LDLIBS += -lrte_pmd_null >>> -endif >>> - >>> -endif # plugins >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet >>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null >>> + >>> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) >>> >>> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >>> >>> -LDLIBS += $(EXECENV_LDLIBS) >>> - >>> -LDLIBS += --end-group >>> - >>> -LDLIBS += --no-whole-archive >>> +LDLIBS-y += $(EXECENV_LDLIBS) >>> +LDLIBS-y += --end-group >>> +LDLIBS-y += --no-whole-archive >>> >>> endif # ifeq ($(NO_AUTOLIBS),) >>> >>> -LDLIBS += $(CPU_LDLIBS) >>> +LDLIBS-y += $(EXTRA_LDLIBS) >>> >>> .PHONY: all >>> all: install >>> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1) >>> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >>> O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \ >>> -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call >>> linkerprefix,$(LDFLAGS)) \ >>> - $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS)) >>> + $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y)) >>> else >>> O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >>> - -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS) >>> + -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y) >>> endif >>> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >>> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," LD $(@)") >>> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \ >>> -include .$(APP).cmd >>> >>> # path where libraries are retrieved >>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>> -Wl$(comma)-L%,$(LDLIBS))) >>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>> -Wl$(comma)-L%,$(LDLIBS-y))) >>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >>> >>> # list of .a files that are linked to this application >>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>> -Wl$(comma)-l%,$(LDLIBS))) >>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>> -Wl$(comma)-l%,$(LDLIBS-y))) >>> >>> # list of found libraries files (useful for deps). If not found, the >>> # library is silently ignored and dep won't be checked >>> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk >>> index c44d0f8..51e8c1d 100644 >>> --- a/mk/rte.hostapp.mk >>> +++ b/mk/rte.hostapp.mk >>> @@ -58,7 +58,7 @@ build: _postbuild >>> exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) >>> >>> O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \ >>> - $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS) >>> + $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y) >>> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >>> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@)") >>> O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)" >>> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \ >>> # list of .a files that are linked to this application >>> LDLIBS_FILES := $(wildcard \ >>> $(addprefix $(RTE_OUTPUT)/lib/, \ >>> - $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))))) >>> + $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))))) >>> >>> # >>> # Compile executable file if needed >>> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk >>> index fc6b0b4..56f20bb 100644 >>> --- a/mk/rte.shared.mk >>> +++ b/mk/rte.shared.mk >>> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst >>> %,%.cmd,$(1)))) >>> ifeq ($(LINK_USING_CC),1) >>> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >>> O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) >>> $(EXTRA_LDFLAGS) \ >>> - -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS)) >>> + -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y)) >>> else >>> O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >>> - -shared -o $@ $(OBJS-y) $(LDLIBS) >>> + -shared -o $@ $(OBJS-y) $(LDLIBS-y) >>> endif >>> >>> O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight >>> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \ >>> -include .$(SHARED).cmd >>> >>> # path where libraries are retrieved >>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>> -Wl$(comma)-L%,$(LDLIBS))) >>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>> -Wl$(comma)-L%,$(LDLIBS-y))) >>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >>> >>> # list of .a files that are linked to this application >>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>> -Wl$(comma)-l%,$(LDLIBS))) >>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>> -Wl$(comma)-l%,$(LDLIBS-y))) >>> >>> # list of found libraries files (useful for deps). If not found, the >>> # library is silently ignored and dep won't be checked >>> >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-30 13:38 ` Olivier MATZ @ 2015-04-30 14:31 ` Wiles, Keith 2015-04-30 15:56 ` Olivier MATZ 2015-04-30 16:22 ` Bruce Richardson 0 siblings, 2 replies; 12+ messages in thread From: Wiles, Keith @ 2015-04-30 14:31 UTC (permalink / raw) To: Olivier MATZ, dev On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: >Hi Keith, > >On 04/30/2015 03:24 PM, Wiles, Keith wrote: >> >> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: >> >>> Hi Keith, >>> >>> Thank you for submitting a clean-up. Please see some comments below. >>> >>> On 04/29/2015 05:25 PM, Keith Wiles wrote: >>>> Trying to simplify the ifdefs in rte.app.mk to make the code >>>> more readable and maintainable by moving LDLIBS variable to use >>>> the same style as LDLIBS-y being used in the rest of the code. >>>> >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps >>>> instead of using LDLIBS directly. >>> >>> If I understand well, the goal of this patch is only a cleanup in >>> rte.app.mk, but at the end, it changes the makefile user "API", >>> which could probably be a problem for applications using the >>> dpdk makefile framework. >>> >>> Why not just having an temporary internal variable (let's say >>> _LDLIBS-y) that would allow to do the clean-up without modifying >>> the user interface? >>> >>> Also, with your patch, the approach for EXTRA_LDLIBS would be >>> different than CFLAGS or LDFLAGS: >>> - CFLAGS/LDFLAGS are in Makefiles only >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line >>> to add flags to the default ones >>> >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, >>> keeping a compatibility with existing application Makefiles. >> >> The docs for DPDK 28.3.6 states they can be used for both command line >>and >> Makefile, so I think I like the current solution unless everyone wants >>it >> as you suggested. >> >> >>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.htm >>l > > From the link you have sent: > >- About CFLAGS: > >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only >[...] >CFLAGS: Flags to use for C compilation. The user should use += to append >data in this variable." > >nothing in 28.3.6 > > >- About EXTRA_CFLAGS: > >nothing in 28.3.4 > >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile >or Command Line >[...] >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when >compiling." The point was that EXTRA_XXX can be used for command line and Makefile as it was pointed out in a previous email the assumption was EXTRA_XXX was only for the command line. (Just to make sure we understood EXTRA_XXX was not just for command line options.) This was the reason I sent the link an to point out using EXTRA_XXX is a much cleaner method then allowing someone to modify what I believe is an internal variable. > > >It seems it's the description I have made above. > >Also, you don't answer to the Makefile API issue. From what I >understand, your patch would break external Makefiles using >LDLIBS just for doing an internal clean-up. I believe using these variables CFLAGS, LDLIBS, Š Are really internal variables and EXTRA_XXX should have been used instead. I would like to fix that issue and I do not feel the impact is that big as it is a simple search replace problem. But it looks like we want to keep the old way just for a very minor change IMHO. I will send out a new patch using _LDLIBS-y as the internal variable and then set LDLIBS at the correct location towards the end of the file. Does that work for you? Regards, ++Keith > > >Regards, >Olivier > > >> >> >>> >>> What do you think? >>> >>> Regards, >>> Olivier >>> >>> >>> >>>> >>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com> >>>> --- >>>> examples/dpdk_qat/Makefile | 4 +- >>>> examples/vm_power_manager/Makefile | 2 +- >>>> mk/rte.app.mk | 254 >>>> ++++++++++--------------------------- >>>> mk/rte.hostapp.mk | 4 +- >>>> mk/rte.shared.mk | 12 +- >>>> 5 files changed, 77 insertions(+), 199 deletions(-) >>>> >>>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >>>> index f1e06a1..90ca1d3 100644 >>>> --- a/examples/dpdk_qat/Makefile >>>> +++ b/examples/dpdk_qat/Makefile >>>> @@ -77,8 +77,8 @@ else >>>> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >>>> endif >>>> >>>> -LDLIBS += -L$(ICP_ROOT)/build >>>> -LDLIBS += $(ICP_LIBRARY_PATH) \ >>>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >>>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >>>> -lz \ >>>> -losal \ >>>> -ladf_proxy \ >>>> diff --git a/examples/vm_power_manager/Makefile >>>> b/examples/vm_power_manager/Makefile >>>> index 113dbc4..8fb78d4 100644 >>>> --- a/examples/vm_power_manager/Makefile >>>> +++ b/examples/vm_power_manager/Makefile >>>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >>>> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >>>> CFLAGS += $(WERROR_FLAGS) >>>> >>>> -LDLIBS += -lvirt >>>> +EXTRA_LDLIBS += -lvirt >>>> >>>> # workaround for a gcc bug with noreturn attribute >>>> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >>>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >>>> index 62a76ae..c41de82 100644 >>>> --- a/mk/rte.app.mk >>>> +++ b/mk/rte.app.mk >>>> @@ -1,7 +1,7 @@ >>>> # BSD LICENSE >>>> # >>>> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >>>> -# Copyright(c) 2014 6WIND S.A. >>>> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >>>> +# Copyright(c) 2015 6WIND S.A. >>>> # All rights reserved. >>>> # >>>> # Redistribution and use in source and binary forms, with or >>>>without >>>> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >>>> endif >>>> >>>> # default path for libs >>>> -LDLIBS += -L$(RTE_SDK_BIN)/lib >>>> +LDLIBS-y += -L$(RTE_SDK_BIN)/lib >>>> >>>> # >>>> # Include libraries depending on config if NO_AUTOLIBS is not set >>>> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >>>> # >>>> ifeq ($(NO_AUTOLIBS),) >>>> >>>> -LDLIBS += --whole-archive >>>> +LDLIBS-y += --whole-archive >>>> >>>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >>>> -LDLIBS += -l$(RTE_LIBNAME) >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS) += -l$(RTE_LIBNAME) >>>> >>>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >>>> -LDLIBS += -lrte_distributor >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >>>> -LDLIBS += -lrte_reorder >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >>>> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>>> -LDLIBS += -lrte_kni >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) += -lrte_kni >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += -lrte_ivshmem >>>> endif >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >>>> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>>> -LDLIBS += -lrte_ivshmem >>>> -endif >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) >>>> -LDLIBS += -lrte_pipeline >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) >>>> -LDLIBS += -lrte_table >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) >>>> -LDLIBS += -lrte_port >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y) >>>> -LDLIBS += -lrte_timer >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y) >>>> -LDLIBS += -lrte_hash >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y) >>>> -LDLIBS += -lrte_jobstats >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) >>>> -LDLIBS += -lrte_lpm >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y) >>>> -LDLIBS += -lrte_power >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y) >>>> -LDLIBS += -lrte_acl >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_METER),y) >>>> -LDLIBS += -lrte_meter >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) >>>> -LDLIBS += -lrte_sched >>>> -LDLIBS += -lm >>>> -LDLIBS += -lrt >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y) >>>> -LDLIBS += -lrte_vhost >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost >>>> >>>> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >>>> -LDLIBS += -lpcap >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lpcap >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn) >>>> -LDLIBS += -lfuse >>>> +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n) >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse >>>> endif >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >>>> -LDLIBS += -libverbs >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -libverbs >>>> >>>> -LDLIBS += --start-group >>>> +LDLIBS-y += --start-group >>>> >>>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y) >>>> -LDLIBS += -lrte_kvargs >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y) >>>> -LDLIBS += -lrte_mbuf >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y) >>>> -LDLIBS += -lrte_ip_frag >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y) >>>> -LDLIBS += -lethdev >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) >>>> -LDLIBS += -lrte_malloc >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y) >>>> -LDLIBS += -lrte_mempool >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_RING),y) >>>> -LDLIBS += -lrte_ring >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_EAL),y) >>>> -LDLIBS += -lrte_eal >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y) >>>> -LDLIBS += -lrte_cmdline >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y) >>>> -LDLIBS += -lrte_cfgfile >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y) >>>> -LDLIBS += -lrte_pmd_bond >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y) >>>> -LDLIBS += -lrte_pmd_xenvirt >>>> -LDLIBS += -lxenstore >>>> -endif >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER) += -lethdev >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC) += -lrte_malloc >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += -lrte_mempool >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond >>>> + >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lxenstore >>>> >>>> ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) >>>> # plugins (link only if static libraries) >>>> >>>> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) >>>> -LDLIBS += -lrte_pmd_vmxnet3_uio >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) >>>> -LDLIBS += -lrte_pmd_virtio >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y) >>>> -LDLIBS += -lrte_pmd_enic >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) >>>> -LDLIBS += -lrte_pmd_i40e >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y) >>>> -LDLIBS += -lrte_pmd_fm10k >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) >>>> -LDLIBS += -lrte_pmd_ixgbe >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y) >>>> -LDLIBS += -lrte_pmd_e1000 >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >>>> -LDLIBS += -lrte_pmd_mlx4 >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) >>>> -LDLIBS += -lrte_pmd_ring >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >>>> -LDLIBS += -lrte_pmd_pcap >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y) >>>> -LDLIBS += -lrte_pmd_af_packet >>>> -endif >>>> - >>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y) >>>> -LDLIBS += -lrte_pmd_null >>>> -endif >>>> - >>>> -endif # plugins >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet >>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null >>>> + >>>> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) >>>> >>>> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >>>> >>>> -LDLIBS += $(EXECENV_LDLIBS) >>>> - >>>> -LDLIBS += --end-group >>>> - >>>> -LDLIBS += --no-whole-archive >>>> +LDLIBS-y += $(EXECENV_LDLIBS) >>>> +LDLIBS-y += --end-group >>>> +LDLIBS-y += --no-whole-archive >>>> >>>> endif # ifeq ($(NO_AUTOLIBS),) >>>> >>>> -LDLIBS += $(CPU_LDLIBS) >>>> +LDLIBS-y += $(EXTRA_LDLIBS) >>>> >>>> .PHONY: all >>>> all: install >>>> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1) >>>> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >>>> O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \ >>>> -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call >>>> linkerprefix,$(LDFLAGS)) \ >>>> - $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS)) >>>> + $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y)) >>>> else >>>> O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >>>> - -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS) >>>> + -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y) >>>> endif >>>> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >>>> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," LD $(@)") >>>> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \ >>>> -include .$(APP).cmd >>>> >>>> # path where libraries are retrieved >>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>> -Wl$(comma)-L%,$(LDLIBS))) >>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>> -Wl$(comma)-L%,$(LDLIBS-y))) >>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >>>> >>>> # list of .a files that are linked to this application >>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>> -Wl$(comma)-l%,$(LDLIBS))) >>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>> -Wl$(comma)-l%,$(LDLIBS-y))) >>>> >>>> # list of found libraries files (useful for deps). If not found, >>>>the >>>> # library is silently ignored and dep won't be checked >>>> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk >>>> index c44d0f8..51e8c1d 100644 >>>> --- a/mk/rte.hostapp.mk >>>> +++ b/mk/rte.hostapp.mk >>>> @@ -58,7 +58,7 @@ build: _postbuild >>>> exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) >>>> >>>> O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \ >>>> - $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS) >>>> + $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y) >>>> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >>>> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@)") >>>> O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)" >>>> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \ >>>> # list of .a files that are linked to this application >>>> LDLIBS_FILES := $(wildcard \ >>>> $(addprefix $(RTE_OUTPUT)/lib/, \ >>>> - $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))))) >>>> + $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))))) >>>> >>>> # >>>> # Compile executable file if needed >>>> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk >>>> index fc6b0b4..56f20bb 100644 >>>> --- a/mk/rte.shared.mk >>>> +++ b/mk/rte.shared.mk >>>> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst >>>> %,%.cmd,$(1)))) >>>> ifeq ($(LINK_USING_CC),1) >>>> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >>>> O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) >>>> $(EXTRA_LDFLAGS) \ >>>> - -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS)) >>>> + -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y)) >>>> else >>>> O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >>>> - -shared -o $@ $(OBJS-y) $(LDLIBS) >>>> + -shared -o $@ $(OBJS-y) $(LDLIBS-y) >>>> endif >>>> >>>> O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight >>>> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \ >>>> -include .$(SHARED).cmd >>>> >>>> # path where libraries are retrieved >>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>> -Wl$(comma)-L%,$(LDLIBS))) >>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>> -Wl$(comma)-L%,$(LDLIBS-y))) >>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >>>> >>>> # list of .a files that are linked to this application >>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>> -Wl$(comma)-l%,$(LDLIBS))) >>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>> -Wl$(comma)-l%,$(LDLIBS-y))) >>>> >>>> # list of found libraries files (useful for deps). If not found, >>>>the >>>> # library is silently ignored and dep won't be checked >>>> >>> >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-30 14:31 ` Wiles, Keith @ 2015-04-30 15:56 ` Olivier MATZ 2015-04-30 16:22 ` Bruce Richardson 1 sibling, 0 replies; 12+ messages in thread From: Olivier MATZ @ 2015-04-30 15:56 UTC (permalink / raw) To: Wiles, Keith, dev Hi, On 04/30/2015 04:31 PM, Wiles, Keith wrote: > > > On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: > >> Hi Keith, >> >> On 04/30/2015 03:24 PM, Wiles, Keith wrote: >>> >>> >>> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: >>> >>>> Hi Keith, >>>> >>>> Thank you for submitting a clean-up. Please see some comments below. >>>> >>>> On 04/29/2015 05:25 PM, Keith Wiles wrote: >>>>> Trying to simplify the ifdefs in rte.app.mk to make the code >>>>> more readable and maintainable by moving LDLIBS variable to use >>>>> the same style as LDLIBS-y being used in the rest of the code. >>>>> >>>>> Added a new variable called EXTRA_LDLIBS to be used by example apps >>>>> instead of using LDLIBS directly. >>>> >>>> If I understand well, the goal of this patch is only a cleanup in >>>> rte.app.mk, but at the end, it changes the makefile user "API", >>>> which could probably be a problem for applications using the >>>> dpdk makefile framework. >>>> >>>> Why not just having an temporary internal variable (let's say >>>> _LDLIBS-y) that would allow to do the clean-up without modifying >>>> the user interface? >>>> >>>> Also, with your patch, the approach for EXTRA_LDLIBS would be >>>> different than CFLAGS or LDFLAGS: >>>> - CFLAGS/LDFLAGS are in Makefiles only >>>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line >>>> to add flags to the default ones >>>> >>>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, >>>> keeping a compatibility with existing application Makefiles. >>> >>> The docs for DPDK 28.3.6 states they can be used for both command line >>> and >>> Makefile, so I think I like the current solution unless everyone wants >>> it >>> as you suggested. >>> >>> >>> http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.htm >>> l >> >> From the link you have sent: >> >> - About CFLAGS: >> >> "28.3.4. Variables that Can be Set/Overridden in a Makefile Only >> [...] >> CFLAGS: Flags to use for C compilation. The user should use += to append >> data in this variable." >> >> nothing in 28.3.6 >> >> >> - About EXTRA_CFLAGS: >> >> nothing in 28.3.4 >> >> "28.3.6. Variables that Can be Set/Overridden by the User in a Makefile >> or Command Line >> [...] >> EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when >> compiling." > > The point was that EXTRA_XXX can be used for command line and Makefile as > it was pointed out in a previous email the assumption was EXTRA_XXX was > only for the command line. (Just to make sure we understood EXTRA_XXX was > not just for command line options.) This was the reason I sent the link an > to point out using EXTRA_XXX is a much cleaner method then allowing > someone to modify what I believe is an internal variable. Although it is allowed, using EXTRA_xxx in a Makefile does not look to be a good idea in my opinion. For instance, EXTRA_CFLAGS can be used to append something to the CFLAGS variable. If we also use EXTRA_CFLAGS in the Makefile, specifying it on the command line would override its value. So, CFLAGS and LDLIBS are not internal variables. The DPDK build framework is designed to make use of these variables provided by the application Makefile. >> It seems it's the description I have made above. >> >> Also, you don't answer to the Makefile API issue. From what I >> understand, your patch would break external Makefiles using >> LDLIBS just for doing an internal clean-up. > > I believe using these variables CFLAGS, LDLIBS, Š Are really internal > variables and EXTRA_XXX should have been used instead. I would like to fix > that issue and I do not feel the impact is that big as it is a simple > search replace problem. But it looks like we want to keep the old way just > for a very minor change IMHO. > > I will send out a new patch using _LDLIBS-y as the internal variable and > then set LDLIBS at the correct location towards the end of the file. > > Does that work for you? Yes, I think it's better. Not breaking the API for external Makefile is important fir users. Regards, Olivier > > Regards, > ++Keith >> >> >> Regards, >> Olivier >> >> >>> >>> >>>> >>>> What do you think? >>>> >>>> Regards, >>>> Olivier >>>> >>>> >>>> >>>>> >>>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com> >>>>> --- >>>>> examples/dpdk_qat/Makefile | 4 +- >>>>> examples/vm_power_manager/Makefile | 2 +- >>>>> mk/rte.app.mk | 254 >>>>> ++++++++++--------------------------- >>>>> mk/rte.hostapp.mk | 4 +- >>>>> mk/rte.shared.mk | 12 +- >>>>> 5 files changed, 77 insertions(+), 199 deletions(-) >>>>> >>>>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >>>>> index f1e06a1..90ca1d3 100644 >>>>> --- a/examples/dpdk_qat/Makefile >>>>> +++ b/examples/dpdk_qat/Makefile >>>>> @@ -77,8 +77,8 @@ else >>>>> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >>>>> endif >>>>> >>>>> -LDLIBS += -L$(ICP_ROOT)/build >>>>> -LDLIBS += $(ICP_LIBRARY_PATH) \ >>>>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >>>>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >>>>> -lz \ >>>>> -losal \ >>>>> -ladf_proxy \ >>>>> diff --git a/examples/vm_power_manager/Makefile >>>>> b/examples/vm_power_manager/Makefile >>>>> index 113dbc4..8fb78d4 100644 >>>>> --- a/examples/vm_power_manager/Makefile >>>>> +++ b/examples/vm_power_manager/Makefile >>>>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >>>>> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >>>>> CFLAGS += $(WERROR_FLAGS) >>>>> >>>>> -LDLIBS += -lvirt >>>>> +EXTRA_LDLIBS += -lvirt >>>>> >>>>> # workaround for a gcc bug with noreturn attribute >>>>> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >>>>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >>>>> index 62a76ae..c41de82 100644 >>>>> --- a/mk/rte.app.mk >>>>> +++ b/mk/rte.app.mk >>>>> @@ -1,7 +1,7 @@ >>>>> # BSD LICENSE >>>>> # >>>>> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >>>>> -# Copyright(c) 2014 6WIND S.A. >>>>> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >>>>> +# Copyright(c) 2015 6WIND S.A. >>>>> # All rights reserved. >>>>> # >>>>> # Redistribution and use in source and binary forms, with or >>>>> without >>>>> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >>>>> endif >>>>> >>>>> # default path for libs >>>>> -LDLIBS += -L$(RTE_SDK_BIN)/lib >>>>> +LDLIBS-y += -L$(RTE_SDK_BIN)/lib >>>>> >>>>> # >>>>> # Include libraries depending on config if NO_AUTOLIBS is not set >>>>> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >>>>> # >>>>> ifeq ($(NO_AUTOLIBS),) >>>>> >>>>> -LDLIBS += --whole-archive >>>>> +LDLIBS-y += --whole-archive >>>>> >>>>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >>>>> -LDLIBS += -l$(RTE_LIBNAME) >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS) += -l$(RTE_LIBNAME) >>>>> >>>>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >>>>> -LDLIBS += -lrte_distributor >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >>>>> -LDLIBS += -lrte_reorder >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >>>>> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>>>> -LDLIBS += -lrte_kni >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) += -lrte_kni >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += -lrte_ivshmem >>>>> endif >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >>>>> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>>>> -LDLIBS += -lrte_ivshmem >>>>> -endif >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) >>>>> -LDLIBS += -lrte_pipeline >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) >>>>> -LDLIBS += -lrte_table >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) >>>>> -LDLIBS += -lrte_port >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y) >>>>> -LDLIBS += -lrte_timer >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y) >>>>> -LDLIBS += -lrte_hash >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y) >>>>> -LDLIBS += -lrte_jobstats >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y) >>>>> -LDLIBS += -lrte_lpm >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y) >>>>> -LDLIBS += -lrte_power >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y) >>>>> -LDLIBS += -lrte_acl >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_METER),y) >>>>> -LDLIBS += -lrte_meter >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) >>>>> -LDLIBS += -lrte_sched >>>>> -LDLIBS += -lm >>>>> -LDLIBS += -lrt >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y) >>>>> -LDLIBS += -lrte_vhost >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost >>>>> >>>>> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >>>>> -LDLIBS += -lpcap >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lpcap >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn) >>>>> -LDLIBS += -lfuse >>>>> +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n) >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse >>>>> endif >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >>>>> -LDLIBS += -libverbs >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -libverbs >>>>> >>>>> -LDLIBS += --start-group >>>>> +LDLIBS-y += --start-group >>>>> >>>>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y) >>>>> -LDLIBS += -lrte_kvargs >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y) >>>>> -LDLIBS += -lrte_mbuf >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y) >>>>> -LDLIBS += -lrte_ip_frag >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y) >>>>> -LDLIBS += -lethdev >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y) >>>>> -LDLIBS += -lrte_malloc >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y) >>>>> -LDLIBS += -lrte_mempool >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_RING),y) >>>>> -LDLIBS += -lrte_ring >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_EAL),y) >>>>> -LDLIBS += -lrte_eal >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y) >>>>> -LDLIBS += -lrte_cmdline >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y) >>>>> -LDLIBS += -lrte_cfgfile >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y) >>>>> -LDLIBS += -lrte_pmd_bond >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y) >>>>> -LDLIBS += -lrte_pmd_xenvirt >>>>> -LDLIBS += -lxenstore >>>>> -endif >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER) += -lethdev >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC) += -lrte_malloc >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += -lrte_mempool >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond >>>>> + >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lxenstore >>>>> >>>>> ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) >>>>> # plugins (link only if static libraries) >>>>> >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) >>>>> -LDLIBS += -lrte_pmd_vmxnet3_uio >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) >>>>> -LDLIBS += -lrte_pmd_virtio >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y) >>>>> -LDLIBS += -lrte_pmd_enic >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y) >>>>> -LDLIBS += -lrte_pmd_i40e >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y) >>>>> -LDLIBS += -lrte_pmd_fm10k >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y) >>>>> -LDLIBS += -lrte_pmd_ixgbe >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y) >>>>> -LDLIBS += -lrte_pmd_e1000 >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y) >>>>> -LDLIBS += -lrte_pmd_mlx4 >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y) >>>>> -LDLIBS += -lrte_pmd_ring >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y) >>>>> -LDLIBS += -lrte_pmd_pcap >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y) >>>>> -LDLIBS += -lrte_pmd_af_packet >>>>> -endif >>>>> - >>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y) >>>>> -LDLIBS += -lrte_pmd_null >>>>> -endif >>>>> - >>>>> -endif # plugins >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet >>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null >>>>> + >>>>> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) >>>>> >>>>> endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS >>>>> >>>>> -LDLIBS += $(EXECENV_LDLIBS) >>>>> - >>>>> -LDLIBS += --end-group >>>>> - >>>>> -LDLIBS += --no-whole-archive >>>>> +LDLIBS-y += $(EXECENV_LDLIBS) >>>>> +LDLIBS-y += --end-group >>>>> +LDLIBS-y += --no-whole-archive >>>>> >>>>> endif # ifeq ($(NO_AUTOLIBS),) >>>>> >>>>> -LDLIBS += $(CPU_LDLIBS) >>>>> +LDLIBS-y += $(EXTRA_LDLIBS) >>>>> >>>>> .PHONY: all >>>>> all: install >>>>> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1) >>>>> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >>>>> O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \ >>>>> -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call >>>>> linkerprefix,$(LDFLAGS)) \ >>>>> - $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS)) >>>>> + $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y)) >>>>> else >>>>> O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >>>>> - -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS) >>>>> + -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y) >>>>> endif >>>>> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >>>>> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," LD $(@)") >>>>> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \ >>>>> -include .$(APP).cmd >>>>> >>>>> # path where libraries are retrieved >>>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>>> -Wl$(comma)-L%,$(LDLIBS))) >>>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >>>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>>> -Wl$(comma)-L%,$(LDLIBS-y))) >>>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >>>>> >>>>> # list of .a files that are linked to this application >>>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >>>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>>> -Wl$(comma)-l%,$(LDLIBS))) >>>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >>>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>>> -Wl$(comma)-l%,$(LDLIBS-y))) >>>>> >>>>> # list of found libraries files (useful for deps). If not found, >>>>> the >>>>> # library is silently ignored and dep won't be checked >>>>> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk >>>>> index c44d0f8..51e8c1d 100644 >>>>> --- a/mk/rte.hostapp.mk >>>>> +++ b/mk/rte.hostapp.mk >>>>> @@ -58,7 +58,7 @@ build: _postbuild >>>>> exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1)))) >>>>> >>>>> O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \ >>>>> - $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS) >>>>> + $(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y) >>>>> O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight >>>>> O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)"," HOSTLD $(@)") >>>>> O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)" >>>>> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \ >>>>> # list of .a files that are linked to this application >>>>> LDLIBS_FILES := $(wildcard \ >>>>> $(addprefix $(RTE_OUTPUT)/lib/, \ >>>>> - $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))))) >>>>> + $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))))) >>>>> >>>>> # >>>>> # Compile executable file if needed >>>>> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk >>>>> index fc6b0b4..56f20bb 100644 >>>>> --- a/mk/rte.shared.mk >>>>> +++ b/mk/rte.shared.mk >>>>> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst >>>>> %,%.cmd,$(1)))) >>>>> ifeq ($(LINK_USING_CC),1) >>>>> override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) >>>>> O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) >>>>> $(EXTRA_LDFLAGS) \ >>>>> - -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS)) >>>>> + -shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y)) >>>>> else >>>>> O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \ >>>>> - -shared -o $@ $(OBJS-y) $(LDLIBS) >>>>> + -shared -o $@ $(OBJS-y) $(LDLIBS-y) >>>>> endif >>>>> >>>>> O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight >>>>> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \ >>>>> -include .$(SHARED).cmd >>>>> >>>>> # path where libraries are retrieved >>>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>>> -Wl$(comma)-L%,$(LDLIBS))) >>>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS))) >>>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter >>>>> -Wl$(comma)-L%,$(LDLIBS-y))) >>>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y))) >>>>> >>>>> # list of .a files that are linked to this application >>>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS))) >>>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>>> -Wl$(comma)-l%,$(LDLIBS))) >>>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y))) >>>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter >>>>> -Wl$(comma)-l%,$(LDLIBS-y))) >>>>> >>>>> # list of found libraries files (useful for deps). If not found, >>>>> the >>>>> # library is silently ignored and dep won't be checked >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-30 14:31 ` Wiles, Keith 2015-04-30 15:56 ` Olivier MATZ @ 2015-04-30 16:22 ` Bruce Richardson 2015-04-30 16:33 ` Wiles, Keith 1 sibling, 1 reply; 12+ messages in thread From: Bruce Richardson @ 2015-04-30 16:22 UTC (permalink / raw) To: Wiles, Keith; +Cc: dev On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote: > > > On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: > > >Hi Keith, > > > >On 04/30/2015 03:24 PM, Wiles, Keith wrote: > >> > >> > >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: > >> > >>> Hi Keith, > >>> > >>> Thank you for submitting a clean-up. Please see some comments below. > >>> > >>> On 04/29/2015 05:25 PM, Keith Wiles wrote: > >>>> Trying to simplify the ifdefs in rte.app.mk to make the code > >>>> more readable and maintainable by moving LDLIBS variable to use > >>>> the same style as LDLIBS-y being used in the rest of the code. > >>>> > >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps > >>>> instead of using LDLIBS directly. > >>> > >>> If I understand well, the goal of this patch is only a cleanup in > >>> rte.app.mk, but at the end, it changes the makefile user "API", > >>> which could probably be a problem for applications using the > >>> dpdk makefile framework. > >>> > >>> Why not just having an temporary internal variable (let's say > >>> _LDLIBS-y) that would allow to do the clean-up without modifying > >>> the user interface? > >>> > >>> Also, with your patch, the approach for EXTRA_LDLIBS would be > >>> different than CFLAGS or LDFLAGS: > >>> - CFLAGS/LDFLAGS are in Makefiles only > >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line > >>> to add flags to the default ones > >>> > >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, > >>> keeping a compatibility with existing application Makefiles. > >> > >> The docs for DPDK 28.3.6 states they can be used for both command line > >>and > >> Makefile, so I think I like the current solution unless everyone wants > >>it > >> as you suggested. > >> > >> > >>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.htm > >>l > > > > From the link you have sent: > > > >- About CFLAGS: > > > >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only > >[...] > >CFLAGS: Flags to use for C compilation. The user should use += to append > >data in this variable." > > > >nothing in 28.3.6 > > > > > >- About EXTRA_CFLAGS: > > > >nothing in 28.3.4 > > > >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile > >or Command Line > >[...] > >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when > >compiling." > > The point was that EXTRA_XXX can be used for command line and Makefile as > it was pointed out in a previous email the assumption was EXTRA_XXX was > only for the command line. (Just to make sure we understood EXTRA_XXX was > not just for command line options.) This was the reason I sent the link an > to point out using EXTRA_XXX is a much cleaner method then allowing > someone to modify what I believe is an internal variable. Just beware that setting EXTRA_* flags on the commandline can override their values in the makefiles, and cause unexpected compilation problems. Therefore, it tends to be best to avoid using the EXTRA_* variables for variables essential to compile. For example: putting "-g -O3" in EXTRA_CFLAGS is ok, as the if the useroverrides those with something else things should still work, but putting "-I/path/to/include" would not be. /Bruce ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-30 16:22 ` Bruce Richardson @ 2015-04-30 16:33 ` Wiles, Keith 2015-05-01 9:09 ` Bruce Richardson 0 siblings, 1 reply; 12+ messages in thread From: Wiles, Keith @ 2015-04-30 16:33 UTC (permalink / raw) To: Richardson, Bruce; +Cc: dev On 4/30/15, 11:22 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote: >On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote: >> >> >> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: >> >> >Hi Keith, >> > >> >On 04/30/2015 03:24 PM, Wiles, Keith wrote: >> >> >> >> >> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: >> >> >> >>> Hi Keith, >> >>> >> >>> Thank you for submitting a clean-up. Please see some comments below. >> >>> >> >>> On 04/29/2015 05:25 PM, Keith Wiles wrote: >> >>>> Trying to simplify the ifdefs in rte.app.mk to make the code >> >>>> more readable and maintainable by moving LDLIBS variable to use >> >>>> the same style as LDLIBS-y being used in the rest of the code. >> >>>> >> >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps >> >>>> instead of using LDLIBS directly. >> >>> >> >>> If I understand well, the goal of this patch is only a cleanup in >> >>> rte.app.mk, but at the end, it changes the makefile user "API", >> >>> which could probably be a problem for applications using the >> >>> dpdk makefile framework. >> >>> >> >>> Why not just having an temporary internal variable (let's say >> >>> _LDLIBS-y) that would allow to do the clean-up without modifying >> >>> the user interface? >> >>> >> >>> Also, with your patch, the approach for EXTRA_LDLIBS would be >> >>> different than CFLAGS or LDFLAGS: >> >>> - CFLAGS/LDFLAGS are in Makefiles only >> >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line >> >>> to add flags to the default ones >> >>> >> >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, >> >>> keeping a compatibility with existing application Makefiles. >> >> >> >> The docs for DPDK 28.3.6 states they can be used for both command >>line >> >>and >> >> Makefile, so I think I like the current solution unless everyone >>wants >> >>it >> >> as you suggested. >> >> >> >> >> >>>>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.h >>>>tm >> >>l >> > >> > From the link you have sent: >> > >> >- About CFLAGS: >> > >> >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only >> >[...] >> >CFLAGS: Flags to use for C compilation. The user should use += to >>append >> >data in this variable." >> > >> >nothing in 28.3.6 >> > >> > >> >- About EXTRA_CFLAGS: >> > >> >nothing in 28.3.4 >> > >> >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile >> >or Command Line >> >[...] >> >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS >>when >> >compiling." >> >> The point was that EXTRA_XXX can be used for command line and Makefile >>as >> it was pointed out in a previous email the assumption was EXTRA_XXX was >> only for the command line. (Just to make sure we understood EXTRA_XXX >>was >> not just for command line options.) This was the reason I sent the link >>an >> to point out using EXTRA_XXX is a much cleaner method then allowing >> someone to modify what I believe is an internal variable. > >Just beware that setting EXTRA_* flags on the commandline can override >their >values in the makefiles, and cause unexpected compilation problems. >Therefore, >it tends to be best to avoid using the EXTRA_* variables for variables >essential >to compile. For example: putting "-g -O3" in EXTRA_CFLAGS is ok, as the >if the >useroverrides those with something else things should still work, but >putting >"-I/path/to/include" would not be. On the command line and makefile you should be using += and not just = or you run into this problem. > >/Bruce > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. 2015-04-30 16:33 ` Wiles, Keith @ 2015-05-01 9:09 ` Bruce Richardson 0 siblings, 0 replies; 12+ messages in thread From: Bruce Richardson @ 2015-05-01 9:09 UTC (permalink / raw) To: Wiles, Keith; +Cc: dev On Thu, Apr 30, 2015 at 05:33:36PM +0100, Wiles, Keith wrote: > > > On 4/30/15, 11:22 AM, "Richardson, Bruce" <bruce.richardson@intel.com> > wrote: > > >On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote: > >> > >> > >> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: > >> > >> >Hi Keith, > >> > > >> >On 04/30/2015 03:24 PM, Wiles, Keith wrote: > >> >> > >> >> > >> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote: > >> >> > >> >>> Hi Keith, > >> >>> > >> >>> Thank you for submitting a clean-up. Please see some comments below. > >> >>> > >> >>> On 04/29/2015 05:25 PM, Keith Wiles wrote: > >> >>>> Trying to simplify the ifdefs in rte.app.mk to make the code > >> >>>> more readable and maintainable by moving LDLIBS variable to use > >> >>>> the same style as LDLIBS-y being used in the rest of the code. > >> >>>> > >> >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps > >> >>>> instead of using LDLIBS directly. > >> >>> > >> >>> If I understand well, the goal of this patch is only a cleanup in > >> >>> rte.app.mk, but at the end, it changes the makefile user "API", > >> >>> which could probably be a problem for applications using the > >> >>> dpdk makefile framework. > >> >>> > >> >>> Why not just having an temporary internal variable (let's say > >> >>> _LDLIBS-y) that would allow to do the clean-up without modifying > >> >>> the user interface? > >> >>> > >> >>> Also, with your patch, the approach for EXTRA_LDLIBS would be > >> >>> different than CFLAGS or LDFLAGS: > >> >>> - CFLAGS/LDFLAGS are in Makefiles only > >> >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line > >> >>> to add flags to the default ones > >> >>> > >> >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS, > >> >>> keeping a compatibility with existing application Makefiles. > >> >> > >> >> The docs for DPDK 28.3.6 states they can be used for both command > >>line > >> >>and > >> >> Makefile, so I think I like the current solution unless everyone > >>wants > >> >>it > >> >> as you suggested. > >> >> > >> >> > >> > >>>>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.h > >>>>tm > >> >>l > >> > > >> > From the link you have sent: > >> > > >> >- About CFLAGS: > >> > > >> >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only > >> >[...] > >> >CFLAGS: Flags to use for C compilation. The user should use += to > >>append > >> >data in this variable." > >> > > >> >nothing in 28.3.6 > >> > > >> > > >> >- About EXTRA_CFLAGS: > >> > > >> >nothing in 28.3.4 > >> > > >> >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile > >> >or Command Line > >> >[...] > >> >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS > >>when > >> >compiling." > >> > >> The point was that EXTRA_XXX can be used for command line and Makefile > >>as > >> it was pointed out in a previous email the assumption was EXTRA_XXX was > >> only for the command line. (Just to make sure we understood EXTRA_XXX > >>was > >> not just for command line options.) This was the reason I sent the link > >>an > >> to point out using EXTRA_XXX is a much cleaner method then allowing > >> someone to modify what I believe is an internal variable. > > > >Just beware that setting EXTRA_* flags on the commandline can override > >their > >values in the makefiles, and cause unexpected compilation problems. > >Therefore, > >it tends to be best to avoid using the EXTRA_* variables for variables > >essential > >to compile. For example: putting "-g -O3" in EXTRA_CFLAGS is ok, as the > >if the > >useroverrides those with something else things should still work, but > >putting > >"-I/path/to/include" would not be. > > On the command line and makefile you should be using += and not just = or > you run into this problem. Using "+=" on the commandline is not normal. It's also rather tricky to do at times, if a value is already defined, as bash shell does not add in whitespace to the existing variable appropriately. For example, I have EXTRA_CFLAGS set to '-g -Wfatal-errors' in my .bashrc so I always get debug builds that stop on first error. Building the dpdk.org as below works fine: EXTRA_CFLAGS=-g gmake == Build lib == Build lib/librte_compat ... However, using += causes very strange behaviour: $ EXTRA_CFLAGS+=-g gmake == Build lib == Build lib/librte_compat == Build lib/librte_eal == Build lib/librte_net == Build lib/librte_eal/common == Build lib/librte_eal/bsdapp == Build lib/librte_eal/bsdapp/eal == Build lib/librte_eal/bsdapp/contigmem == Build lib/librte_eal/bsdapp/nic_uio Warning: Object directory not changed from original /usr/home/bruce/dpdk.org/x86_64-native-bsdapp-clang/build/lib/librte_eal/bsdapp/nic_uio Warning: Object directory not changed from original /usr/home/bruce/dpdk.org/x86_64-native-bsdapp-clang/build/lib/librte_eal/bsdapp/contigmem CC eal.o CC eal_memory.o CC eal_hugepage_info.o CC eal_thread.o error: unknown warning option '-Wfatal-errors=g' [-Werror,-Wunknown-warning-option] So, in short, you must assume that EXTRA_CFLAGS will be specified using "=" on the commandline, and that things build appropriately when the user does so. [Given you use "+=" internally in the makefiles, I assume this is the case, but flagging this as something worth double-checking] /Bruce ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-05-01 9:09 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles 2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles 2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon 2015-04-29 17:49 ` Wiles, Keith 2015-04-30 9:45 ` Olivier MATZ 2015-04-30 13:24 ` Wiles, Keith 2015-04-30 13:38 ` Olivier MATZ 2015-04-30 14:31 ` Wiles, Keith 2015-04-30 15:56 ` Olivier MATZ 2015-04-30 16:22 ` Bruce Richardson 2015-04-30 16:33 ` Wiles, Keith 2015-05-01 9:09 ` Bruce Richardson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).