Accumulated fixes v2

Accumulated fixes v2

[Jan Engelhardt]

The following changes since commit 166f20a3665a28e0f5fcedd0914c8e7d41521428:

  doc: document nat table for IPv6 (2013-01-28 12:38:21 +0100)

are available in the git repository at:

  git://git.inai.de/iptables master

for you to fetch changes up to 22a75444ef99f72235ef31ae516c15999e268b04:

  iptables: reword warning on using an alias (2013-01-28 14:39:00 +0100)

----------------------------------------------------------------
Jan Engelhardt (6):
      build: also use libtool for install stage
      build: do not dereference symlinks on installation
      libxtables: centralize checking for a .save function
      extensions: eui64: set userspacesize=0
      iptables: fall back to using save function when print is not defined
      iptables: reword warning on using an alias

 extensions/GNUmakefile.in   |   21 +++++++++++++++------
 extensions/libip6t_eui64.c  |    2 +-
 extensions/libxt_standard.c |   14 ++++++++++++++
 iptables/ip6tables.c        |   31 +++++++++++++------------------
 iptables/iptables.c         |   31 +++++++++++++------------------
 libxtables/xtables.c        |   21 +++++++++++++++++++++
 tests/options-most.rules    |    2 ++
 7 files changed, 79 insertions(+), 43 deletions(-)

[PATCH 1/6] build: also use libtool for install stage

[Jan Engelhardt]

The just-built library has DT_RPATH entries to be runnable from
whereever it is. If we just install that, the distro build systems
will throw warnings like

	(Open Build Service) ERROR: RPATH
	"/home/abuild/rpmbuild/BUILD/iptables-1.4.17a/libxtables/.libs"
	on /home/abuild/rpmbuild/BUILDROOT/iptables-1.4.17a-0.x86_64/usr/lib64/xtables/libxt_unclean.so
	is not allowed

These RPATH entries are indeed undesired for libraries is in a system
location, which is why libtool produces another copy of the library on
installation. The makefile however missed using libtool during
installation (introduced with commit v1.4.17-1-g68e77a2).
This patch now resolves this.

There will be extra "warnings" emitted by libtool during `make install`,
like:

	libtool: install: warning: relinking "libxt_AUDIT.la"

This is normal (and unfixable), and is because
${builddir}/libxt_AUDIT.so links against
"/home/user/iptables/libxtables/.libs/libxtables.so.9", which is
correct for everything in the build tree, but for installation,
"libxtables.so.9" is the right value for the DT_NEEDED entry, which is
why libtool has to, and will accordingly, relink on installation.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 extensions/GNUmakefile.in |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/extensions/GNUmakefile.in b/extensions/GNUmakefile.in
index adad4d6..e520c86 100644
--- a/extensions/GNUmakefile.in
+++ b/extensions/GNUmakefile.in
@@ -49,6 +49,9 @@ pf6_build_mod := $(filter-out @blacklist_modules@,${pf6_build_mod})
 pfx_objs      := $(patsubst %,libxt_%.o,${pfx_build_mod})
 pf4_objs      := $(patsubst %,libipt_%.o,${pf4_build_mod})
 pf6_objs      := $(patsubst %,libip6t_%.o,${pf6_build_mod})
+pfx_lalibs    := $(patsubst %,libxt_%.la,${pfx_build_mod})
+pf4_lalibs    := $(patsubst %,libipt_%.la,${pf4_build_mod})
+pf6_lalibs    := $(patsubst %,libip6t_%.la,${pf6_build_mod})
 pfx_solibs    := $(patsubst %,libxt_%.so,${pfx_build_mod} ${pfx_symlinks})
 pf4_solibs    := $(patsubst %,libipt_%.so,${pf4_build_mod})
 pf6_solibs    := $(patsubst %,libip6t_%.so,${pf6_build_mod})
@@ -63,7 +66,8 @@ targets_install :=
 @ENABLE_STATIC_TRUE@ libext4_objs := ${pf4_objs}
 @ENABLE_STATIC_TRUE@ libext6_objs := ${pf6_objs}
 @ENABLE_STATIC_FALSE@ targets += ${pfx_solibs} ${pf4_solibs} ${pf6_solibs}
