Re: copy_file_preserving variant that doesn't exit on error?

2011-08-01 Thread Reuben Thomas 
By the way, I note that the return value of chown is ignored where no
other return value is; is this an oversight, or is it really the case
that this is the one operation whose failure can be overlooked?

Re: copy_file_preserving variant that doesn't exit on error?

2011-08-01 Thread Reuben Thomas 
On 24 July 2011 21:05, Reuben Thomas  wrote:
> I just came across the copy-file module, which does exactly what I
> want (it is even geared to making backup files), but (unfortunately
> for use in an interactive program) exits on error.

Just to move this along a bit, I attach a patch which changes
copy_file_preserving to return error codes instead of calling error.

Obviously, I don't expect this to be applied to gnulib, but as per my
original message, I'd like to know if a version of
copy_file_preserving along these lines is thought to be a good idea
for interactive use (I need it for creating backup files in GNU Zile),
and how I might produce a patch that provides an error-returning
variant without duplicating all the code.

diff --git a/lib/copy-file.h b/lib/copy-file.h
index cb8b1f7..b0c29e0 100644
--- a/lib/copy-file.h
+++ b/lib/copy-file.h
@@ -26,7 +26,7 @@ extern "C" {
Modification times, owner, group and access permissions are preserved as
far as possible.
Exit upon failure.  */
-extern void copy_file_preserving (const char *src_filename, const
char *dest_filename);
+extern int copy_file_preserving (const char *src_filename, const char
*dest_filename);


 #ifdef __cplusplus
diff --git a/lib/copy-file.c b/lib/copy-file.c
index f9cd9c0..b5b8447 100644
--- a/lib/copy-file.c
+++ b/lib/copy-file.c
@@ -53,7 +53,7 @@

 enum { IO_SIZE = 32 * 1024 };

-void
+int
 copy_file_preserving (const char *src_filename, const char *dest_filename)
 {
   int src_fd;
@@ -63,37 +63,37 @@ copy_file_preserving (const char *src_filename,
const char *dest_filename)
   char *buf = xmalloc (IO_SIZE);

   src_fd = open (src_filename, O_RDONLY | O_BINARY);
-  if (src_fd < 0 || fstat (src_fd, &statbuf) < 0)
-error (EXIT_FAILURE, errno, _("error while opening \"%s\" for reading"),
-   src_filename);
+  if (src_fd < 0)
+return -1;
+  if (fstat (src_fd, &statbuf) < 0)
+goto error_exit_2;

   mode = statbuf.st_mode & 0;

   dest_fd = open (dest_filename, O_WRONLY | O_CREAT | O_TRUNC |
O_BINARY, 0600);
   if (dest_fd < 0)
-error (EXIT_FAILURE, errno, _("cannot open backup file \"%s\" for
writing"),
-   dest_filename);
+return -1;

   /* Copy the file contents.  */
   for (;;)
 {
   size_t n_read = safe_read (src_fd, buf, IO_SIZE);
   if (n_read == SAFE_READ_ERROR)
-error (EXIT_FAILURE, errno, _("error reading \"%s\""), src_filename);
+goto error_exit;
   if (n_read == 0)
 break;

   if (full_write (dest_fd, buf, n_read) < n_read)
-error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
+goto error_exit;
 }

   free (buf);

 #if !USE_ACL
   if (close (dest_fd) < 0)
-error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
+goto error_exit_2;
   if (close (src_fd) < 0)
-error (EXIT_FAILURE, errno, _("error after reading \"%s\""), src_filename);
+return -1;
 #endif

   /* Preserve the access and modification times.  */
@@ -123,15 +123,23 @@ copy_file_preserving (const char *src_filename,
const char *dest_filename)
   /* Preserve the access permissions.  */
 #if USE_ACL
   if (copy_acl (src_filename, src_fd, dest_filename, dest_fd, mode))
-exit (EXIT_FAILURE);
+goto error_exit;
 #else
   chmod (dest_filename, mode);
 #endif

 #if USE_ACL
   if (close (dest_fd) < 0)
-error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
+goto error_exit_2;
   if (close (src_fd) < 0)
-error (EXIT_FAILURE, errno, _("error after reading \"%s\""), src_filename);
+return -1;
 #endif
+
+  return 0;
+
+ error_exit:
+  close (dest_fd);
+ error_exit_2:
+  close (src_fd);
+  return -1;
 }

Re: relocatable-prog : relocatable symbol not found

2011-08-01 Thread Sylvain Beucler 
Hi,

I'd like some input about this issue.
I don't understand how gnulib-tool generates the Makefile.
In this case relocatable.c is added to EXTRA_libgnu_a_SOURCES but not
libgnu_a_SOURCES :/

(btw, now could be a good time to add FreeDink to 'users.txt' :))

- Sylvain

On Thu, Jul 28, 2011 at 12:19:00AM +0200, Sylvain Beucler wrote:
> Hi,
> 
> I updated gnulib and I now have an error when compiling my freedink project:
> 
> gcc  -g -O2 -Wall -std=c99  -I/usr/include/SDL -D_GNU_SOURCE=1 
> -D_REENTRANT `"../autotools/reloc-ldflags" "x86_64-unknown-linux-gnu" "" 
> /usr/local/bin`  -o freedink bgm.o dinkini.o dinkc.o dinkc_bindings.o 
> dinkc_console.o dinkc_sp_custom.o dinkvar.o fastfile.o game_engine.o 
> str_util.o io_util.o sfx.o gfx.o gfx_fade.o gfx_tiles.o gfx_palette.o 
> gfx_fonts.o init.o rect.o input.o binreloc.o freedink_xpm.o paths.o log.o 
> gfx_sprites.o vgasys_fon.o msgbox.o i18n.o meminfo.o screen.o  
> SDL_rwops_libzip.o freedink.o update_frame.o  ../gnulib/lib/libgnu.a  
> -lSDL_mixer -lSDL_image -lSDL_ttf -lSDL_gfx -lpthread -L/usr/lib -lSDL  -lzip 
> -lz   -lfontconfig-lm
> paths.c:90: error: undefined reference to 'relocate'
> progreloc.c:297: error: undefined reference to 'compute_curr_prefix'
> progreloc.c:302: error: undefined reference to 'set_relocation_prefix'
> collect2: ld returned 1 exit status
> 
> (paths.c is my source file)
> 
> 
> I note that relocatable.c is not included anymore in libgnu.a:
> ar cru libgnu.a allocator.o areadlink.o areadlink-with-size.o canonicalize.o 
> careadlinkat.o cloexec.o dirname.o basename.o dirname-lgpl.o basename-lgpl.o 
> stripslash.o exitfail.o fd-hook.o file-set.o filenamecat-lgpl.o hash.o 
> hash-pjw.o hash-triple.o malloca.o openat-die.o progname.o same.o save-cwd.o 
> dup-safer.o fd-safer.o pipe-safer.o xmalloc.o xalloc-die.o xgetcwd.o 
> xreadlink.o xstrndup.o asnprintf.o chdir-long.o fcntl.o getcwd.o 
> getcwd-lgpl.o getopt.o getopt1.o openat-proc.o printf-args.o printf-parse.o 
> progreloc.o vasnprintf.o 
> 
> My modules in gnulib/m4/gnulib-cache.m4:
> gl_MODULES([
>   alloca
>   canonicalize
>   dirname
>   getopt-gnu
>   gettext
>   hash
>   relocatable-prog
>   strcase
>   strdup-posix
>   strings
>   vasprintf
>   xalloc
> ])
> 
> 
> I didn't find a note in NEWS that would recommend a change in the
> freedink code.
> 
> Any tip? :)
> 
> -- 
> Sylvain

