Skip to contentSkip to conversation

configure: add support for libdir option

PATCH Applied Andrea Claudi 3 participants 13 messages Oct 14, 2021 – Oct 16, 2021

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v5 cover configure: add support for libdir option

This series add support for the libdir parameter in iproute2 configure script. The idea is to make use of the fact that packaging systems may assume that 'configure' comes from autotools allowing a syntax similar to the autotools one, and using it to tell iproute2 where the distro expects to find its lib files.

Patches 1-2 fix a parsing issue on current configure options, that may trigger an endless loop when no value is provided with some options;

Patch 3 fixes a parsing issue bailing out when more than one value is provided for a single option;

Patch 4 simplifies options parsing, moving semantic checks out of the while loop processing options;

Patch 5 introduces support for the --opt=value style on current options, for uniformity;

Patch 6 adds the --prefix option, that may be used by some packaging systems when calling the configure script;

Patch 7 finally adds the --libdir option, and also drops the static LIBDIR var from the Makefile.

Changelog:
----------
v4 -> v5
- bail out when multiple values are provided with a single option
- simplify option parsing and reduce code duplication, as suggested by Phil Sutter
- remove a nasty eval on libdir option processing

v3 -> v4
- fix parsing issue on '--include_dir' and '--libbpf_dir'
- split '--opt value' and '--opt=value' use cases, avoid code duplication moving semantic checks on value to dedicated functions

v2 -> v3
- fix parsing error on prefix and libdir options.

v1 -> v2
- consolidate '--opt value' and '--opt=value' use cases, as suggested by David Ahern.
- added patch 2 to manage the --prefix option, used by the Debian packaging system, as reported by Luca Boccassi, and use it when setting lib directory.

Andrea Claudi (7):
configure: fix parsing issue on include_dir option
configure: fix parsing issue on libbpf_dir option
configure: fix parsing issue with more than one value per option
configure: simplify options parsing
configure: support --param=value style
configure: add the --prefix option
configure: add the --libdir option

 Makefile  |  7 ++---
 configure | 78 +++++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 63 insertions(+), 22 deletions(-)
Signature · 1 line
2.31.1
#1 lore raw

PATCH v5 1/7 configure: fix parsing issue on include_dir option

configure is stuck in an endless loop if '--include_dir' option is used without a value:

$ ./configure --include_dir
./configure: line 506: shift: 2: shift count out of range
./configure: line 506: shift: 2: shift count out of range
[...]

Fix it splitting 'shift 2' into two consecutive shifts, and making the second one conditional to the number of remaining arguments.

A check is also provided after the while loop to verify the include dir exists; this avoid to produce an erroneous configuration.

Fixes a9c3d70d902a ("configure: add options ability")
---
 configure | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
configure +6 -3
index 7f4f3bd9..ea9051ab 100755
@@ -485,7 +485,7 @@ usage()
 {
 	cat <<EOF
 Usage: $0 [OPTIONS]
-	--include_dir		Path to iproute2 include dir
+	--include_dir <dir>	Path to iproute2 include dir
 	--libbpf_dir		Path to libbpf DESTDIR
 	--libbpf_force		Enable/disable libbpf by force. Available options:
 				  on: require link against libbpf, quit config if no libbpf support
@@ -502,8 +502,9 @@ else
 	while true; do
 		case "$1" in
 			--include_dir)
-				INCLUDE=$2
-				shift 2 ;;
+				shift
+				INCLUDE="$1"
+				[ "$#" -gt 0 ] && shift ;;
 			--libbpf_dir)
 				LIBBPF_DIR="$2"
 				shift 2 ;;
@@ -523,6 +524,8 @@ else
 	done
 fi
 
+[ -d "$INCLUDE" ] || usage 1
+
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG
 
Signature · 1 line
2.31.1
#2 lore raw

PATCH v5 2/7 configure: fix parsing issue on libbpf_dir option

In reply to Andrea Claudi

configure is stuck in an endless loop if '--libbpf_dir' option is used without a value:

$ ./configure --libbpf_dir
./configure: line 515: shift: 2: shift count out of range
./configure: line 515: shift: 2: shift count out of range
[...]

Fix it splitting 'shift 2' into two consecutive shifts, and making the second one conditional to the number of remaining arguments.

A check is also provided after the while loop to verify the libbpf dir exists; also, as LIBBPF_DIR does not have a default value, configure bails out if the user does not specify a value after --libbpf_dir, thus avoiding to produce an erroneous configuration.