-@ENABLE_STATIC_FALSE@ targets_install += ${pfx_solibs} ${pf4_solibs} ${pf6_solibs}
+@ENABLE_STATIC_FALSE@ targets_la_install += ${pfx_lalibs} ${pf4_lalibs} ${pf6_lalibs}
+@ENABLE_STATIC_FALSE@ targets_so_install += ${pfx_solibs}
 
 .SECONDARY:
 
@@ -71,9 +75,11 @@ targets_install :=
 
 all: ${targets}
 
-install: ${targets_install}
+install: ${targets_la_install} ${targets_so_install}
 	@mkdir -p "${DESTDIR}${xtlibdir}";
-	if test -n "${targets_install}"; then install -pm0755 $^ "${DESTDIR}${xtlibdir}/"; fi;
+	${AM_VERBOSE_NULL} ../libtool ${AM_LIBTOOL_SILENT} --mode=install install -pm0755 ${targets_la_install} "${DESTDIR}${xtlibdir}/"
+	rm -f "${DESTDIR}${xtlibdir}"/*.la;
+	install -pm0755 ${targets_so_install} "${DESTDIR}${xtlibdir}/"
 
 clean:
 	rm -f *.la *.o *.lo *.so *.a {matches,targets}.man initext.c initext4.c initext6.c;
@@ -94,7 +100,7 @@ lib%.so: lib%.la
 	${AM_VERBOSE_NULL} ln -fs .libs/$@ $@
 
 lib%.la: lib%.lo
-	${AM_VERBOSE_CCLD} ../libtool ${AM_LIBTOOL_SILENT} --tag=CC --mode=link ${CCLD} ${AM_LDFLAGS} -module ${LDFLAGS} -o $@ $< ../libxtables/libxtables.la ${$*_LIBADD} -rpath ${xtlibdir}
+	${AM_VERBOSE_CCLD} ../libtool ${AM_LIBTOOL_SILENT} --tag=CC --mode=link ${CCLD} ${AM_LDFLAGS} -avoid-version -module ${LDFLAGS} -o $@ $< ../libxtables/libxtables.la ${$*_LIBADD} -rpath ${xtlibdir}
 
 lib%.lo: ${srcdir}/lib%.c
 	${AM_VERBOSE_CC} ../libtool ${AM_LIBTOOL_SILENT} --tag=CC --mode=compile ${CC} ${AM_CPPFLAGS} ${AM_DEPFLAGS} ${AM_CFLAGS} -D_INIT=lib$*_init ${CFLAGS} -o $@ -c $<
-- 
1.7.10.4

[PATCH 2/6] build: do not dereference symlinks on installation

[Jan Engelhardt]

By using install(1), libxt_NOTRACK.so was inadvertently installed as
an actual file to /usr/lib/xtables rather than as a symlink. Switch to
using cp(1).

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 extensions/GNUmakefile.in |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/extensions/GNUmakefile.in b/extensions/GNUmakefile.in
index e520c86..15ede9f 100644
--- a/extensions/GNUmakefile.in
+++ b/extensions/GNUmakefile.in
@@ -52,9 +52,10 @@ pf6_objs      := $(patsubst %,libip6t_%.o,${pf6_build_mod})
 pfx_lalibs    := $(patsubst %,libxt_%.la,${pfx_build_mod})
 pf4_lalibs    := $(patsubst %,libipt_%.la,${pf4_build_mod})
 pf6_lalibs    := $(patsubst %,libip6t_%.la,${pf6_build_mod})
-pfx_solibs    := $(patsubst %,libxt_%.so,${pfx_build_mod} ${pfx_symlinks})
+pfx_solibs    := $(patsubst %,libxt_%.so,${pfx_build_mod})
 pf4_solibs    := $(patsubst %,libipt_%.so,${pf4_build_mod})
 pf6_solibs    := $(patsubst %,libip6t_%.so,${pf6_build_mod})
+pfx_solinks   := $(patsubst %,libxt_%.so,${pfx_symlinks})
 
 
 #
@@ -65,9 +66,9 @@ targets_install :=
 @ENABLE_STATIC_TRUE@ libext_objs := ${pfx_objs}
 @ENABLE_STATIC_TRUE@ libext4_objs := ${pf4_objs}
 @ENABLE_STATIC_TRUE@ libext6_objs := ${pf6_objs}
-@ENABLE_STATIC_FALSE@ targets += ${pfx_solibs} ${pf4_solibs} ${pf6_solibs}
+@ENABLE_STATIC_FALSE@ targets += ${pfx_solibs} ${pf4_solibs} ${pf6_solibs} ${pfx_solinks}
 @ENABLE_STATIC_FALSE@ targets_la_install += ${pfx_lalibs} ${pf4_lalibs} ${pf6_lalibs}
-@ENABLE_STATIC_FALSE@ targets_so_install += ${pfx_solibs}
+@ENABLE_STATIC_FALSE@ targets_so_install += ${pfx_solinks}
 
 .SECONDARY:
 
@@ -79,7 +80,9 @@ install: ${targets_la_install} ${targets_so_install}
 	@mkdir -p "${DESTDIR}${xtlibdir}";
 	${AM_VERBOSE_NULL} ../libtool ${AM_LIBTOOL_SILENT} --mode=install install -pm0755 ${targets_la_install} "${DESTDIR}${xtlibdir}/"
 	rm -f "${DESTDIR}${xtlibdir}"/*.la;
-	install -pm0755 ${targets_so_install} "${DESTDIR}${xtlibdir}/"
+	# targets_so_install contains symlinks, and these should be preserved,
+	# so using install(1) is out of the question for them.
+	cp -a ${targets_so_install} "${DESTDIR}${xtlibdir}/"
 
 clean:
 	rm -f *.la *.o *.lo *.so *.a {matches,targets}.man initext.c initext4.c initext6.c;
-- 
1.7.10.4

[PATCH 3/6] libxtables: centralize checking for a .save function

[Jan Engelhardt]

Both iptables.c and ip6tables.c check for target->save == NULL, which
can be consolidated. In fact, we should also check for match->save ==
NULL, which this patch addds to libxtables.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 extensions/libxt_standard.c |   14 ++++++++++++++
 iptables/ip6tables.c        |   12 ------------
 iptables/iptables.c         |   12 ------------
 libxtables/xtables.c        |   21 +++++++++++++++++++++
 4 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/extensions/libxt_standard.c b/extensions/libxt_standard.c
index c64ba29..601e709 100644
--- a/extensions/libxt_standard.c
+++ b/extensions/libxt_standard.c
@@ -9,12 +9,26 @@ static void standard_help(void)
 "(If target is DROP, ACCEPT, RETURN or nothing)\n");
 }
 
+static void standard_save(const void *ip, const struct xt_entry_target *t)
+{
+	/*
+	 * This function left blank intentionally - it only serves to make
+	 * iptables not exit with "target lacks a save function". The
+	 * "standard" target is special, since we do not emit -j standard, but
+	 * -j <verdict>. This is printed by iptables.c's function
+	 * print_rule4(), which ultimately calls TC_GET_TARGET in
+	 * libiptc/libiptc.c that will emit the verdict name based upon data in
+	 * the parameter block (@t->data in this "standard_save" function).
+	 */
+}
+
 static struct xtables_target standard_target = {
 	.family		= NFPROTO_UNSPEC,
 	.name		= "standard",
 	.version	= XTABLES_VERSION,
 	.size		= XT_ALIGN(sizeof(int)),
 	.userspacesize	= XT_ALIGN(sizeof(int)),
+	.save		= standard_save,
 	.help		= standard_help,
 };
 
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 556647f..371d700 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1112,18 +1112,6 @@ void print_rule6(const struct ip6t_entry *e,
 
 		if (target->save)
 			target->save(&e->ipv6, t);
-		else {
-			/* If the target size is greater than xt_entry_target
-			 * there is something to be saved, we just don't know
-			 * how to print it */
-			if (t->u.target_size !=
-			    sizeof(struct xt_entry_target)) {
-				fprintf(stderr, "Target `%s' is missing "
-						"save function\n",
-					t->u.user.name);
-				exit(1);
-			}
-		}
 	}
 	printf("\n");
 }
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 00e3f01..6e5250e 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1103,18 +1103,6 @@ void print_rule4(const struct ipt_entry *e,
 
 		if (target->save)
 			target->save(&e->ip, t);
-		else {
-			/* If the target size is greater than xt_entry_target
-			 * there is something to be saved, we just don't know
-			 * how to print it */
-			if (t->u.target_size !=
-			    sizeof(struct xt_entry_target)) {
-				fprintf(stderr, "Target `%s' is missing "
-						"save function\n",
-					t->u.user.name);
-				exit(1);
-			}
-		}
 	}
 	printf("\n");
 }
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 009ab91..b81013a 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -852,6 +852,16 @@ void xtables_register_match(struct xtables_match *me)
 		xtables_option_metavalidate(me->name, me->x6_options);
 	if (me->extra_opts != NULL)
 		xtables_check_options(me->name, me->extra_opts);