Re: REPLACE_TOWLOWER='1'

2011-08-01 Thread Bruno Haible 
Hi Sam,

> >> why is tolower replaced on linux?
> ...
> ac_cv_have_decl_towlower=yes
> 
> REPLACE_TOWLOWER='1'

Oops. that was a mistake yesterday. Thanks for having spotted it!

Fixed as follows.


2011-08-01  Bruno Haible  

wctype-h: Fix last change.
* m4/wctype_h.m4 (gl_WCTYPE_H): If towlower is defined, set
REPLACE_TOWLOWER to 0.
Reported by Sam Steingold .

*** m4/wctype_h.m4.orig Mon Aug  1 21:56:33 2011
--- m4/wctype_h.m4  Mon Aug  1 21:54:29 2011
***
*** 1,4 
! # wctype_h.m4 serial 15
  
  dnl A placeholder for ISO C99 , for platforms that lack it.
  
--- 1,4 
! # wctype_h.m4 serial 16
  
  dnl A placeholder for ISO C99 , for platforms that lack it.
  
***
*** 82,107 
if test $REPLACE_ISWCNTRL = 1; then
  REPLACE_TOWLOWER=1
else
! AC_CHECK_DECLS([towlower],,,
!   [[/* Tru64 with Desktop Toolkit C has a bug:  must be
!included before .
!BSD/OS 4.0.1 has a bug: ,  and 
!must be included before .  */
! #include 
! #include 
! #include 
! #include 
! #if HAVE_WCTYPE_H
! # include 
! #endif
!   ]])
! if test $ac_cv_have_decl_towlower = yes; then
!   dnl On Minix 3.1.8, the system's  declares towlower() and
!   dnl towupper() although it does not have the functions. Avoid a 
collision
!   dnl with gnulib's replacement.
!   REPLACE_TOWLOWER=1
! else
REPLACE_TOWLOWER=0
  fi
fi
AC_SUBST([REPLACE_TOWLOWER])
--- 82,112 
if test $REPLACE_ISWCNTRL = 1; then
  REPLACE_TOWLOWER=1
else
! AC_CHECK_FUNCS([towlower])
! if test $ac_cv_func_towlower = yes; then
REPLACE_TOWLOWER=0
+ else
+   AC_CHECK_DECLS([towlower],,,
+ [[/* Tru64 with Desktop Toolkit C has a bug:  must be
+  included before .
+  BSD/OS 4.0.1 has a bug: ,  and 
+  must be included before .  */
+   #include 
+   #include 
+   #include 
+   #include 
+   #if HAVE_WCTYPE_H
+   # include 
+   #endif
+ ]])
+   if test $ac_cv_have_decl_towlower = yes; then
+ dnl On Minix 3.1.8, the system's  declares towlower() and
+ dnl towupper() although it does not have the functions. Avoid a
+ dnl collision with gnulib's replacement.
+ REPLACE_TOWLOWER=1
+   else
+ REPLACE_TOWLOWER=0
+   fi
  fi
fi
AC_SUBST([REPLACE_TOWLOWER])

-- 
In memoriam Marie Trintignant

Re: REPLACE_TOWLOWER='1'

2011-08-01 Thread Sam Steingold 
> * Eric Blake  [2011-08-01 09:00:07 -0600]:
> On 08/01/2011 08:56 AM, Sam Steingold wrote:
>> why is tolower replaced on linux?
>
> Did you mean towlower rather than tolower?

towlower, sorry

> What does config.log say?

configure:29272: checking whether towlower is declared
configure:29272: gcc -c   conftest.c >&5
configure:29272: $? = 0
configure:29272: result: yes

| #define HAVE_DECL_TOWLOWER 1

ac_cv_have_decl_towlower=yes

REPLACE_TOWLOWER='1'

these are the only mentions of towlower in config.log

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://truepeace.org http://palestinefacts.org http://iris.org.il
http://ffii.org http://thereligionofpeace.com http://jihadwatch.org
20% of people do 80% of work; also 80% of people think they are in those 20%.

Re: top_srcdir in Makefile.am & Makefile.in

2011-08-01 Thread Sam Steingold 
> * Bruno Haible  [2011-07-30 02:20:08 +0200]:
>
> Sam Steingold wrote:
>> clisp directory structure is:
>> clisp - top level; hand-written configure script (ask Bruno)
>> clisp/src - most sources, configure.in, configure, aclocal.m4
>> clisp/src/gllib, clisp/src/glm4 - imported from gnulib
>> clisp/src/build-aux - imported from many places, mostly gnulib
>
> So, in clisp, $(top_srcdir) ought to be clisp/src.

thanks.

> You can get in trouble here if you use fake configure.ac / configure
> file that are present at the moment 'automake' is run but are removed
> afterwards.

I get the exact same files if I run aclocal on clisp/src/configure.ac as
on clisp/configure.ac.

>> $ grep top_srcdir src/gllib/Makefile.am
>> appears to indicate that top_srcdir should point to clisp.
>
> Maybe this is related to this hack in clisp/Makefile.devel:
>
> src/gllib/Makefile.in : src/gllib/Makefile.am src/configure.in src/aclocal.m4
> cd src && automake gllib/Makefile && \
> sed -i -e 
> 's,$$(top_srcdir)/src/build-aux,$$(CLISP_LIBDIR)/build-aux,' \
>   -e 's,$$(top_srcdir)/$$cl_cv_clisp_libdir,$$(CLISP_LIBDIR),' \
> gllib/Makefile.in

you are out of date, the current hack is

src/gllib/Makefile.in : src/gllib/Makefile.am src/configure.in src/aclocal.m4
cd src && automake --gnits gllib/Makefile && \
sed -i -e 's,$$(top_srcdir)/src/build-aux,$$(top_srcdir)/build-aux,' \
gllib/Makefile.in

(whose intent you should approve of)

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://pmw.org.il http://palestinefacts.org http://ffii.org
http://thereligionofpeace.com http://openvotingconsortium.org http://memri.org
The difference between theory and practice is that in theory there isn't any.

Re: REPLACE_TOWLOWER='1'

2011-08-01 Thread Eric Blake 
On 08/01/2011 08:56 AM, Sam Steingold wrote:
> why is tolower replaced on linux?

Did you mean towlower rather than tolower?  What does config.log say?
Without knowing why the configure test failed, it's hard to say if you
might have stumbled over a bug in the wctype_h.m4 file.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

REPLACE_TOWLOWER='1'

2011-08-01 Thread Sam Steingold 
why is tolower replaced on linux?
-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://iris.org.il http://palestinefacts.org http://honestreporting.com
http://thereligionofpeace.com http://www.PetitionOnline.com/tap12009/
Why use Windows, when there are Doors?

Re: [PATCH]

2011-08-01 Thread Jim Meyering 
Iain Nicol wrote:
> Gnulib's file  contains copy-pastable usage
> instructions.  It seems to me that these instructions are missing a
> line.  As a result, for example, ``make distcheck'' in a clone of GNU
> Patch's repo fails with the following:
>
> echo 2.6.1.143-5862 > ../.version-t && mv ../.version-t ../.version
> /bin/bash: ../.version-t: Permission denied
>
> I notice that a few GNU projects (gcal, Guile, M4) explicitly add the
> file ``.version'' to EXTRA_DIST.  This prevents the above distcheck
> failure.
>
> At first I wasn't sure if this was the right solution, but then I
> noticed git-version-gen explicitly says:
>
>   # .version - present in a checked-out repository and in a distribution
>   # tarball.
>
> Adding .version to EXTRA_DIST, unsurprisingly, seems to be required for
> the latter half of that sentence to be true.
>
> For lazy people like me who want copy pasted code to Just Work, please
> consider making the need for this explicit in the instructions, like in
> the attached patch.  Assuming you agree this is the right solution, that
> is.
...

Thanks for the patch.
I've adjusted the log and will push the following later today.
The only change I considered was to use "+=" in place of "=".
But that would have to affect BUILT_SOURCES, too, and would then
require prior definition of those two variables.  Not sure it's worth it.
It's only a suggestion, after all.

>From 835a226d823da6dacd67b77165d5009f5382daad Mon Sep 17 00:00:00 2001
From: Iain Nicol 
Date: Sun, 31 Jul 2011 13:30:59 +0100
Subject: [PATCH] git-version-gen: document that EXTRA_DIST must include
 .version

* build-aux/git-version-gen: In the how-to-use comment, document
that EXTRA_DIST must include .version.  Otherwise, "make distcheck"
will fail when run from an unpacked distribution tarball.
---
 ChangeLog |7 +++
 build-aux/git-version-gen |7 ---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2160e20..55577f0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2011-07-31  Iain Nicol  
+
+   git-version-gen: document that EXTRA_DIST must include .version
+   * build-aux/git-version-gen: In the how-to-use comment, document
+   that EXTRA_DIST must include .version.  Otherwise, "make distcheck"
+   will fail when run from an unpacked distribution tarball.
+
 2011-07-25  Jim Meyering  

tests: test-update-copyright.sh: remove unnecessary "rm" commands
diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
index 686f703..6d71446 100755
--- a/build-aux/git-version-gen
+++ b/build-aux/git-version-gen
@@ -1,6 +1,6 @@
 #!/bin/sh
 # Print a version string.
-scriptversion=2011-02-19.19; # UTC
+scriptversion=2011-07-31.12; # UTC

 # Copyright (C) 2007-2011 Free Software Foundation, Inc.
 #
@@ -57,9 +57,10 @@ scriptversion=2011-02-19.19; # UTC
 # [bug-project@example])
 #
 # Then use the following lines in your Makefile.am, so that .version
-# will be present for dependencies, and so that .tarball-version will
-# exist in distribution tarballs.
+# will be present for dependencies, and so that .version and
+# .tarball-version will exist in distribution tarballs.
 #
+# EXTRA_DIST = $(top_srcdir)/.version
 # BUILT_SOURCES = $(top_srcdir)/.version
 # $(top_srcdir)/.version:
 #  echo $(VERSION) > $@-t && mv $@-t $@
--
1.7.6.347.g4db0d