Fixes 7ae2585b865a ("configure: convert LIBBPF environment variables to command-line options")
---
 configure | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
configure +7 -3
index ea9051ab..0f304206 100755
@@ -486,7 +486,7 @@ usage()
 	cat <<EOF
 Usage: $0 [OPTIONS]
 	--include_dir <dir>	Path to iproute2 include dir
-	--libbpf_dir		Path to libbpf DESTDIR
+	--libbpf_dir <dir>	Path to libbpf DESTDIR
 	--libbpf_force		Enable/disable libbpf by force. Available options:
 				  on: require link against libbpf, quit config if no libbpf support
 				  off: disable libbpf probing
@@ -506,8 +506,9 @@ else
 				INCLUDE="$1"
 				[ "$#" -gt 0 ] && shift ;;
 			--libbpf_dir)
-				LIBBPF_DIR="$2"
-				shift 2 ;;
+				shift
+				LIBBPF_DIR="$1"
+				[ "$#" -gt 0 ] && shift ;;
 			--libbpf_force)
 				if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then
 					usage 1
@@ -525,6 +526,9 @@ else
 fi
 
 [ -d "$INCLUDE" ] || usage 1
+if [ "${LIBBPF_DIR-unused}" != "unused" ]; then
+	[ -d "$LIBBPF_DIR" ] || usage 1
+fi
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG
Signature · 1 line
2.31.1
#3 lore raw

PATCH v5 3/7 configure: fix parsing issue with more than one value per option

In reply to Andrea Claudi

With commit a9c3d70d902a ("configure: add options ability") users are no more able to provide wrong command lines like:

$ ./configure --include_dir foo bar

The script simply bails out when user provides more than one value for a single option. However, in doing so, it breaks backward compatibility with some packaging system, which expects unknown options to be ignored.

Commit a3272b93725a ("configure: restore backward compatibility") fix this issue, but makes it possible again for users to provide wrong command lines such as the one above.

This fixes the issue simply ignoring autoconf-like options such as '--opt=value'.

Fixes a3272b93725a ("configure: restore backward compatibility")
---
 configure | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
configure +3 -1
index 0f304206..9ec19a5b 100755
@@ -517,10 +517,12 @@ else
 				shift 2 ;;
 			-h | --help)
 				usage 0 ;;
+			--*)
+				shift ;;
 			"")
 				break ;;
 			*)
-				shift 1 ;;
+				usage 1 ;;
 		esac
 	done
 fi
Signature · 1 line
2.31.1
#4 lore raw

PATCH v5 4/7 configure: simplify options parsing

In reply to Andrea Claudi

This commit simplifies options parsing moving all the code not related to parsing out of the case statement.

- The conditional shift after the assignments is moved right after the case, reducing code duplication.
- The semantic checks on the LIBBPF_FORCE value is moved after the loop like we already did for INCLUDE and LIBBPF_DIR.
- Finally, the loop condition is changed to check remaining arguments, thus making it possible to get rid of the null string case break.

As a bonus, now the help message states that on or off should follow --libbpf_force

---
 configure | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)
configure +18 -19
index 9ec19a5b..26e06eb8 100755
@@ -485,12 +485,12 @@ usage()
 {
 	cat <<EOF
 Usage: $0 [OPTIONS]
-	--include_dir <dir>	Path to iproute2 include dir
-	--libbpf_dir <dir>	Path to libbpf DESTDIR
-	--libbpf_force		Enable/disable libbpf by force. Available options:
-				  on: require link against libbpf, quit config if no libbpf support
-				  off: disable libbpf probing
-	-h | --help		Show this usage info
+	--include_dir <dir>		Path to iproute2 include dir
+	--libbpf_dir <dir>		Path to libbpf DESTDIR
+	--libbpf_force <on|off>		Enable/disable libbpf by force. Available options:
+					  on: require link against libbpf, quit config if no libbpf support
+					  off: disable libbpf probing
+	-h | --help			Show this usage info
 EOF
 	exit $1
 }
@@ -499,31 +499,25 @@ EOF
 if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then
 	INCLUDE="$1"
 else
-	while true; do
+	while [ "$#" -gt 0 ]; do
 		case "$1" in
 			--include_dir)
 				shift
-				INCLUDE="$1"
-				[ "$#" -gt 0 ] && shift ;;
+				INCLUDE="$1" ;;
 			--libbpf_dir)
 				shift