+	if (me->userspacesize > 0 && me->save == NULL &&
+	    me->real_name == NULL) {
+		/*
+		 * Catch extensions that have data to be saved, but which
+		 * forgot to define a save method.
+		 */
+		fprintf(stderr, "Match \"%s\" is missing a save function\n",
+		        me->name);
+		exit(1);
+	}
 
 	/* ignore not interested match */
 	if (me->family != afinfo->family && me->family != AF_UNSPEC)
@@ -1010,6 +1020,17 @@ void xtables_register_target(struct xtables_target *me)
 		xtables_option_metavalidate(me->name, me->x6_options);
 	if (me->extra_opts != NULL)
 		xtables_check_options(me->name, me->extra_opts);
+	if (me->userspacesize > 0 && me->save == NULL &&
+	    me->real_name == NULL) {
+		/*
+		 * Catch extensions that have data to be saved, but which
+		 * forgot to define a save method. This only applies to true
+		 * modules (real_name==NULL), not aliases.
+		 */
+		fprintf(stderr, "Target \"%s\" is missing a save function\n",
+		        me->name);
+		exit(1);
+	}
 
 	/* ignore not interested target */
 	if (me->family != afinfo->family && me->family != AF_UNSPEC)
-- 
1.7.10.4

[PATCH 4/6] extensions: eui64: set userspacesize=0

