Skip to content

Use pkg-config to discover libnl-3 #1637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+12 −7
Open

Use pkg-config to discover libnl-3 #1637

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
966d515
Use pkg-config to find libnl-3 headers
fpletz Mar 12, 2025
40dcb84
Dynamically load libnl-3 libs from pkg-config libdir
fpletz Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,14 +961,16 @@ case "$enable_delayacct" in
enable_delayacct=no
else
old_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS -I/usr/include/libnl3"
htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null` || continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish some quality improvements to the check:

  1. There's no error handling here. What would you do if the pkg-config call fails?
  2. Is there any reason not to use PKG_CHECK_MODULES macro as in the old code?
  3. You should allow the user to override the compiler flags detected. If you have used PKG_CHECK_MODULES, then the variable LIBNL3_CFLAGS would be defined automatically to allow that user override. Otherwise, maybe you can define it yourself? (AC_ARG_VAR([LIBNL3_CFLAGS], [C compiler flags for libnl3, overriding pkg-config]))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no error handling here. What would you do if the pkg-config call fails?

In this case the AC_CHECK_HEADERS fails like it would have done before since htop_config_cflags is empty and the headers aren't found. But PKG_CHECK_MODULES should take care of that.

Is there any reason not to use PKG_CHECK_MODULES macro as in the old code?

I used the current style since that is used to discover ncurses in the same file. I agree that PKG_CHECK_MODULES is the better solution and will change the code accordingly.

You should allow the user to override the compiler flags detected. If you have used PKG_CHECK_MODULES, then the variable LIBNL3_CFLAGS would be defined automatically to allow that user override. Otherwise, maybe you can define it yourself? (AC_ARG_VAR([LIBNL3_CFLAGS], [C compiler flags for libnl3, overriding pkg-config]))

Yes, makes sense. I will add this. And also introduce the variable LIBNL3_LIBDIR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason the ncurses detection code doesn't use PKG_CHECK_MODULES is that ncurses may come with many different library names (as well as .pc file names), so the code has to implement fallbacks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICS, this should be || true, as there is no loop.

Suggested change
htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null` || continue
htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null` || true

CFLAGS="$CFLAGS $htop_config_cflags"
AC_CHECK_HEADERS([netlink/attr.h netlink/handlers.h netlink/msg.h], [enable_delayacct=yes], [enable_delayacct=no])
CFLAGS="$old_CFLAGS"
fi
;;
yes)
old_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS -I/usr/include/libnl3"
htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null` || continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here …

Suggested change
htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null` || continue
htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null` || true

CFLAGS="$CFLAGS $htop_config_cflags"
AC_CHECK_HEADERS([netlink/attr.h netlink/handlers.h netlink/msg.h], [], [AC_MSG_ERROR([can not find required header files netlink/attr.h, netlink/handlers.h, netlink/msg.h])])
CFLAGS="$old_CFLAGS"
;;
Expand All @@ -978,7 +980,8 @@ case "$enable_delayacct" in
esac
if test "$enable_delayacct" = yes; then
AC_DEFINE([HAVE_DELAYACCT], [1], [Define if delay accounting support should be enabled.])
AM_CFLAGS="$AM_CFLAGS -I/usr/include/libnl3"
htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null`
AM_CFLAGS="$AM_CFLAGS $htop_config_cflags"
fi
AM_CONDITIONAL([HAVE_DELAYACCT], [test "$enable_delayacct" = yes])

Expand Down