Re: [PATCH] mkdir/split: send --verbose output to stdout + [PATCH]: same for rmdir/install

[Jim Meyering]

Ondřej Vašík <address@hidden> wrote:
> Ok, hopefully fixed your objections from that mail and from the
> discussion off the list, patch is in attachement.

Thank you!

I've made a few name changes by transforming the patch before
applying it, so that the function name and file name match, and
so that the new files are named prog-fprintf.[ch].  I shouldn't have
suggested prog-printf.c: it takes a FILE* parameter like *f*printf.

  perl -pe \
  
's/PROG_PRINTF_H/PROG_FPRINTF_H/;s/prog_vfprintf/prog_fprintf/;s/prog-printf\./prog-fprintf\./g'

Doing it that way has the nice side effect of adjusting the log
entries at the same time.

Oops.  Trying to apply, I see that there are still trailing blanks:

  $ git am /t/prog-vfprintf.patch
  Applying install, rmdir: write --verbose output to stdout, not stderr.
  .dotest/patch:23: trailing whitespace.
    install, mkdir, rmdir and split now write --verbose output to stdout,
  .dotest/patch:37: trailing whitespace.
  Display to standard error all status updates as sterilization proceeds.
  .dotest/patch:147: trailing whitespace.
  /* prog-fprintf.c - common formating output functions and definitions
  .dotest/patch:171: trailing whitespace.
  /* Display program name followed by variable list.
  .dotest/patch:189: trailing whitespace.
  /* prog-fprintf.h - common formating output functions and definitions
  warning: 5 lines add whitespace errors.

so I backed out that patch:

  $ git reset --hard HEAD^

removed the trailing blanks it would have added,

  $ perl -pi -e 's/^(\+.*?) +$/$1/' /t/prog-vfprintf.patch

and reapplied, and this time "git am" succeeded.

But then "make syntax-check" failed like this:

  ...
  src/prog-fprintf.c
  Makefile.maint: the above files do not include <config.h>
  make: *** [sc_require_config_h] Error 1

I fixed that, and ran "make syntax-check" again.  Failed with this:

  prog_fprintf
  the above functions should have static scope
  make[1]: *** [sc_tight_scope] Error 1

To fix that, my policy is that you must mark the declaration "extern",
to tell the test that you really do intend to make it public.

I did that, but to my surprise, it was not enough.  I still got the same
"make syntax-check" failure.  Investigation shows that since this is the
first such function declared in coreutils/src/*.h, the sc_tight_scope
rule didn't cover it.  It searched for extern function declarations only
in $(SOURCES), which doesn't happen to include that new .h file.
The existing search wouldn't have found it anyhow, since it's
all on one line, as is standard for a prototype in a .h file.

So I fixed the rule to accommodate the new extern-function-in-.h,
and now, *finally*, "make syntax-check" passes.

The only other change was to move "#include <stdio.h>"
from the .c file into the .h file, since the declaration
uses the stdio.h-provided FILE type.

Here's what I expect to push:
---------------------
Subject: [PATCH] install, rmdir: write --verbose output to stdout, not to 
stderr.

* src/install.c (announce_mkdir): Write verbose output to stdout,
not to stderr.
* src/mkdir.c (announce mkdir): Use prog_fprintf for verbose output.
* src/prog-fprintf.c (prog_fprintf): New function and file.
* src/prog-fprintf.h: New file.
* src/rmdir.c (main): Write verbose output to stdout, not to stderr.
Quote directory name in a diagnostic.
* src/rmdir.c (remove_parents): Write verbose output to stdout,
not to stderr.
* doc/coreutils.texi: Mention that shred verbose output is to stderr.
* NEWS: Mention the changes.

Signed-off-by: Ondřej Vašík <address@hidden>
Signed-off-by: Jim Meyering <address@hidden>
---
 NEWS               |    3 ++-
 doc/coreutils.texi |    2 +-
 src/Makefile.am    |    6 +++++-
 src/install.c      |    5 +++--
 src/mkdir.c        |   16 ++--------------
 src/prog-fprintf.c |   37 +++++++++++++++++++++++++++++++++++++
 src/prog-fprintf.h |   24 ++++++++++++++++++++++++
 src/rmdir.c        |    7 ++++---
 8 files changed, 78 insertions(+), 22 deletions(-)
 create mode 100644 src/prog-fprintf.c
 create mode 100644 src/prog-fprintf.h

diff --git a/NEWS b/NEWS
index a738dab..948bced 100644
--- a/NEWS
+++ b/NEWS
@@ -40,7 +40,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 ** Consistency

-  mkdir and split now write --verbose output to stdout, not stderr.
+  install, mkdir, rmdir and split now write --verbose output to stdout,
+  not to stderr.


 * Noteworthy changes in release 6.10 (2008-01-22) [stable]
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index df6792d..f161c4d 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8190,7 +8190,7 @@ If a file has multiple links, only the named links will 
be removed.
 @itemx --verbose
 @opindex -v
 @opindex --verbose
-Display status updates as sterilization proceeds.
+Display to standard error all status updates as sterilization proceeds.

 @item -x
 @itemx --exact
diff --git a/src/Makefile.am b/src/Makefile.am
index c85f853..44d802e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -54,6 +54,7 @@ noinst_HEADERS = \
   fs.h \
   group-list.h \
   ls.h \
+  prog-fprintf.h \
   remove.h \
   system.h \
   wheel-size.h \
@@ -203,7 +204,7 @@ copy_sources = copy.c cp-hash.c
 # to install before applying any user-specified name transformations.

 transform = s/ginstall/install/; @program_transform_name@
-ginstall_SOURCES = install.c $(copy_sources)
+ginstall_SOURCES = install.c prog-fprintf.c $(copy_sources)

 # This is for the '[' program.  Automake transliterates '[' to '_'.
 __SOURCES = lbracket.c
@@ -221,6 +222,9 @@ chgrp_SOURCES = chgrp.c chown-core.c
 mv_SOURCES = mv.c remove.c $(copy_sources)
 rm_SOURCES = rm.c remove.c

+mkdir_SOURCES = mkdir.c prog-fprintf.c
+rmdir_SOURCES = rmdir.c prog-fprintf.c
+
 uname_SOURCES = uname.c uname-uname.c
 arch_SOURCES = uname.c uname-arch.c

diff --git a/src/install.c b/src/install.c
index db08751..1d04373 100644
--- a/src/install.c
+++ b/src/install.c
@@ -1,5 +1,5 @@
 /* install - copy files and set attributes
-   Copyright (C) 89, 90, 91, 1995-2007 Free Software Foundation, Inc.
+   Copyright (C) 89, 90, 91, 1995-2008 Free Software Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -34,6 +34,7 @@
 #include "mkancesdirs.h"
 #include "mkdir-p.h"
 #include "modechange.h"
+#include "prog-fprintf.h"
 #include "quote.h"
 #include "quotearg.h"
 #include "savewd.h"
@@ -762,7 +763,7 @@ announce_mkdir (char const *dir, void *options)
 {
   struct cp_options const *x = options;
   if (x->verbose)
-    error (0, 0, _("creating directory %s"), quote (dir));
+    prog_fprintf (stdout, _("creating directory %s"), quote (dir));
 }

 /* Make ancestor directory DIR, whose last file name component is
diff --git a/src/mkdir.c b/src/mkdir.c
index 3781065..3952594 100644
--- a/src/mkdir.c
+++ b/src/mkdir.c
@@ -27,6 +27,7 @@
 #include "lchmod.h"
 #include "mkdir-p.h"
 #include "modechange.h"
+#include "prog-fprintf.h"
 #include "quote.h"
 #include "savewd.h"

@@ -79,19 +80,6 @@ Mandatory arguments to long options are mandatory for short 
options too.\n\
   exit (status);
 }

-/* Verbose formatted output of variable count of arguments.  */
-static void
-verbose_output (FILE *fp, char const *fmt, ...)
-{
-  va_list ap;
-  fputs (program_name, fp);
-  fputs (": ", fp);
-  va_start (ap, fmt);
-  vfprintf (fp, fmt, ap);
-  va_end (ap);
-  fputc ('\n', fp);
-}
-
 /* Options passed to subsidiary functions.  */
 struct mkdir_options
 {
@@ -118,7 +106,7 @@ announce_mkdir (char const *dir, void *options)
 {
   struct mkdir_options const *o = options;
   if (o->created_directory_format)
-    verbose_output (stdout, o->created_directory_format, quote (dir));
+    prog_fprintf (stdout, o->created_directory_format, quote (dir));
 }

 /* Make ancestor directory DIR, whose last component is COMPONENT,
diff --git a/src/prog-fprintf.c b/src/prog-fprintf.c
new file mode 100644
index 0000000..85aceb6
--- /dev/null
+++ b/src/prog-fprintf.c
@@ -0,0 +1,37 @@
+/* prog-fprintf.c - common formating output functions and definitions
+   Copyright (C) 2008 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+#include <stdarg.h>
+#include <sys/types.h>
+
+#include "prog-fprintf.h"
+
+extern char *program_name;
+
+/* Display program name followed by variable list.
+   Used for e.g. verbose output */
+void
+prog_fprintf (FILE *fp, char const *fmt, ...)
+{
+  va_list ap;
+  fputs (program_name, fp);
+  fputs (": ", fp);
+  va_start (ap, fmt);
+  vfprintf (fp, fmt, ap);
+  va_end (ap);
+  fputc ('\n', fp);
+}
diff --git a/src/prog-fprintf.h b/src/prog-fprintf.h
new file mode 100644
index 0000000..d5c3d42
--- /dev/null
+++ b/src/prog-fprintf.h
@@ -0,0 +1,24 @@
+/* prog-fprintf.h - common formating output functions and definitions
+   Copyright (C) 2008 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef PROG_FPRINTF_H
+# define PROG_FPRINTF_H
+
+# include <stdio.h>
+
+extern void prog_fprintf (FILE *fp, char const *fmt, ...);
+
+#endif
diff --git a/src/rmdir.c b/src/rmdir.c
index bb1a0c8..5f7f541 100644
--- a/src/rmdir.c
+++ b/src/rmdir.c
@@ -1,6 +1,6 @@
 /* rmdir -- remove directories

-   Copyright (C) 90, 91, 1995-2002, 2004, 2005, 2006, 2007 Free Software
+   Copyright (C) 90, 91, 1995-2002, 2004-2008 Free Software
    Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
@@ -30,6 +30,7 @@

 #include "system.h"
 #include "error.h"
+#include "prog-fprintf.h"
 #include "quote.h"

 /* The official name of this program (e.g., no `g' prefix).  */
@@ -134,7 +135,7 @@ remove_parents (char *dir)

       /* Give a diagnostic for each attempted removal if --verbose.  */
       if (verbose)
-       error (0, 0, _("removing directory, %s"), quote (dir));
+       prog_fprintf (stdout, _("removing directory, %s"), quote (dir));

       ok = (rmdir (dir) == 0);

@@ -233,7 +234,7 @@ main (int argc, char **argv)

       /* Give a diagnostic for each attempted removal if --verbose.  */
       if (verbose)
-       error (0, 0, _("removing directory, %s"), dir);
+       prog_fprintf (stdout, _("removing directory, %s"), quote (dir));

       if (rmdir (dir) != 0)
        {
--
1.5.4.4.482.g5f904