[Jan Engelhardt]

The eui64 match does not use its parameter block at all, so
userspacesize should be 0. This resolves "eui64 is missing a save
function".

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 extensions/libip6t_eui64.c |    2 +-
 tests/options-most.rules   |    2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/extensions/libip6t_eui64.c b/extensions/libip6t_eui64.c
index 607bf86..3bd7a8f 100644
--- a/extensions/libip6t_eui64.c
+++ b/extensions/libip6t_eui64.c
@@ -6,7 +6,7 @@ static struct xtables_match eui64_mt6_reg = {
 	.version	= XTABLES_VERSION,
 	.family		= NFPROTO_IPV6,
 	.size		= XT_ALIGN(sizeof(int)),
-	.userspacesize	= XT_ALIGN(sizeof(int)),
+	.userspacesize	= 0,
 };
 
 void _init(void)
diff --git a/tests/options-most.rules b/tests/options-most.rules
index ef4e7f1..c26847e 100644
--- a/tests/options-most.rules
+++ b/tests/options-most.rules
@@ -91,6 +91,8 @@
 -A matches
 -A matches -p esp -m esp --espspi 5:4294967295
 -A matches
+-A matches -m eui64
+-A matches
 -A matches -m hashlimit --hashlimit-upto 1/sec --hashlimit-burst 1 --hashlimit-name mini1 --hashlimit-htable-expire 2000
 -A matches -m hashlimit --hashlimit-upto 1/sec --hashlimit-burst 1 --hashlimit-name mini1
 -A matches -m hashlimit --hashlimit-upto 1/min --hashlimit-burst 1 --hashlimit-name mini2
-- 
1.7.10.4

[PATCH 5/6] iptables: fall back to using save function when print is not defined

