Use pkg-config to discover libnl-3 #1637

fpletz · 2025-03-13T07:53:00Z

Since #1427, the location of libnl-3 is not discovered anymore with pkg-config. Instead, the hardcoded path /usr/include/libnl3 was introduced for headers without any possibility to override it except by patching.

This PR adds support for pkg-config to discover the location of the headers. Additionally, it discovers the path to libnl-3 libraries and modifies the dlopen calls to load the configured libraries from their fully qualified path.

Also note the discussion on the pull request.

Explorer09 · 2025-03-13T08:16:11Z

configure.ac

@@ -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


I wish some quality improvements to the check:

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

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

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]))

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.

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.

BenBE

Please squash/fixup your changes into the appropriate commits.

BenBE · 2025-03-13T08:37:36Z

configure.ac

@@ -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


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

BenBE · 2025-03-13T08:38:03Z

configure.ac

         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


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

BenBE · 2025-03-13T08:42:08Z

configure.ac

+  htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null`
+  AM_CFLAGS="$AM_CFLAGS $htop_config_cflags"
+  libdir_libnl=`$PKG_CONFIG --variable=libdir libnl-3.0 2>/dev/null`


Failure to read these should result in a proper error message generated, or a warning notifying of a fallback to /usr/include being used.

Explorer09 · 2025-03-13T09:13:51Z

configure.ac

@@ -982,6 +982,8 @@ if test "$enable_delayacct" = yes; then
  AC_DEFINE([HAVE_DELAYACCT], [1], [Define if delay accounting support should be enabled.])
  htop_config_cflags=`$PKG_CONFIG --cflags libnl-3.0 2>/dev/null`
  AM_CFLAGS="$AM_CFLAGS $htop_config_cflags"
+  libdir_libnl=`$PKG_CONFIG --variable=libdir libnl-3.0 2>/dev/null`
+  AC_DEFINE_UNQUOTED([LIBDIR_LIBNL], ["$libdir_libnl"], [Path to the libnl-3 libraries])


Um. A small question (maybe this is bike shedding).
Which name for the shell variable / C macro is better, LIBNL_LIBDIR or LIBDIR_LIBNL?

I would vote for LIBNL_LIBDIR.

Other places like for hwloc use FOO_LIBS, thus LIBNL_LIBDIR would be reasonable.

Explorer09 · 2025-04-25T13:35:07Z

linux/LibNl.c

@@ -78,17 +78,17 @@ static int load_libnl(void) {
   if (libnlHandle && libnlGenlHandle)
      return 0;

-   libnlHandle = dlopen("libnl-3.so", RTLD_LAZY);
+   libnlHandle = dlopen(LIBDIR_LIBNL "/libnl-3.so", RTLD_LAZY);


I noticed a slash is used here unconditionally. Normally for libraries it's better to load them from default search paths rather than forcing a directory prefix at compile time. I know the directory is grabbed from pkg-config with this patch, but what if no directory prefix is specified there?

fpletz added 2 commits March 12, 2025 13:18

Use pkg-config to find libnl-3 headers

966d515

Dynamically load libnl-3 libs from pkg-config libdir

40dcb84

fpletz mentioned this pull request Mar 13, 2025

htop: 3.3.0 -> 3.4.0 NixOS/nixpkgs#389164

Merged

13 tasks

BenBE added enhancement build system 🔧 Linux 🐧 labels Mar 13, 2025

BenBE added this to the 3.4.1 milestone Mar 13, 2025

Explorer09 reviewed Mar 13, 2025

View reviewed changes

BenBE requested changes Mar 13, 2025

View reviewed changes

Explorer09 reviewed Mar 13, 2025

View reviewed changes

thesamesam mentioned this pull request Mar 14, 2025

Regression in libnl3 detection (no longer uses pkg-config) #1639

Open

BenBE linked an issue Mar 15, 2025 that may be closed by this pull request

Regression in libnl3 detection (no longer uses pkg-config) #1639

Open

BenBE modified the milestones: 3.4.1, 3.5.0 Apr 10, 2025

henry-hsieh mentioned this pull request Apr 14, 2025

feat(htop): update htop version to v3.4.1 henry-hsieh/htop.appimage#19

Open

1 task

Explorer09 reviewed Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use pkg-config to discover libnl-3 #1637

Use pkg-config to discover libnl-3 #1637

fpletz commented Mar 13, 2025

Explorer09 Mar 13, 2025

fpletz Mar 13, 2025

Explorer09 Mar 13, 2025

BenBE left a comment

BenBE Mar 13, 2025

BenBE Mar 13, 2025

BenBE Mar 13, 2025

Explorer09 Mar 13, 2025

BenBE Mar 13, 2025

Explorer09 Apr 25, 2025

	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

Use pkg-config to discover libnl-3 #1637

Are you sure you want to change the base?

Use pkg-config to discover libnl-3 #1637

Conversation

fpletz commented Mar 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenBE left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment