Patchwork Do not look for libattr. <sys/xattr.h> is enough

login
register
mail settings
Submitter Cristian Rodríguez
Date June 23, 2014, 9:51 p.m.
Message ID <1403560273-109277-1-git-send-email-crrodriguez@opensuse.org>
Download mbox | patch
Permalink /patch/363203/
State Superseded
Headers show

Comments

Cristian Rodríguez - June 23, 2014, 9:51 p.m.
libattr never gets used in the final build as xattr functions
are provided by libc.
This commit avoids requiring an used library during package build.

Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
---
 configure.ac                       | 5 -----
 src/libjunction/display-junction.c | 2 +-
 src/libjunction/junction.c         | 2 +-
 3 files changed, 2 insertions(+), 7 deletions(-)
Chuck Lever - June 23, 2014, 10:16 p.m.
Hi Cristian-

On Jun 23, 2014, at 5:51 PM, Cristian Rodríguez <crrodriguez@opensuse.org> wrote:

> libattr never gets used in the final build as xattr functions
> are provided by libc.

Is this true for all versions of glibc? My copy of glibc 2.7 shows
its copy of the *xattr functions are just stubs, for example.

> This commit avoids requiring an used library during package build.

I assume you mean “an unused library” here.

Unfortunately the patch description for commit f5c16606beca0 is not
clear why I added this check a year ago. But I seem to recall that
one of the RH-based systems I worked on (EL6, perhaps) had an issue
when libattr and headers were missing in the mockbuild.

Fedora packaging has a BuildRequires libattr-devel. I’m not sure it
is universally safe to simply rip this out. Some testing is required,
and maybe something that has a more precise configure.ac test would
work better for both legacy and newer systems?

> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
> ---
> configure.ac                       | 5 -----
> src/libjunction/display-junction.c | 2 +-
> src/libjunction/junction.c         | 2 +-
> 3 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index a35c798..f7a63cf 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -141,11 +141,6 @@ AC_CHECK_LIB([ssl], [SSL_CTX_new],
> 		 AC_DEFINE([HAVE_LIBSSL], [1],
> 			   [Define if you have libssl])],
> 		[AC_MSG_ERROR([libssl not found.])])
> -AC_CHECK_LIB([attr], [fgetxattr],
> -		[AC_SUBST([LIBATTR], ["-lattr"])
> -		 AC_DEFINE([HAVE_LIBATTR], [1],
> -			   [Define if you have libattr])],
> -		[AC_MSG_ERROR([libattr not found.])])
> AC_CHECK_LIB([gssapi_krb5], [gss_acquire_cred],
> 		[AC_SUBST([LIBGSSAPI_KRB5], ["-lgssapi_krb5"])
> 		 AC_DEFINE([HAVE_LIBGSSAPI_KRB5], [1],
> diff --git a/src/libjunction/display-junction.c b/src/libjunction/display-junction.c
> index ac5797b..2e924da 100644
> --- a/src/libjunction/display-junction.c
> +++ b/src/libjunction/display-junction.c
> @@ -35,7 +35,7 @@
> #include <locale.h>
> #include <langinfo.h>
> 
> -#include <attr/xattr.h>
> +#include <sys/xattr.h>

My fgetxattr man page says <attr/xattr.h> is the correct header:

NAME
       getxattr, lgetxattr, fgetxattr - retrieve an extended attribute value

SYNOPSIS
       #include <sys/types.h>
       #include <attr/xattr.h>

       ssize_t getxattr (const char *path, const char *name,
                            void *value, size_t size);
       ssize_t lgetxattr (const char *path, const char *name,
                            void *value, size_t size);
       ssize_t fgetxattr (int filedes, const char *name,
                            void *value, size_t size);

But this is a Fedora 19 system. Is it different on SuSE? Maybe my
man page is out of date? Feel free to wield a clue bat.

> #include "junction.h"
> #include "junction-internal.h"
> diff --git a/src/libjunction/junction.c b/src/libjunction/junction.c
> index 7b7b2e4..71ccc7a 100644
> --- a/src/libjunction/junction.c
> +++ b/src/libjunction/junction.c
> @@ -38,7 +38,7 @@
> #include <errno.h>
> #include <dirent.h>
> 
> -#include <attr/xattr.h>
> +#include <sys/xattr.h>
> 
> #include "fedfs.h"
> #include "nsdb.h"
> -- 
> 2.0.0
> 

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
Cristian Rodríguez - June 23, 2014, 10:38 p.m.
El lun 23 jun 2014 18:16:14 CLT, Chuck Lever escribió:
> Hi Cristian-
>
> On Jun 23, 2014, at 5:51 PM, Cristian Rodríguez <crrodriguez@opensuse.org> wrote:
>
>> libattr never gets used in the final build as xattr functions
>> are provided by libc.
>
> Is this true for all versions of glibc? My copy of glibc 2.7 shows
> its copy of the *xattr functions are just stubs, for example.

huh ? glibc binary interface provides this functions as version 2.3..

>
>> This commit avoids requiring an used library during package build.
>
> I assume you mean “an unused library” here.

Yes.

>
> Unfortunately the patch description for commit f5c16606beca0 is not
> clear why I added this check a year ago. But I seem to recall that
> one of the RH-based systems I worked on (EL6, perhaps) had an issue
> when libattr and headers were missing in the mockbuild.
>
> Fedora packaging has a BuildRequires libattr-devel. I’m not sure it
> is universally safe to simply rip this out. Some testing is required,
> and maybe something that has a more precise configure.ac test would
> work better for both legacy and newer systems?

That does not mean anything..one rule on distribution packages you need 
to understand first is that buildrequires are added but rarely if ever 
removed
unless someone takes the time to clean-up the house from time to time.. 
or for some reason the build breaks and someone realizes it is 
superfluous, this is however seldom happens.

Of course the package build will have libattr-devel as a build 
dependency, since this code include <attr/xattr.h> and will fail 
without it.. however...the code is not even linking the attr library so 
it will never get used.

AC_CHECK_LIB([attr], [fgetxattr],
              [AC_SUBST([LIBATTR], ["-lattr"])
                AC_DEFINE([HAVE_LIBATTR], [1],
                          [Define if you have libattr])],
               [AC_MSG_ERROR([libattr not found.])])

Does not add -lattr to $LIBS and the variable LIBATTR is never 
referenced anywhere in the makefiles.. the build succeeds because 
libattr duplicates libc functionality....



--
Cristian
"I don't know the key to success, but the key to failure is trying to 
please everybody."
Chuck Lever - June 23, 2014, 10:54 p.m.
On Jun 23, 2014, at 6:38 PM, Cristian Rodríguez <crrodriguez@opensuse.org> wrote:

> Of course the package build will have libattr-devel as a build 
> dependency, since this code include <attr/xattr.h> and will fail 
> without it.. however...the code is not even linking the attr library so 
> it will never get used.
> 
> AC_CHECK_LIB([attr], [fgetxattr],
>              [AC_SUBST([LIBATTR], ["-lattr"])
>                AC_DEFINE([HAVE_LIBATTR], [1],
>                          [Define if you have libattr])],
>               [AC_MSG_ERROR([libattr not found.])])
> 
> Does not add -lattr to $LIBS and the variable LIBATTR is never 
> referenced anywhere in the makefiles.. the build succeeds because 
> libattr duplicates libc functionality….

Then commit f5c16606 is incorrect: configure.ac should be looking for
the attr/xattr.h header, not the libattr.so library.

A problem for both fedfs-utils 0.9 and 0.10.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
Cristian Rodríguez - June 23, 2014, 11 p.m.
El 23/06/14 18:54, Chuck Lever escribió:
> 
> On Jun 23, 2014, at 6:38 PM, Cristian Rodríguez <crrodriguez@opensuse.org> wrote:
> 
>> Of course the package build will have libattr-devel as a build 
>> dependency, since this code include <attr/xattr.h> and will fail 
>> without it.. however...the code is not even linking the attr library so 
>> it will never get used.
>>
>> AC_CHECK_LIB([attr], [fgetxattr],
>>              [AC_SUBST([LIBATTR], ["-lattr"])
>>                AC_DEFINE([HAVE_LIBATTR], [1],
>>                          [Define if you have libattr])],
>>               [AC_MSG_ERROR([libattr not found.])])
>>
>> Does not add -lattr to $LIBS and the variable LIBATTR is never 
>> referenced anywhere in the makefiles.. the build succeeds because 
>> libattr duplicates libc functionality….
> 
> Then commit f5c16606 is incorrect: configure.ac should be looking for
> the attr/xattr.h header, not the libattr.so library.

Nope, it should be only looking for <sys/xattr.h> as glibc 2.3 that
contains this functionality was released in 2002.. 12 years ago.. the
xattr functions that are provided by glibc will disappear in upcoming
releases of libattr...working on that too..
Chuck Lever - June 23, 2014, 11:17 p.m.
On Jun 23, 2014, at 7:00 PM, Cristian Rodríguez <crrodriguez@opensuse.org> wrote:

> El 23/06/14 18:54, Chuck Lever escribió:
>> 
>> On Jun 23, 2014, at 6:38 PM, Cristian Rodríguez <crrodriguez@opensuse.org> wrote:
>> 
>>> Of course the package build will have libattr-devel as a build 
>>> dependency, since this code include <attr/xattr.h> and will fail 
>>> without it.. however...the code is not even linking the attr library so 
>>> it will never get used.
>>> 
>>> AC_CHECK_LIB([attr], [fgetxattr],
>>>             [AC_SUBST([LIBATTR], ["-lattr"])
>>>               AC_DEFINE([HAVE_LIBATTR], [1],
>>>                         [Define if you have libattr])],
>>>              [AC_MSG_ERROR([libattr not found.])])
>>> 
>>> Does not add -lattr to $LIBS and the variable LIBATTR is never 
>>> referenced anywhere in the makefiles.. the build succeeds because 
>>> libattr duplicates libc functionality….
>> 
>> Then commit f5c16606 is incorrect: configure.ac should be looking for
>> the attr/xattr.h header, not the libattr.so library.
> 
> Nope, it should be only looking for <sys/xattr.h> as glibc 2.3 that
> contains this functionality was released in 2002.. 12 years ago.. the
> xattr functions that are provided by glibc will disappear in upcoming
> releases of libattr...working on that too..

Can you point me to something that documents the deprecation of attr/xattr.h?

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Patch

diff --git a/configure.ac b/configure.ac
index a35c798..f7a63cf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -141,11 +141,6 @@  AC_CHECK_LIB([ssl], [SSL_CTX_new],
 		 AC_DEFINE([HAVE_LIBSSL], [1],
 			   [Define if you have libssl])],
 		[AC_MSG_ERROR([libssl not found.])])
-AC_CHECK_LIB([attr], [fgetxattr],
-		[AC_SUBST([LIBATTR], ["-lattr"])
-		 AC_DEFINE([HAVE_LIBATTR], [1],
-			   [Define if you have libattr])],
-		[AC_MSG_ERROR([libattr not found.])])
 AC_CHECK_LIB([gssapi_krb5], [gss_acquire_cred],
 		[AC_SUBST([LIBGSSAPI_KRB5], ["-lgssapi_krb5"])
 		 AC_DEFINE([HAVE_LIBGSSAPI_KRB5], [1],
diff --git a/src/libjunction/display-junction.c b/src/libjunction/display-junction.c
index ac5797b..2e924da 100644
--- a/src/libjunction/display-junction.c
+++ b/src/libjunction/display-junction.c
@@ -35,7 +35,7 @@ 
 #include <locale.h>
 #include <langinfo.h>
 
-#include <attr/xattr.h>
+#include <sys/xattr.h>
 
 #include "junction.h"
 #include "junction-internal.h"
diff --git a/src/libjunction/junction.c b/src/libjunction/junction.c
index 7b7b2e4..71ccc7a 100644
--- a/src/libjunction/junction.c
+++ b/src/libjunction/junction.c
@@ -38,7 +38,7 @@ 
 #include <errno.h>
 #include <dirent.h>
 
-#include <attr/xattr.h>
+#include <sys/xattr.h>
 
 #include "fedfs.h"
 #include "nsdb.h"