gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK is wrong in some cases

[Pavel Fedin]

 Hello!

 gnulib is used by libvirt project (http://libvirt.org), and during
cross-compiling libvirt i stumbled upon a problem in gnulib's autoconf
module.
 The problem happens under the following conditions:
--build=i686-mingw32 --host=arm-linux-gnueabihf
 Yes, i am cross-compiling Linux version of libvirt under Windows. It works
fine except this small test. The problem is in the first line:
--- cut ---
 if test "$as_ln_s" = "ln -s" && ln -s conftest.file conftest.sym; then
   AC_RUN_IFELSE(
 [AC_LANG_PROGRAM(
[AC_INCLUDES_DEFAULT],
[[struct stat sbuf;
  /* Linux will dereference the symlink and fail, as required by
 POSIX.  That is better in the sense that it means we will
not
 have to compile and use the lstat wrapper.  */
  return lstat ("conftest.sym/", &sbuf) == 0;
]])],
 [gl_cv_func_lstat_dereferences_slashed_symlink=yes],
 [gl_cv_func_lstat_dereferences_slashed_symlink=no],
 [case "$host_os" in
# Guess yes on glibc systems.
*-gnu*) gl_cv_func_lstat_dereferences_slashed_symlink="guessing
yes" ;;
# If we don't know, assume the worst.
*)  gl_cv_func_lstat_dereferences_slashed_symlink="guessing
no" ;;
  esac
 ])
 else
   # If the 'ln -s' command failed, then we probably don't even
   # have an lstat function.
   gl_cv_func_lstat_dereferences_slashed_symlink="guessing no"
 fi
--- cut ---
 AC_RUN_IFELSE() is supposed to perfectly handle cross-compilation case and
it knows that *-gnu* systems normally answer "yes" to this test. However,
"$as_ln_s" = "ln -s" test spoils everything on MinGW because 'ln' is
substituted by 'cp' there (because there is no support for symlinks in MinGW
for historical reasons).
 I would suggest to fix this by explicit check for "cross-compiling" = "no"
before attempting the whole thing:
--- cut ---
 if test "$cross_compiling" = no; then
   --- the whole original AC_RUN_IFELSE() thing goes here, except
cross-compile fragment ---
 else
   # cross-compiling
   case "$host_os" in
 # Guess yes on glibc systems.
 *-gnu*) gl_cv_func_lstat_dereferences_slashed_symlink="guessing
yes" ;;
 # If we don't know, assume the worst.
 *)  gl_cv_func_lstat_dereferences_slashed_symlink="guessing no"
;;
   esac
 fi
--- cut ---
 Anyway, this test makes no sense when build != host.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [COREUTILS 2/2] ls: Don't treat lack of acl support as an error

[Pádraig Brady]

On 12/04/15 15:37, Andreas Gruenbacher wrote:
> * src/ls.c (file_has_acl_cache): When a file system doesn't support
> acls, fail with errno set to ENOTSUP.
> (gobble_file): Don't treat lack of acl support as an error.
> ---
>  src/ls.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ls.c b/src/ls.c
> index b308dd3..884e042 100644
> --- a/src/ls.c
> +++ b/src/ls.c
> @@ -2866,7 +2866,7 @@ getfilecon_cache (char const *file, struct fileinfo *f, 
> bool deref)
>  
>  /* Cache file_has_acl failure, when it's trivial to do.
> Like file_has_acl, but when F's st_dev says it's on a file
> -   system lacking ACL support, return 0 with ENOTSUP immediately.  */
> +   system lacking ACL support, fail with ENOTSUP immediately.  */
>  static int
>  file_has_acl_cache (char const *file, struct fileinfo *f)
>  {
> @@ -2877,14 +2877,11 @@ file_has_acl_cache (char const *file, struct fileinfo 
> *f)
>if (f->stat.st_dev == unsupported_device)
>  {
>errno = ENOTSUP;
> -  return 0;
> +  return -1;
>  }
>  
> -  /* Zero errno so that we can distinguish between two 0-returning cases:
> - "has-ACL-support, but only a default ACL" and "no ACL support". */
> -  errno = 0;
>int n = file_has_acl (file, &f->stat);
> -  if (n <= 0 && errno_unsupported (errno))
> +  if (n < 0 && errno_unsupported (errno))
>  unsupported_device = f->stat.st_dev;
>return n;
>  }
> @@ -3076,7 +3073,7 @@ gobble_file (char const *name, enum filetype type, 
> ino_t inode,
>if (err == 0 && format == long_format)
>  {
>int n = file_has_acl_cache (absolute_name, f);
> -  err = (n < 0);
> +  err = (n < 0 && ! errno_unsupported (errno));
>have_acl = (0 < n);
>  }

I dislike this change actually.
Or more accurately, the gnulib change that changed the file_has_acl()
interface, requiring this change.

Previously in gnulib we mapped ENOTSUP to return 0 using:
http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/acl-errno-valid.c

Since we've now changed file_has_acl() to return -1 in this case,
all gnulib users may now be printing erroneous errors etc.

Is there any reason not to use the same gnulib acl_errno_valid() logic
in the newly added "avoiding libacl" path?

thanks,
Pádraig.