| Message ID | 1403560273-109277-1-git-send-email-crrodriguez@opensuse.org |
|---|---|
| State | Superseded |
| Headers | show |
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
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."
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
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..
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
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"
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(-)