-				LIBBPF_DIR="$1"
-				[ "$#" -gt 0 ] && shift ;;
+				LIBBPF_DIR="$1" ;;
 			--libbpf_force)
-				if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then
-					usage 1
-				fi
-				LIBBPF_FORCE=$2
-				shift 2 ;;
+				shift
+				LIBBPF_FORCE="$1" ;;
 			-h | --help)
 				usage 0 ;;
 			--*)
-				shift ;;
-			"")
-				break ;;
+				;;
 			*)
 				usage 1 ;;
 		esac
+		[ "$#" -gt 0 ] && shift
 	done
 fi
 
@@ -531,6 +525,11 @@ fi
 if [ "${LIBBPF_DIR-unused}" != "unused" ]; then
 	[ -d "$LIBBPF_DIR" ] || usage 1
 fi
+if [ "${LIBBPF_FORCE-unused}" != "unused" ]; then
+	if [ "$LIBBPF_FORCE" != 'on' ] && [ "$LIBBPF_FORCE" != 'off' ]; then
+		usage 1
+	fi
+fi
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG
Signature · 1 line
2.31.1
#5 lore raw

PATCH v5 5/7 configure: support --param=value style

In reply to Andrea Claudi

This commit makes it possible to specify values for configure params using the common autotools configure syntax '--param=value'.

---
 configure | 6 ++++++
 1 file changed, 6 insertions(+)
configure +6 -0
index 26e06eb8..9a2645d9 100755
@@ -504,12 +504,18 @@ else
 			--include_dir)
 				shift
 				INCLUDE="$1" ;;
+			--include_dir=*)
+				INCLUDE="${1#*=}" ;;
 			--libbpf_dir)
 				shift
 				LIBBPF_DIR="$1" ;;
+			--libbpf_dir=*)
+				LIBBPF_DIR="${1#*=}" ;;
 			--libbpf_force)
 				shift
 				LIBBPF_FORCE="$1" ;;
+			--libbpf_force=*)
+				LIBBPF_FORCE="${1#*=}" ;;
 			-h | --help)
 				usage 0 ;;
 			--*)
Signature · 1 line
2.31.1
#6 lore raw

PATCH v5 6/7 configure: add the --prefix option

In reply to Andrea Claudi

This commit add the '--prefix' option to the iproute2 configure script.

This mimics the '--prefix' option that autotools configure provides, and will be used later to allow users or packagers to set the lib directory.

---
 configure | 8 ++++++++
 1 file changed, 8 insertions(+)
configure +8 -0
index 9a2645d9..05e23eff 100755
@@ -3,6 +3,7 @@
 # This is not an autoconf generated configure
 
 INCLUDE="$PWD/include"
+PREFIX="/usr"
 
 # Output file which is input to Makefile
 CONFIG=config.mk
@@ -490,6 +491,7 @@ Usage: $0 [OPTIONS]
 	--libbpf_force <on|off>		Enable/disable libbpf by force. Available options:
 					  on: require link against libbpf, quit config if no libbpf support
 					  off: disable libbpf probing
+	--prefix <dir>			Path prefix of the lib files to install
 	-h | --help			Show this usage info
 EOF
 	exit $1
@@ -516,6 +518,11 @@ else
 				LIBBPF_FORCE="$1" ;;
 			--libbpf_force=*)
 				LIBBPF_FORCE="${1#*=}" ;;
+			--prefix)
+				shift
+				PREFIX="$1" ;;
+			--prefix=*)
+				PREFIX="${1#*=}" ;;
 			-h | --help)
 				usage 0 ;;
 			--*)
@@ -536,6 +543,7 @@ if [ "${LIBBPF_FORCE-unused}" != "unused" ]; then
 		usage 1
 	fi
 fi
+[ -z "$PREFIX" ] && usage 1
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG
Signature · 1 line
2.31.1
#7 lore raw

PATCH v5 7/7 configure: add the --libdir option

In reply to Andrea Claudi

This commit allows users/packagers to choose a lib directory to store iproute2 lib files.

At the moment iproute2 ship lib files in /usr/lib and offers no way to modify this setting. However, according to the FHS, distros may choose "one or more variants of the /lib directory on systems which support more than one binary format" (e.g. /usr/lib64 on Fedora).

As Luca states in commit a3272b93725a ("configure: restore backward compatibility"), packaging systems may assume that 'configure' is from autotools, and try to pass it some parameters.

