configure: add support for libdir and prefix option
Subsystems (3)
v3 cover configure: add support for libdir and prefix option
This series add support for the libdir parameter in iproute2 configure system. 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.
Patch 1 introduces support for the --param=value style on current params, for uniformity.
Patch 2 add the --prefix option, that may be used by some packaging systems when calling the configure script.
Patch 3 add the --libdir option to the configure script, and also drops the static LIBDIR var from the Makefile.
Changelog:
----------
v2 -> v3
- Fix parsing error on prefix and libdir options.
v1 -> v2
- consolidate '--param value' and '--param=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 (3):
configure: support --param=value style
configure: add the --prefix option
configure: add the --libdir option
Makefile | 7 +++--- configure | 72 +++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 66 insertions(+), 13 deletions(-)
Signature · 1 line
2.31.1
PATCH v3 1/3 configure: support --param=value style
This commit makes it possible to specify values for configure params using the common autotools configure syntax '--param=value'.
--- configure | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-)
configure +22 -10
@@ -501,18 +501,30 @@ if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then else while true; do case "$1" in - --include_dir) - INCLUDE=$2 - shift 2 ;; - --libbpf_dir) - LIBBPF_DIR="$2" - shift 2 ;; - --libbpf_force) - if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then + --include_dir | --include_dir=*) + INCLUDE="${1#*=}" + if [ "$INCLUDE" == "--include_dir" ]; then + INCLUDE=$2 + shift + fi + shift ;; + --libbpf_dir | --libbpf_dir=*) + LIBBPF_DIR="${1#*=}" + if [ "$LIBBPF_DIR" == "--libbpf_dir" ]; then + LIBBPF_DIR="$2" + shift + fi + shift ;; + --libbpf_force | --libbpf_force=*) + LIBBPF_FORCE="${1#*=}" + if [ "$LIBBPF_FORCE" == "--libbpf_force" ]; then + LIBBPF_FORCE="$2" + shift + fi + if [ "$LIBBPF_FORCE" != 'on' ] && [ "$LIBBPF_FORCE" != 'off' ]; then usage 1 fi - LIBBPF_FORCE=$2 - shift 2 ;; + shift ;; -h | --help) usage 0 ;; "")
Signature · 1 line
2.31.1
Re: [PATCH iproute2 v3 1/3] configure: support --param=value style
Hi Andrea,
A remark regarding coding style:
On Wed, Oct 06, 2021 at 12:08:04AM +0200, Andrea Claudi wrote:
[...]
Quoted 7 lines
configure +0
-0@@ -501,18 +501,30 @@ if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then else while true; do
configure if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then · line 504
Quoted 10 lines
configure +1
-8case "$1" in - --include_dir) - INCLUDE=$2 - shift 2 ;; - --libbpf_dir) - LIBBPF_DIR="$2" - shift 2 ;; - --libbpf_force) - if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then + --include_dir | --include_dir=*)
So here the code combines the two cases,
configure if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then · line 506
Quoted 2 lines
configure +2
-0+ INCLUDE="${1#*=}" + if [ "$INCLUDE" == "--include_dir" ]; then
just to fiddle it apart again. Did you consider leaving the old cases in place and adding separate ones for the --opt=val cases like so:
| --include_dir=*)
| INCLUDE="${1#*=}"
| shift
| ;;[...]
configure if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then · line 524
Quoted 7 lines
configure +7
-0+ --libbpf_force | --libbpf_force=*) + LIBBPF_FORCE="${1#*=}" + if [ "$LIBBPF_FORCE" == "--libbpf_force" ]; then + LIBBPF_FORCE="$2" + shift + fi + if [ "$LIBBPF_FORCE" != 'on' ] && [ "$LIBBPF_FORCE" != 'off' ]; then
To avoid duplication here, I would move semantic checks into a second step. This would allow for things like:
| --libbpf_force=invalid --libbpf_force=on
but separating the syntactic parsing from semantic checks might be beneficial by itself, too.
Cheers, Phil
Re: [PATCH iproute2 v3 1/3] configure: support --param=value style
Quoted 4 lines · Phil Sutter
Hi Andrea,
A remark regarding coding style:
Hi Phil,
Thanks for your review.
Quoted 28 lines · Andrea Claudi
[...]
Quoted 7 lines
configure +0
-0@@ -501,18 +501,30 @@ if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then else while true; doconfigure if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then · line 504
Quoted 10 lines
configure +1
-8case "$1" in - --include_dir) - INCLUDE=$2 - shift 2 ;; - --libbpf_dir) - LIBBPF_DIR="$2" - shift 2 ;; - --libbpf_force) - if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then + --include_dir | --include_dir=*)So here the code combines the two cases,
configure if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then · line 506
Quoted 2 lines
configure +2
-0+ INCLUDE="${1#*=}" + if [ "$INCLUDE" == "--include_dir" ]; thenjust to fiddle it apart again. Did you consider leaving the old cases in place and adding separate ones for the --opt=val cases like so:
Quoted 6 lines
| --include_dir=*) | INCLUDE="${1#*=}" | shift | ;;[...]
That was my first proposal in v1 [1]. I changed it on David's suggestion to consolidate the two cases into a single one.
Looking at the resulting code, v3 code results in an extra check to discriminate between the two use cases, while v0 uses the "case" structure to the same end.
Quoted 11 lines
configure if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then · line 524
Quoted 7 lines
configure +7
-0+ --libbpf_force | --libbpf_force=*) + LIBBPF_FORCE="${1#*=}" + if [ "$LIBBPF_FORCE" == "--libbpf_force" ]; then + LIBBPF_FORCE="$2" + shift + fi + if [ "$LIBBPF_FORCE" != 'on' ] && [ "$LIBBPF_FORCE" != 'off' ]; thenTo avoid duplication here, I would move semantic checks into a second step. This would allow for things like:
Quoted 4 lines
| --libbpf_force=invalid --libbpf_force=on
but separating the syntactic parsing from semantic checks might be beneficial by itself, too.
Yes, I agree with you. David, does this answer to your concern about v1? If yes, I would proceed with a v4 integrating Phil's suggestions.
Quoted 3 lines
Cheers, Phil
Re: [PATCH iproute2 v3 1/3] configure: support --param=value style
Hi Andrea,
On Wed, Oct 06, 2021 at 11:49:34AM +0200, Andrea Claudi wrote:
[...]
Quoted 2 lines
That was my first proposal in v1 [1]. I changed it on David's suggestion to consolidate the two cases into a single one.
Oh, sorry. I missed the 'v3' tag and hence didn't check any earlier version.
Quoted 3 lines
Looking at the resulting code, v3 code results in an extra check to discriminate between the two use cases, while v0 uses the "case" structure to the same end.
Given that David explicitly requested the change I'm complaining about in his reply to your initial version, I guess any further debate about it is irrelevant.
Thanks, Phil
Re: [PATCH iproute2 v3 1/3] configure: support --param=value style
Quoted 10 lines · Phil Sutter
Hi Andrea,
On Wed, Oct 06, 2021 at 11:49:34AM +0200, Andrea Claudi wrote:
[...]Quoted 2 lines
That was my first proposal in v1 [1]. I changed it on David's suggestion to consolidate the two cases into a single one.
Oh, sorry. I missed the 'v3' tag and hence didn't check any earlier version.
Quoted 8 lines
Quoted 3 lines
Looking at the resulting code, v3 code results in an extra check to discriminate between the two use cases, while v0 uses the "case" structure to the same end.
Given that David explicitly requested the change I'm complaining about in his reply to your initial version, I guess any further debate about it is irrelevant.
I did not like the exploding duplication of checks on the value just to support '=' in the command line.
Phil's suggestion seems reasonable.
PATCH v3 2/3 configure: add the --prefix option
In reply to Andrea ClaudiThis 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 | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
configure +18 -0
@@ -148,6 +148,15 @@ EOF rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest } +check_prefix() +{ + if [ -n "$PREFIX" ]; then + prefix="$PREFIX" + else + prefix="/usr" + fi +} + check_ipt() { if ! grep TC_CONFIG_XT $CONFIG > /dev/null; then
@@ -490,6 +499,7 @@ Usage: $0 [OPTIONS] --libbpf_force Enable/disable libbpf by force. Available options: on: require link against libbpf, quit config if no libbpf support off: disable libbpf probing + --prefix Path prefix of the lib files to install -h | --help Show this usage info EOF exit $1
@@ -525,6 +535,13 @@ else usage 1 fi shift ;; + --prefix | --prefix=*) + PREFIX="${1#*=}" + if [ "$PREFIX" == "--prefix" ]; then + PREFIX="$2" + shift + fi + shift ;; -h | --help) usage 0 ;; "")
@@ -558,6 +575,7 @@ if ! grep -q TC_CONFIG_NO_XT $CONFIG; then fi echo +check_prefix if ! grep -q TC_CONFIG_NO_XT $CONFIG; then echo -n "iptables modules directory: " check_ipt_lib_dir
Signature · 1 line
2.31.1
PATCH v3 3/3 configure: add the --libdir option
In reply to Andrea ClaudiThis 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.
--- Makefile | 7 ++++--- configure | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-)
Makefile +4 -3
@@ -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
@@ -60,7 +61,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
@@ -80,7 +81,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 +22 -0
@@ -157,6 +157,19 @@ check_prefix() fi } +check_lib_dir() +{ + echo -n "lib directory: " + if [ -n "$LIBDIR" ]; then + eval echo "$LIBDIR" + eval echo "LIBDIR:=$LIBDIR" >> $CONFIG + return + fi + + eval echo "${prefix}/lib" + eval echo "LIBDIR:=${prefix}/lib" >> $CONFIG +} + check_ipt() { if ! grep TC_CONFIG_XT $CONFIG > /dev/null; then
@@ -495,6 +508,7 @@ usage() cat <<EOF Usage: $0 [OPTIONS] --include_dir Path to iproute2 include dir + --libdir Path to iproute2 lib 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
@@ -518,6 +532,13 @@ else shift fi shift ;; + --libdir | --libdir=*) + LIBDIR="${1#*=}" + if [ "$LIBDIR" == "--libdir" ]; then + LIBDIR="$2" + shift + fi + shift ;; --libbpf_dir | --libbpf_dir=*) LIBBPF_DIR="${1#*=}" if [ "$LIBBPF_DIR" == "--libbpf_dir" ]; then
@@ -576,6 +597,7 @@ fi echo check_prefix +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