2014-11-23 15:09:24

by Yann E. MORIN

[permalink] [raw]
Subject: [PATCH nfs-utils] configure: use pkg-config to find libtirpc

Currently, we use a custom function to find libtirpc's headers and
libraries. This works fine for shared linking.

But for static linking, this forgets to link with -lpthread, which is
required by libtirpc.

A recent patch was sent to libtirpc to add that missing -lpthread in its
Libs.private section of its .pc file. Thus, pkg-config will soon be able
to return the appropriate libraries.

So, use pkg-config to find libtirpc.

And for older libtirpc versions, there is no change in behaviour: we're
still missing the -lpthread. But once libtirpc has been fixed, we'll
automatically get that missing library for free! :-)

Remove the --with-libirpc flag as it is no longer needed: pkg-config
will provide us with the -I and -L flags, now.

Signed-off-by: "Yann E. MORIN" <[email protected]>
Cc: Steve Dickson <[email protected]>

---
Steve,

This is related to the libtirpc patch I sent just mere hours ago.

I was pretty amused to see, when looking who to send this patch to, that you
were taking care of applying patches for both libtirpc and nfs-utils. :-)

At least, now you've seen both patches, all should make sense! ;-)

Regards,
Yann E. MORIN.

---
aclocal/libtirpc.m4 | 64 ++++++++---------------------------------------------
1 file changed, 9 insertions(+), 55 deletions(-)

diff --git a/aclocal/libtirpc.m4 b/aclocal/libtirpc.m4
index b823364..dc74638 100644
--- a/aclocal/libtirpc.m4
+++ b/aclocal/libtirpc.m4
@@ -2,61 +2,15 @@ dnl Checks for TI-RPC library and headers
dnl
AC_DEFUN([AC_LIBTIRPC], [

- AC_ARG_WITH([tirpcinclude],
- [AC_HELP_STRING([--with-tirpcinclude=DIR],
- [use TI-RPC headers in DIR])],
- [tirpc_header_dir=$withval],
- [tirpc_header_dir=/usr/include/tirpc])
-
- dnl if --enable-tirpc was specifed, the following components
- dnl must be present, and we set up HAVE_ macros for them.
-
- if test "$enable_tirpc" != "no"; then
-
- dnl look for the library
- AC_CHECK_LIB([tirpc], [clnt_tli_create], [:],
- [if test "$enable_tirpc" = "yes"; then
- AC_MSG_ERROR([libtirpc not found.])
- else
- AC_MSG_WARN([libtirpc not found. TIRPC disabled!])
- enable_tirpc="no"
- fi])
- fi
-
- if test "$enable_tirpc" != "no"; then
-
- dnl Check if library contains authgss_free_private_data
- AC_CHECK_LIB([tirpc], [authgss_free_private_data], [have_free_private_data=yes],
- [have_free_private_data=no])
- fi
-
- if test "$enable_tirpc" != "no"; then
- dnl also must have the headers installed where we expect
- dnl look for headers; add -I compiler option if found
- AC_CHECK_HEADERS([${tirpc_header_dir}/netconfig.h],
- AC_SUBST([AM_CPPFLAGS], ["-I${tirpc_header_dir}"]),
- [if test "$enable_tirpc" = "yes"; then
- AC_MSG_ERROR([libtirpc headers not found.])
- else
- AC_MSG_WARN([libtirpc headers not found. TIRPC disabled!])
- enable_tirpc="no"
- fi])
-
- fi
-
- dnl now set $LIBTIRPC accordingly
- if test "$enable_tirpc" != "no"; then
- AC_DEFINE([HAVE_LIBTIRPC], 1,
- [Define to 1 if you have and wish to use libtirpc.])
- LIBTIRPC="-ltirpc"
- if test "$have_free_private_data" = "yes"; then
- AC_DEFINE([HAVE_AUTHGSS_FREE_PRIVATE_DATA], 1,
- [Define to 1 if your rpcsec library provides authgss_free_private_data,])
- fi
- else
- LIBTIRPC=""
- fi
-
+ PKG_CHECK_MODULES([TIRPC], [libtirpc >= 0.2.4],
+ [LIBTIRPC="${TIRPC_LIBS}"
+ AM_CPPFLAGS="${AM_CPPFLAGS} ${TIRPC_CFLAGS}"
+ AC_DEFINE([HAVE_LIBTIRPC], [1],
+ [Define to 1 if you have and wish to use libtirpc.])],
+ [AS_IF([test "$enable_tirpc" != "no"], [AC_MSG_ERROR([libtirpc not found.])],
+ [LIBTIRPC=""])])
+
+ AC_SUBST([AM_CPPFLAGS])
AC_SUBST(LIBTIRPC)

])dnl
--
1.9.1



2015-01-05 13:14:52

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils] configure: use pkg-config to find libtirpc

Hello,

On 11/23/2014 10:09 AM, Yann E. MORIN wrote:
> Currently, we use a custom function to find libtirpc's headers and
> libraries. This works fine for shared linking.
>
> But for static linking, this forgets to link with -lpthread, which is
> required by libtirpc.
>
> A recent patch was sent to libtirpc to add that missing -lpthread in its
> Libs.private section of its .pc file. Thus, pkg-config will soon be able
> to return the appropriate libraries.
>
> So, use pkg-config to find libtirpc.
>
> And for older libtirpc versions, there is no change in behaviour: we're
> still missing the -lpthread. But once libtirpc has been fixed, we'll
> automatically get that missing library for free! :-)
>
> Remove the --with-libirpc flag as it is no longer needed: pkg-config
> will provide us with the -I and -L flags, now.
It turns out that using pkg-config break builds on legacy OSs which
is something I don't want to do. I think it's important to at least
try to maintain legacy builds so I'm looking to revert this patch.

