diff mbox

build: unbreak linkage of m_xt.so

Message ID 1355617968-26138-1-git-send-email-jengelh@inai.de
State Not Applicable
Headers show

Commit Message

Jan Engelhardt Dec. 16, 2012, 12:32 a.m. UTC
Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
to be present at the time of calling make, leading to tc/m_xt.so
not linked with -lxtables (result from pkg-config xtables --libs),
that in turn leading to

tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
xtables_init_all

Fixing that.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 configure |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jamal Hadi Salim Dec. 16, 2012, 10:30 a.m. UTC | #1
On 12-12-15 07:32 PM, Jan Engelhardt wrote:
> Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
> to be present at the time of calling make, leading to tc/m_xt.so
> not linked with -lxtables (result from pkg-config xtables --libs),
> that in turn leading to
>
> tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
> xtables_init_all
>

Yep - run into this problem, scratching my head thinking something
wrong with my environment with latest iproute2 git tree. I hacked
mine to just always include xtables in LDLIBS.

> Fixing that.
>
> Signed-off-by: Jan Engelhardt <jengelh@inai.de>

I can confirm it builds fine for me now if i take out the hack I had and 
use this patch.
Acked-by: Jamal Hadi Salim <hadi@mojatatu.com>

Stephen, I think this patch would be equivalent of "stable fix".

cheers,
jamal

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jamal Hadi Salim Dec. 16, 2012, 5:03 p.m. UTC | #2
On 12-12-16 05:30 AM, Jamal Hadi Salim wrote:

> I can confirm it builds fine for me now if i take out the hack I had and
> use this patch.


Sorry, I take what i said back and went back to explicitly adding -l 
xtables. The problem is still the intepretation of tc/Makefile. Here's 
the compile output.
----
gcc -Wall -Wstrict-prototypes -O2 -I../include -DRESOLVE_HOSTNAMES 
-DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE 
-DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" 
-DYY_NO_INPUT -Wl,-export-dynamic -shared -fpic -o m_xt.so m_xt.c 
$(pkg-config xtables --cflags --libs)
----

Note the missing expansion.

cheers,
jamal
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt Dec. 16, 2012, 5:43 p.m. UTC | #3
On Sunday 2012-12-16 18:03, Jamal Hadi Salim wrote:

> On 12-12-16 05:30 AM, Jamal Hadi Salim wrote:
>
>> I can confirm it builds fine for me now if i take out the hack I had and
>> use this patch.
>
>
> Sorry, I take what i said back and went back to explicitly adding -l xtables.
> The problem is still the intepretation of tc/Makefile. Here's the compile
> output.
> ----
> gcc -Wall -Wstrict-prototypes -O2 -I../include -DRESOLVE_HOSTNAMES
> -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -DCONFIG_GACT
> -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT
> -Wl,-export-dynamic -shared -fpic -o m_xt.so m_xt.c $(pkg-config xtables
> --cflags --libs)
> ----

I saw the same during make, _but_, on running `ldd tc/m_xt.so`, I got a
libxtables.so entry, so I thought I was fine.



"$() "is something for the shell to expand, not make. See this testcase.

$ make
echo $(pkg-config xtables --cflags --libs)
-I/usr/include/iptables-1.4.16.3 -lxtables
$ cat Makefile 
a:
        echo $$(pkg-config xtables --cflags --libs)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jamal Hadi Salim Dec. 16, 2012, 6:05 p.m. UTC | #4
On 12-12-16 12:43 PM, Jan Engelhardt wrote:
> On Sunday 2012-12-16 18:03, Jamal Hadi Salim wrote:
>

>
> I saw the same during make, _but_, on running `ldd tc/m_xt.so`, I got a
> libxtables.so entry, so I thought I was fine.
>

Sorry, you are right. Without your patch that doesnt happen. I had 
removed the global dlopen while debugging, so it was using the wrong
m_xt.so

So my Ack is back on;->

cheers,
jamal
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Frysinger Dec. 16, 2012, 10:02 p.m. UTC | #5
On Saturday 15 December 2012 19:32:48 Jan Engelhardt wrote:
> --- a/configure
> +++ b/configure
> @@ -4,7 +4,6 @@
>  INCLUDE=${1:-"$PWD/include"}
> 
>  : ${PKG_CONFIG:=pkg-config}
>  : ${CC=gcc}
> 
> -echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
> 
>  # Make a temp directory in build tree.
>  TMPDIR=$(mktemp -d config.XXXXXX)
> @@ -224,6 +223,7 @@ rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
>  }
> 
>  echo "# Generated config based on" $INCLUDE >Config
> +echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
> 
>  echo "TC schedulers"

the use of un-indented shell functions makes the code read in a way it doesn't 
actually execute.  i'd suggest moving this logic into a function to match 
existing style rather than simply moving the Config write.  i'll post a patch.
-mike
stephen hemminger Dec. 18, 2012, 5:21 p.m. UTC | #6
On Sun, 16 Dec 2012 01:32:48 +0100
Jan Engelhardt <jengelh@inai.de> wrote:

> Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
> to be present at the time of calling make, leading to tc/m_xt.so
> not linked with -lxtables (result from pkg-config xtables --libs),
> that in turn leading to
> 
> tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
> xtables_init_all
> 
> Fixing that.
> 
> Signed-off-by: Jan Engelhardt <jengelh@inai.de>
> ---
>  configure |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 9912114..573ee55 100755
> --- a/configure
> +++ b/configure
> @@ -4,7 +4,6 @@
>  INCLUDE=${1:-"$PWD/include"}
>  : ${PKG_CONFIG:=pkg-config}
>  : ${CC=gcc}
> -echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
>  
>  # Make a temp directory in build tree.
>  TMPDIR=$(mktemp -d config.XXXXXX)
> @@ -224,6 +223,7 @@ rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
>  }
>  
>  echo "# Generated config based on" $INCLUDE >Config
> +echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
>  
>  echo "TC schedulers"
>  

Ok, manually did the diff (conflicted with other previous changes).
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Frysinger Dec. 18, 2012, 6:47 p.m. UTC | #7
On Tuesday 18 December 2012 12:21:30 Stephen Hemminger wrote:
> On Sun, 16 Dec 2012 01:32:48 +0100 Jan Engelhardt wrote:
> > Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
> > to be present at the time of calling make, leading to tc/m_xt.so
> > not linked with -lxtables (result from pkg-config xtables --libs),
> > that in turn leading to
> > 
> > tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
> > xtables_init_all
> > 
> > Fixing that.
> > 
> > --- a/configure
> > +++ b/configure
> > @@ -4,7 +4,6 @@
> >  INCLUDE=${1:-"$PWD/include"}
> >  : ${PKG_CONFIG:=pkg-config}
> >  : ${CC=gcc}
> > -echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
> > 
> >  # Make a temp directory in build tree.
> >  TMPDIR=$(mktemp -d config.XXXXXX)
> > @@ -224,6 +223,7 @@ rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
> >  }
> >  
> >  echo "# Generated config based on" $INCLUDE >Config
> > +echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
> >  echo "TC schedulers"
> 
> Ok, manually did the diff (conflicted with other previous changes).

this patch is no longer necessary one you merged my:
	configure: move toolchain init to a function

it's actually undesirable to apply this after that since it makes the configure 
script less clear again ...

sorry if my commit message wasn't obvious.
-mike
stephen hemminger Dec. 20, 2012, 12:03 a.m. UTC | #8
On Tue, 18 Dec 2012 13:47:58 -0500
Mike Frysinger <vapier@gentoo.org> wrote:

> this patch is no longer necessary one you merged my:
> 	configure: move toolchain init to a function
> 
> it's actually undesirable to apply this after that since it makes the configure 
> script less clear again ...
> 
> sorry if my commit message wasn't obvious.
> -mike

ok, went back to old way.
diff mbox

Patch

diff --git a/configure b/configure
index 9912114..573ee55 100755
--- a/configure
+++ b/configure
@@ -4,7 +4,6 @@ 
 INCLUDE=${1:-"$PWD/include"}
 : ${PKG_CONFIG:=pkg-config}
 : ${CC=gcc}
-echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
 
 # Make a temp directory in build tree.
 TMPDIR=$(mktemp -d config.XXXXXX)
@@ -224,6 +223,7 @@  rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
 }
 
 echo "# Generated config based on" $INCLUDE >Config
+echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
 
 echo "TC schedulers"