[Jan Engelhardt]

This way we can avoid having to provide two dumping function for new
plugins.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 iptables/ip6tables.c |   10 ++++++++--
 iptables/iptables.c  |   10 ++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 371d700..ec7818d 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -487,10 +487,14 @@ print_match(const struct xt_entry_match *m,
 		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
 
 	if (match) {
-		if (match->print)
+		if (match->print) {
 			match->print(ip, m, numeric);
-		else
+		} else if (match->save != NULL) {
+			printf("-m %s ", match->name);
+			match->save(ip, m);
+		} else {
 			printf("%s ", match->name);
+		}
 	} else {
 		if (m->u.user.name[0])
 			printf("UNKNOWN match `%s' ", m->u.user.name);
@@ -617,6 +621,8 @@ print_firewall(const struct ip6t_entry *fw,
 		if (target->print)
 			/* Print the target information. */
 			target->print(&fw->ipv6, t, format & FMT_NUMERIC);
+		else if (target->save != NULL)
+			target->save(&fw->ipv6, t);
 	} else if (t->u.target_size != sizeof(*t))
 		printf("[%u bytes of unknown target data] ",
 		       (unsigned int)(t->u.target_size - sizeof(*t)));
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 6e5250e..5be5e78 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -472,10 +472,14 @@ print_match(const struct xt_entry_match *m,
 		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
 
 	if (match) {
-		if (match->print)
+		if (match->print) {
 			match->print(ip, m, numeric);
-		else
+		} else if (match->save != NULL) {
+			printf("-m %s ", match->name);
+			match->save(ip, m);
+		} else {
 			printf("%s ", match->name);
+		}
 	} else {
 		if (m->u.user.name[0])
 			printf("UNKNOWN match `%s' ", m->u.user.name);
@@ -602,6 +606,8 @@ print_firewall(const struct ipt_entry *fw,
 		if (target->print)
 			/* Print the target information. */
 			target->print(&fw->ip, t, format & FMT_NUMERIC);
+		else if (target->save != NULL)
+			target->save(&fw->ip, t);
 	} else if (t->u.target_size != sizeof(*t))
 		printf("[%u bytes of unknown target data] ",
 		       (unsigned int)(t->u.target_size - sizeof(*t)));
-- 
1.7.10.4

[PATCH 6/6] iptables: reword warning on using an alias

[Jan Engelhardt]

This by suggestion of Nick Edward.

References: http://marc.info/?l=netfilter&m=135829245822520&w=2
Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 iptables/ip6tables.c |    9 +++++----
 iptables/iptables.c  |    9 +++++----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index ec7818d..a8c71e2 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1223,8 +1223,8 @@ static void command_jump(struct iptables_command_state *cs)
 		strcpy(cs->target->t->u.user.name, cs->jumpto);
 	} else {
 		strcpy(cs->target->t->u.user.name, cs->target->real_name);
-		fprintf(stderr, "WARNING: The %s target is obsolete. "
-		        "Use %s instead.\n",
+		fprintf(stderr, "WARNING: The %s target is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
 		        cs->jumpto, cs->target->real_name);
 	}
 	cs->target->t->u.user.revision = cs->target->revision;
@@ -1259,8 +1259,9 @@ static void command_match(struct iptables_command_state *cs)
 		strcpy(m->m->u.user.name, m->name);
 	} else {
 		strcpy(m->m->u.user.name, m->real_name);
-		fprintf(stderr, "WARNING: The %s match is obsolete. "
-		        "Use %s instead.\n", m->name, m->real_name);
+		fprintf(stderr, "WARNING: The %s match is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
+		        m->name, m->real_name);
 	}
 	m->m->u.user.revision = m->revision;
 
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 5be5e78..fc6fdf4 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1216,8 +1216,8 @@ static void command_jump(struct iptables_command_state *cs)
 	} else {
 		/* Alias support for userspace side */
 		strcpy(cs->target->t->u.user.name, cs->target->real_name);
-		fprintf(stderr, "WARNING: The %s target is obsolete. "
-		        "Use %s instead.\n",
+		fprintf(stderr, "WARNING: The %s target is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
 		        cs->jumpto, cs->target->real_name);
 	}
 	cs->target->t->u.user.revision = cs->target->revision;
@@ -1253,8 +1253,9 @@ static void command_match(struct iptables_command_state *cs)
 		strcpy(m->m->u.user.name, m->name);
 	} else {
 		strcpy(m->m->u.user.name, m->real_name);
-		fprintf(stderr, "WARNING: The %s match is obsolete. "
-		        "Use %s instead.\n", m->name, m->real_name);
+		fprintf(stderr, "WARNING: The %s match is obsolete and will "
+		        "eventually go away. Use %s instead.\n",
+		        m->name, m->real_name);
 	}
 	m->m->u.user.revision = m->revision;
 
-- 
1.7.10.4

Re: Accumulated fixes v2

[Jan Engelhardt]

On Monday 2013-01-28 14:59, Jan Engelhardt wrote:

>----------------------------------------------------------------
>Jan Engelhardt (6):
>      build: also use libtool for install stage
>      build: do not dereference symlinks on installation
>      libxtables: centralize checking for a .save function
>      extensions: eui64: set userspacesize=0
>      iptables: fall back to using save function when print is not defined
>      iptables: reword warning on using an alias

Any disposition on this?

Re: Accumulated fixes v2

[Pablo Neira Ayuso]

On Thu, Jan 31, 2013 at 03:13:32AM +0100, Jan Engelhardt wrote:
> On Monday 2013-01-28 14:59, Jan Engelhardt wrote:
> 
> >----------------------------------------------------------------
> >Jan Engelhardt (6):
> >      build: also use libtool for install stage
> >      build: do not dereference symlinks on installation
> >      libxtables: centralize checking for a .save function
> >      extensions: eui64: set userspacesize=0
> >      iptables: fall back to using save function when print is not defined
> >      iptables: reword warning on using an alias
> 
> Any disposition on this?

Jozsef's patchset is more important at this moment. Once those
applied, I'll look again into these.

Re: [PATCH 1/6] build: also use libtool for install stage

[Pablo Neira Ayuso]

On Mon, Jan 28, 2013 at 02:59:41PM +0100, Jan Engelhardt wrote:
> The just-built library has DT_RPATH entries to be runnable from
> whereever it is. If we just install that, the distro build systems
> will throw warnings like
> 
> 	(Open Build Service) ERROR: RPATH
> 	"/home/abuild/rpmbuild/BUILD/iptables-1.4.17a/libxtables/.libs"
> 	on /home/abuild/rpmbuild/BUILDROOT/iptables-1.4.17a-0.x86_64/usr/lib64/xtables/libxt_unclean.so
> 	is not allowed
> 
> These RPATH entries are indeed undesired for libraries is in a system
> location, which is why libtool produces another copy of the library on
> installation. The makefile however missed using libtool during
> installation (introduced with commit v1.4.17-1-g68e77a2).
> This patch now resolves this.
> 
> There will be extra "warnings" emitted by libtool during `make install`,
> like:
> 
> 	libtool: install: warning: relinking "libxt_AUDIT.la"
> 
> This is normal (and unfixable)

I prefer that distros live with that warning. Users will be annoyed by
that warning for sure.

Re: [PATCH 1/6] build: also use libtool for install stage

[Jan Engelhardt]

On Tuesday 2013-02-05 04:30, Pablo Neira Ayuso wrote:

>On Mon, Jan 28, 2013 at 02:59:41PM +0100, Jan Engelhardt wrote:
>> The just-built library has DT_RPATH entries to be runnable from
>> whereever it is. If we just install that, the distro build systems
>> will throw warnings like
>> 
>> 	(Open Build Service) ERROR: RPATH
>> 	"/home/abuild/rpmbuild/BUILD/iptables-1.4.17a/libxtables/.libs"
>> 	on /home/abuild/rpmbuild/BUILDROOT/iptables-1.4.17a-0.x86_64/usr/lib64/xtables/libxt_unclean.so
>> 	is not allowed
>> 
>> These RPATH entries are indeed undesired for libraries is in a system
>> location, which is why libtool produces another copy of the library on
>> installation. The makefile however missed using libtool during
>> installation (introduced with commit v1.4.17-1-g68e77a2).
>> This patch now resolves this.
>> 
>> There will be extra "warnings" emitted by libtool during `make install`,
>> like:
>> 
>> 	libtool: install: warning: relinking "libxt_AUDIT.la"
>> 
>> This is normal (and unfixable)
>
>I prefer that distros live with that warning.

It is not just a warning that distros have to face. It's a complete
abort of the build.

>Users will be annoyed by that warning for sure. 
   
No they won't. They know libtool, they know what to expect.
But whatever, I can add --no-warn, at the expensive of people
who like to know what is going on.

Re: [PATCH 1/6] build: also use libtool for install stage

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

On Tue, Feb 05, 2013 at 04:30:07AM +0100, Pablo Neira Ayuso wrote:
> On Mon, Jan 28, 2013 at 02:59:41PM +0100, Jan Engelhardt wrote:
> > The just-built library has DT_RPATH entries to be runnable from
> > whereever it is. If we just install that, the distro build systems
> > will throw warnings like
> > 
> > 	(Open Build Service) ERROR: RPATH
> > 	"/home/abuild/rpmbuild/BUILD/iptables-1.4.17a/libxtables/.libs"
> > 	on /home/abuild/rpmbuild/BUILDROOT/iptables-1.4.17a-0.x86_64/usr/lib64/xtables/libxt_unclean.so
> > 	is not allowed
> > 
> > These RPATH entries are indeed undesired for libraries is in a system
> > location, which is why libtool produces another copy of the library on
> > installation. The makefile however missed using libtool during
> > installation (introduced with commit v1.4.17-1-g68e77a2).
> > This patch now resolves this.
> > 
> > There will be extra "warnings" emitted by libtool during `make install`,
> > like:
> > 
> > 	libtool: install: warning: relinking "libxt_AUDIT.la"
> > 
> > This is normal (and unfixable)
> 
> I prefer that distros live with that warning. Users will be annoyed by
> that warning for sure.

There is a clear misunderstanding on your side, a RPATH entry containing
buildroot is a FATAL ERROR that distro maintainers have to fix just to
release new builds of iptables package.  The question is how long distro
maintainers would have to reinvent the wheel and rebase these build fixes
onto new iptables releases.  To be honest, I was much surprised to find
out that these build fixes didn't get into 1.4.18.

With regards to libtool's warnings about relinking, it happens to all
hundreds of packages that manipulate RPATH entries during build, so
the amount of annoyance among libtool users would hardly increase.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/6] build: also use libtool for install stage

[Jan Engelhardt]

On Wednesday 2013-03-06 12:08, Dmitry V. Levin wrote:
>> > 
>> > There will be extra "warnings" emitted by libtool during `make install`,
>> > like:
>> > 
>> > 	libtool: install: warning: relinking "libxt_AUDIT.la"
>> > 
>> > This is normal (and unfixable)
>> 
>> I prefer that distros live with that warning. Users will be annoyed by
>> that warning for sure.
>
>[...]
>With regards to libtool's warnings about relinking, it happens to all
>hundreds of packages that manipulate RPATH entries during build, so
>the amount of annoyance among libtool users would hardly increase.

That said, the --no-warn option is only available in libtool >= 2.4.2
or so; it is not present in 2.4(.0).
I nevertheless have a patch (unpushed) ontop of my latest set (pushed).

Re: [PATCH 1/6] build: also use libtool for install stage

[Pablo Neira Ayuso]

On Wed, Mar 06, 2013 at 03:08:36PM +0400, Dmitry V. Levin wrote:
[...]
> With regards to libtool's warnings about relinking, it happens to all
> hundreds of packages that manipulate RPATH entries during build, so
> the amount of annoyance among libtool users would hardly increase.

The thing is that the libtool patch for iptables was introduced to
resolve a linking problem in ip6t_NETMAP. That could have been easily
fixed with a little patch by using xtables_ip6mask_to_cidr.

I'm going to post a patch to fix that linking issue that way and roll
back to the previous situation where we were not using libtool.