Allowing the '--libdir=/path/to/libdir' syntax, we can use this to our advantage, and let the lib directory to be chosen by the distro packaging system.

Note that LIBDIR uses "\${prefix}/lib" as default value because autoconf allows this to be expanded to the --prefix value at configure runtime. "\${prefix}" is replaced with the PREFIX value in check_lib_dir().

---
 Makefile  |  7 ++++---
 configure | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)
Makefile +4 -3
index 5eddd504..f6214534 100644
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Top level Makefile for iproute2
 
+-include config.mk
+
 ifeq ("$(origin V)", "command line")
 VERBOSE = $(V)
 endif
@@ -13,7 +15,6 @@ MAKEFLAGS += --no-print-directory
 endif
 
 PREFIX?=/usr
-LIBDIR?=$(PREFIX)/lib
 SBINDIR?=/sbin
 CONFDIR?=/etc/iproute2
 NETNS_RUN_DIR?=/var/run/netns
@@ -69,7 +70,7 @@ SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma dcb man vdpa
 LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
 LDLIBS += $(LIBNETLINK)
 
-all: config
+all: config.mk
 	@set -e; \
 	for i in $(SUBDIRS); \
 	do echo; echo $$i; $(MAKE) -C $$i; done
@@ -89,7 +90,7 @@ help:
 	@echo "Make Arguments:"
 	@echo " V=[0|1]             - set build verbosity level"
 
-config:
+config.mk:
 	@if [ ! -f config.mk -o configure -nt config.mk ]; then \
 		sh configure $(KERNEL_INCLUDE); \
 	fi
configure +18 -0
index 05e23eff..8ddff43c 100755
@@ -4,6 +4,7 @@
 
 INCLUDE="$PWD/include"
 PREFIX="/usr"
+LIBDIR="\${prefix}/lib"
 
 # Output file which is input to Makefile
 CONFIG=config.mk
@@ -149,6 +150,15 @@ EOF
 	rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
 
+check_lib_dir()
+{
+	LIBDIR=$(echo $LIBDIR | sed "s|\${prefix}|$PREFIX|")
+
+	echo -n "lib directory: "
+	echo "$LIBDIR"
+	echo "LIBDIR:=$LIBDIR" >> $CONFIG
+}
+
 check_ipt()
 {
 	if ! grep TC_CONFIG_XT $CONFIG > /dev/null; then
@@ -487,6 +497,7 @@ usage()
 	cat <<EOF
 Usage: $0 [OPTIONS]
 	--include_dir <dir>		Path to iproute2 include dir
+	--libdir <dir>			Path to iproute2 lib dir
 	--libbpf_dir <dir>		Path to libbpf DESTDIR
 	--libbpf_force <on|off>		Enable/disable libbpf by force. Available options:
 					  on: require link against libbpf, quit config if no libbpf support
@@ -508,6 +519,11 @@ else
 				INCLUDE="$1" ;;
 			--include_dir=*)
 				INCLUDE="${1#*=}" ;;
+			--libdir)
+				shift
+				LIBDIR="$1" ;;
+			--libdir=*)
+				LIBDIR="${1#*=}" ;;
 			--libbpf_dir)
 				shift
 				LIBBPF_DIR="$1" ;;
@@ -544,6 +560,7 @@ if [ "${LIBBPF_FORCE-unused}" != "unused" ]; then
 	fi
 fi
 [ -z "$PREFIX" ] && usage 1
+[ -z "$LIBDIR" ] && usage 1
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG
@@ -568,6 +585,7 @@ if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
 fi
 
 echo
+check_lib_dir
 if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
 	echo -n "iptables modules directory: "
 	check_ipt_lib_dir
Signature · 1 line
2.31.1
#8 lore raw

Re: [PATCH iproute2 v5 7/7] configure: add the --libdir option

Hi Andrea,

On Thu, Oct 14, 2021 at 10:50:55AM +0200, Andrea Claudi wrote:
[...]

Makefile line 5

Quoted 10 lines
Makefile +2 -0
index 5eddd504..f6214534 100644
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Top level Makefile for iproute2
 
+-include config.mk
+

Assuming config.mk may be missing (as dash-prefix is used).

Makefile MAKEFLAGS += --no-print-directory · line 16

Quoted 8 lines
Makefile +0 -1
 ifeq ("$(origin V)", "command line")
 VERBOSE = $(V)
 endif
