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

login
register
mail settings
Submitter Cristian Rodríguez
Date June 24, 2014, 12:05 a.m.
Message ID <53A8C0D8.9070802@opensuse.org>
Download mbox | patch
Permalink /patch/363250/
State Superseded
Headers show

Comments

Cristian Rodríguez - June 24, 2014, 12:05 a.m.
El 23/06/14 19:17, Chuck Lever escribió:
> 
> 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?

There is no document about it but it is quite simple.. no one wants to
maintain duplicate interfaces..

attr/xattr.h wont go away.. the library and headers will be modified,
attr/xattr.h will just include <sys/xattr.h>.. the functions that are
already provided by glibc will only stay available for existing binaries
but they will disappear from the default version, hence AC_CHECK_LIB or
similar won't find it there anymore..

Ok,, attached is a different patch that will probably make everyone
happy..otherwise we will be arguing about this until next year thing
that I am not particularly interested in doing.
Chuck Lever - June 24, 2014, 2:49 p.m.
Hi Cristian-

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

> El 23/06/14 19:17, Chuck Lever escribió:
>> 
>> 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?
> 
> There is no document about it but it is quite simple.. no one wants to
> maintain duplicate interfaces..
> 
> attr/xattr.h wont go away.. the library and headers will be modified,
> attr/xattr.h will just include <sys/xattr.h>.. the functions that are
> already provided by glibc will only stay available for existing binaries
> but they will disappear from the default version, hence AC_CHECK_LIB or
> similar won't find it there anymore..

> Ok,, attached is a different patch that will probably make everyone
> happy..otherwise we will be arguing about this until next year thing
> that I am not particularly interested in doing.

Thanks for pursuing this. I know that due diligence can sometimes be a
twisty little maze of passages, all alike.

I agree that the current CHECK_LIB(attr) is bogus and should be removed.
But I still don’t understand why the libjunction changes are necessary
if attr/xattr.h is not being deprecated.

No new patch is needed, just a clarification.

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

Patch

>From 1cfae152d1a2d6d2fbfe3621ccbf693e7339c349 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cristian=20Rodr=C3=ADguez?= <crrodriguez@opensuse.org>
Date: Mon, 23 Jun 2014 20:00:22 -0400
Subject: [PATCH] Prefer sys/xattr.h instead of attr/xattr.h

Xattrs functions are implemented in glibc nowadays and must
be prefered instead of those in libattr.
---
 configure.ac                       | 8 +++-----
 src/libjunction/display-junction.c | 7 +++++++
 src/libjunction/junction.c         | 7 +++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index a35c798..a87f6d0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -141,11 +141,9 @@  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_SEARCH_LIBS([fgetxattr], [attr], [AC_CHECK_HEADERS([sys/xattr.h attr/xattr.h])], AC_MSG_ERROR([No xattr support 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..87d8f6d 100644
--- a/src/libjunction/display-junction.c
+++ b/src/libjunction/display-junction.c
@@ -23,6 +23,7 @@ 
  *	http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt
  */
 
+#include "config.h"
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -35,7 +36,13 @@ 
 #include <locale.h>
 #include <langinfo.h>
 
+#ifdef HAVE_SYS_XATTR_H
+#include <sys/xattr.h>
+#elif HAVE_ATTR_XATTR_H
 #include <attr/xattr.h>
+#else
+#error no xattr implementation present
+#endif
 
 #include "junction.h"
 #include "junction-internal.h"
diff --git a/src/libjunction/junction.c b/src/libjunction/junction.c
index 7b7b2e4..6227bf0 100644
--- a/src/libjunction/junction.c
+++ b/src/libjunction/junction.c
@@ -23,6 +23,7 @@ 
  *	http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt
  */
 
+#include "config.h"
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -38,7 +39,13 @@ 
 #include <errno.h>
 #include <dirent.h>
 
+#ifdef HAVE_SYS_XATTR_H
+#include <sys/xattr.h>
+#elif HAVE_ATTR_XATTR_H
 #include <attr/xattr.h>
+#else
+#error no xattr implementation present
+#endif
 
 #include "fedfs.h"
 #include "nsdb.h"
-- 
2.0.0