From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH 1/2] mk: prevent overlinking in applications
Date: Thu, 09 Jun 2016 12:10:18 +0200
Message-ID: <7354386.zBuGXxEf9j@xps13>
References: <574872B3.6040702@intel.com>
 <1464367686-3475-1-git-send-email-ferruh.yigit@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, Panu Matilainen <pmatilai@redhat.com>,
 Christian Ehrhardt <christian.ehrhardt@canonical.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52])
 by dpdk.org (Postfix) with ESMTP id 9C002C37C
 for <dev@dpdk.org>; Thu,  9 Jun 2016 12:10:20 +0200 (CEST)
Received: by mail-wm0-f52.google.com with SMTP id m124so53003935wme.1
 for <dev@dpdk.org>; Thu, 09 Jun 2016 03:10:20 -0700 (PDT)
In-Reply-To: <1464367686-3475-1-git-send-email-ferruh.yigit@intel.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Ferruh,

2016-05-27 17:48, Ferruh Yigit:
> Replace --no-as-needed linker flag with --as-needed flag, which will
> only link libraries directly called by application. This requires inter
> library dependencies resolved correctly.
> 
> Not linking all libraries cause a compile error for lpcap and possible
> to have other similar compiler errors, so increasing the scope of
> --start-group argument.

What is the error?

> cmdline_test application causes compile error because of cyclic
> dependency between librte_eal <-> librte_mempool. A workaround added to
> cmdline_test for compile error.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

> --- a/app/cmdline_test/Makefile
> +++ b/app/cmdline_test/Makefile
> @@ -46,6 +46,7 @@ SRCS-y += commands.c
>  
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)

A comment is required here to explain the workaround.

> +LDFLAGS += -no-as-needed

The option should be --no-as-needed.

> --- a/mk/exec-env/linuxapp/rte.vars.mk
> +++ b/mk/exec-env/linuxapp/rte.vars.mk
> @@ -46,7 +46,7 @@ EXECENV_CFLAGS  = -pthread
>  endif
>  
>  # Workaround lack of DT_NEEDED entry

This comment is obsolete now.

> -EXECENV_LDFLAGS = --no-as-needed
> +EXECENV_LDFLAGS = --as-needed

Why put this option for Linux only?
Shouldn't we restrict this option to app.mk only?

> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -58,6 +58,7 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib
>  #
>  
>  _LDLIBS-y += --whole-archive
> +_LDLIBS-y += --start-group

--start-group must be used only to solve circular dependencies.
Ideally we should not use it. I think it's needed only because
of EAL logs using mempool (must be removed).
Why extending it?
I'm afraid we are masking some issues.

>  _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)    += -lrte_distributor
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
> @@ -111,8 +112,6 @@ _LDLIBS-y                                   += -lcrypto
>  endif
>  endif # !CONFIG_RTE_BUILD_SHARED_LIBS
>  
> -_LDLIBS-y += --start-group
> -
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)         += -lrte_kvargs
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)           += -lrte_mbuf
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)        += -lrte_ip_frag