@@ -13,7 +15,6 @@ MAKEFLAGS += --no-print-directory
 endif
 
 PREFIX?=/usr
-LIBDIR?=$(PREFIX)/lib

Dropping this leads to trouble if config.mk is missing or didn't define it. Can't you just leave it in place? Usually config.mk would override it anyway, no?

Rest of the series looks great, thanks a lot for spending the extra cycles giving it the mirror finish it has now. :)

Cheers, Phil

#9 lore raw

Re: [PATCH iproute2 v5 7/7] configure: add the --libdir option

Quoted 23 lines · Phil Sutter

Hi Andrea,

On Thu, Oct 14, 2021 at 10:50:55AM +0200, Andrea Claudi wrote:
[...]

Makefile line 5

Quoted 10 lines
Makefile +2 -0
index 5eddd504..f6214534 100644
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Top level Makefile for iproute2
 
+-include config.mk
+

Assuming config.mk may be missing (as dash-prefix is used).

Quoted 6 lines
Makefile +0 -0
 ifeq ("$(origin V)", "command line")
 VERBOSE = $(V)
 endif
@@ -13,7 +15,6 @@ MAKEFLAGS += --no-print-directory
 endif
 
Quoted 6 lines

Makefile MAKEFLAGS += --no-print-directory · line 16

Quoted 2 lines
Makefile +0 -1
 PREFIX?=/usr
-LIBDIR?=$(PREFIX)/lib

Dropping this leads to trouble if config.mk is missing or didn't define it. Can't you just leave it in place? Usually config.mk would override it anyway, no?

config.mk may miss at the first make call, but the "all" target calls config.mk, which in turns re-generate it. Thus LIBDIR is defined when the target all executes.

Also, LIBDIR must be defined in config.mk, as a default value for it is provided in configure, and will be used if the user does not provide it at command line.

I verified this deleting config.mk and printing DEFINES in Makefile to verify it includes the correct path for LIBDIR.

Acked-by
#10 lore raw

Re: [PATCH iproute2 v5 7/7] configure: add the --libdir option

Quoted 31 lines · Andrea Claudi
Quoted 23 lines · Phil Sutter

Hi Andrea,

On Thu, Oct 14, 2021 at 10:50:55AM +0200, Andrea Claudi wrote:
[...]

Makefile line 5

Quoted 10 lines
Makefile +2 -0
index 5eddd504..f6214534 100644
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Top level Makefile for iproute2
 
+-include config.mk
+

Assuming config.mk may be missing (as dash-prefix is used).

Quoted 6 lines
Makefile +0 -0
 ifeq ("$(origin V)", "command line")
 VERBOSE = $(V)
 endif
@@ -13,7 +15,6 @@ MAKEFLAGS += --no-print-directory
 endif
 
Quoted 6 lines

Makefile MAKEFLAGS += --no-print-directory · line 16

Quoted 2 lines
Makefile +0 -1
 PREFIX?=/usr
-LIBDIR?=$(PREFIX)/lib

Dropping this leads to trouble if config.mk is missing or didn't define it. Can't you just leave it in place? Usually config.mk would override it anyway, no?

Quoted 3 lines

config.mk may miss at the first make call, but the "all" target calls config.mk, which in turns re-generate it. Thus LIBDIR is defined when the target all executes.

Ah, I forgot the call to configure from make. So full series:

Thanks, Phil

Applied
#11 lore raw
In reply to Andrea Claudi
Quoted 15 lines · Andrea Claudi

This series add support for the libdir parameter in iproute2 configure script. The idea is to make use of the fact that packaging systems may assume that 'configure' comes from autotools allowing a syntax similar to the autotools one, and using it to tell iproute2 where the distro expects to find its lib files.

Patches 1-2 fix a parsing issue on current configure options, that may trigger an endless loop when no value is provided with some options;

Patch 3 fixes a parsing issue bailing out when more than one value is provided for a single option;

Patch 4 simplifies options parsing, moving semantic checks out of the while loop processing options;

Quoted 9 lines

Patch 5 introduces support for the --opt=value style on current options, for uniformity;

Patch 6 adds the --prefix option, that may be used by some packaging systems when calling the configure script;

Patch 7 finally adds the --libdir option, and also drops the static LIBDIR var from the Makefile.

applied to net-next. Thanks for working on this.

#12 lore raw
Quoted 1 line · David Ahern

applied to net-next. Thanks for working on this.

sigh. iproute2-next