So if I revert this patch, what will break in your world?

steved.

2015-01-05 18:29:36

by Yann E. MORIN

[permalink] [raw]
Subject: Re: [PATCH nfs-utils] configure: use pkg-config to find libtirpc

Steve, All,

On 2015-01-05 08:14 -0500, Steve Dickson spake thusly:
> On 11/23/2014 10:09 AM, Yann E. MORIN wrote:
> > Currently, we use a custom function to find libtirpc's headers and
> > libraries. This works fine for shared linking.
> >
> > But for static linking, this forgets to link with -lpthread, which is
> > required by libtirpc.
> >
> > A recent patch was sent to libtirpc to add that missing -lpthread in its
> > Libs.private section of its .pc file. Thus, pkg-config will soon be able
> > to return the appropriate libraries.
> >
> > So, use pkg-config to find libtirpc.
> >
> > And for older libtirpc versions, there is no change in behaviour: we're
> > still missing the -lpthread. But once libtirpc has been fixed, we'll
> > automatically get that missing library for free! :-)
> >
> > Remove the --with-libirpc flag as it is no longer needed: pkg-config
> > will provide us with the -I and -L flags, now.
> It turns out that using pkg-config break builds on legacy OSs which
> is something I don't want to do. I think it's important to at least
> try to maintain legacy builds so I'm looking to revert this patch.

Sorry to read that it breaks on some systems... :-(

> So if I revert this patch, what will break in your world?

What breaks is when libtirpc is available only as a static library.

libtirpc uses pthreads, but NFS-utils does not link to -lpthread, and so
there are undefined symbols at link time. Using pkg-config ensures that
we get the proper LDFLAGS for libtirpc.

What are those "legacy OSs"? Can I help in finding an adequate solution?
(note: any Linux distro I can install without problem, I could try to
give a spin to a *BSD one if really needed, but other "OSes" not so
much...)

If at all possible, I would like to keep pkg-config as the default, and
use the old (or another) method only as a fallback, because pkg-config
is so much nicer to deal with than trying to cope for all cases
ourselves. Would that be an acceptable solution for you?

Regards,
Yann E. MORIN.

PS. I'm only available in the evening UTC+1, so please bear with the
delay in my replies...
YEM.

--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'

2015-01-06 18:45:42

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils] configure: use pkg-config to find libtirpc

On 01/05/2015 01:23 PM, Yann E. MORIN wrote:
>
> What are those "legacy OSs"? Can I help in finding an adequate solution?
> (note: any Linux distro I can install without problem, I could try to
> give a spin to a *BSD one if really needed, but other "OSes" not so
> much...)
The problem is here...
http://www.spinics.net/lists/linux-nfs/msg48570.html

>
> If at all possible, I would like to keep pkg-config as the default, and
> use the old (or another) method only as a fallback, because pkg-config
> is so much nicer to deal with than trying to cope for all cases
> ourselves. Would that be an acceptable solution for you?
Yes.. I would be more than willing to work out something that works in
both cases....

steved.


2015-01-06 19:05:32

by Yann E. MORIN

[permalink] [raw]
Subject: Re: [PATCH nfs-utils] configure: use pkg-config to find libtirpc

Steve, All,

On 2015-01-06 13:45 -0500, Steve Dickson spake thusly:
> On 01/05/2015 01:23 PM, Yann E. MORIN wrote:
> >
> > What are those "legacy OSs"? Can I help in finding an adequate solution?
> > (note: any Linux distro I can install without problem, I could try to
> > give a spin to a *BSD one if really needed, but other "OSes" not so
> > much...)
> The problem is here...
> http://www.spinics.net/lists/linux-nfs/msg48570.html

Ah, OK! That's because the check I implemented requires libtirpc 0.2.4.
I choose to require that version because it was the one I tested
against.

But if older versions are allowed, or even any other, it is simple to
change the required version string:

-PKG_CHECK_MODULES([TIRPC], [libtirpc >= 0.2.4],
+PKG_CHECK_MODULES([TIRPC], [libtirpc >= 0.2.1],

Or for accepting any version:

-PKG_CHECK_MODULES([TIRPC], [libtirpc >= 0.2.4],
+PKG_CHECK_MODULES([TIRPC], [libtirpc],

> > If at all possible, I would like to keep pkg-config as the default, and
> > use the old (or another) method only as a fallback, because pkg-config
> > is so much nicer to deal with than trying to cope for all cases
> > ourselves. Would that be an acceptable solution for you?
> Yes.. I would be more than willing to work out something that works in
> both cases....

Yes, I checked the old code, and it does not require any version for
libtirpc, just that it exists.

The old code also checks for authgss_free_private_data, that was
introduced in libtirpc between 0.2.2 and 0.2.3, and the change I
proposed did not handle that. It completely slipped by.

So, to propose a fix, I need to know what the oldest libtirpc version
should be supported. From the mesage pointed to above, it seems we
should support at least back to libtirpc 0.2.1. Do we have to support
yet older versions, or even any version?

Second, I'll also work on providing a way to restore detection and use
of authgss_free_private_data.

Note: I'll hopefuly have time to work on this tonight, and send a fix
before midnight UTC.

Sorry for the mishap... :-(

Regards,
Yann E. MORIN.

--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'