Hello Jim,

new version of patch is attached. Options --backup and --no-clobber are now 
mutually exclusive.


Kamil


On Thursday 08 January 2009 08:36:13 Jim Meyering wrote:
> Eric Blake <e...@byu.net> wrote:
> > According to Jim Meyering on 1/7/2009 12:27 PM:
> >>>> Anyone else have a preference?
> >>>
> >>> Now the option is --no-clobber in attached patch. Anyway the change is
> >>> trivial. No problem to change it if there will be a consensus
> >>> for --no-overwrite or something else.
> >>
> >> Thanks!  Almost done, I think.
> >> Unless anyone pipes up about the choice of long option name.
> >
> > I like --no-clobber, particularly since it is similar to the bash (and
> > tcsh) noclobber shell option.
> >
> >> What do you think about making cp/mv fail if -n and --backup are used
> >> together? Then there would be no risk of misunderstanding.
> >
> > Seems reasonable to me.
> >
> >> +++ b/NEWS
> >> @@ -4,6 +4,9 @@ GNU coreutils NEWS
> >
> > -*- outline -*-
> >
> >>  ** New features
> >>
> >> +  cp and mv accept a new option, --no-clobber (-n): silently refrain
> >> +  from overwriting any existing destination file
> >
> > Missing period.
>
> Thanks for the review.
>
> >> +                                 opened, remove it and try again (The
> >
> > -f option\n\
> >
> >> +                                 is redundant if the -n option is
> >> used.)\n\ +  -i, --interactive            prompt before overwrite  (The
> >> -i option\n\ +                                 overrides a previous -n
> >> option.)\n\ -H                           follow command-line symbolic
> >> links in
> >
> > SOURCE\n\
> >
> >>  "), stdout);
> >>        fputs (_("\
> >> @@ -176,6 +179,8 @@ Mandatory arguments to long options are mandatory
> >
> > for short options too.\n\
> >
> >>    -L, --dereference            always follow symbolic links in
> >> SOURCE\n\ "), stdout);
> >>        fputs (_("\
> >> +  -n, --no-clobber             do not overwrite an existing file  (The
> >
> > -n\n\
> >
> >> +                                 option overrides a previous -i
> >> option.)\n\
> >
> > The formatting is not very consistent here (are these full sentences,
> > with capitalization, periods, and two spaces, or just phrases?)
>
> It's a little bit unusual to insert text into the option list,
> but works for chown --help, so I suggest doing it here, too:
>
> diff --git a/src/mv.c b/src/mv.c
> index 635c4e0..d8ca50b 100644
> --- a/src/mv.c
> +++ b/src/mv.c
> @@ -1,5 +1,5 @@
>  /* mv -- move or rename files
> -   Copyright (C) 86, 89, 90, 91, 1995-2008 Free Software Foundation, Inc.
> +   Copyright (C) 86, 89, 90, 91, 1995-2009 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
> @@ -295,13 +295,10 @@ Mandatory arguments to long options are mandatory for
> short options too.\n\ fputs (_("\
>        --backup[=CONTROL]       make a backup of each existing destination
> file\n\ -b                           like --backup but does not accept an
> argument\n\ -  -f, --force                  do not prompt before
> overwriting  (The -f option\n\ -                                 overrides
> any previous -i or -n options.)\n\ -  -i, --interactive            prompt
> before overwrite  (The -i option\n\ -                                
> overrides any previous -f or -n options.)\n\ -  -n, --no-replace           
>  do not overwrite an existing file  (The -n\n\ -                           
>      option overrides any previous -f or -i\n\ -                           
>      options.)\n\
> +  -f, --force                  do not prompt before overwriting\n\
> +  -i, --interactive            prompt before overwrite\n\
> +  -n, --no-replace             do not overwrite an existing file\n\
> +If you specify more than one of -i, -f, -n, only the final one takes
> effect.\n\ "), stdout);
>        fputs (_("\
>        --strip-trailing-slashes  remove any trailing slashes from each
> SOURCE\n\
>
> >> +  case 'n':
> >> +    x.interactive = I_ALWAYS_NO;
> >> +    break;
> >
> > Does this actually do the right thing, or does it only prevent overwrites
> > at spots where, without arguments, a prompt would be issued?  In other
> > words, is this particular implementation reinventing the same problems as
> > the deprecated --reply=no?
>
> Kamil's test additions appear to cover most cases.
> Once -n --backup evokes failure, it'd be nice to exercise that, too.
>
>
> _______________________________________________
> Bug-coreutils mailing list
> Bug-coreutils@gnu.org
> http://lists.gnu.org/mailman/listinfo/bug-coreutils


From d73bf5792e5f679aa806a85134271d110a8d3f82 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdu...@redhat.com>
Date: Tue, 13 Jan 2009 18:35:00 +0100
Subject: [PATCH] cp/mv: add --no-clobber (-n) option to not overwrite target

* src/cp.c (usage): Show new option -n in --help.
(main): Handle new option -n.
* src/mv.c (usage): Show new option -n in --help.
(main): Handle new option -n.
* doc/coreutils.texi: Document new cp/mv option -n.
* tests/cp/cp-i: Add tests for -f, -i and -n options.
* tests/mv/mv-n: New test for mv -n.
* tests/Makefile.am: Add test mv/mv-n to the list.
* NEWS: Mention the change.
---
 NEWS               |    3 ++
 doc/coreutils.texi |   29 +++++++++++++++++++++++-
 src/cp.c           |   24 ++++++++++++++++---
 src/mv.c           |   17 ++++++++++++-
 tests/Makefile.am  |    1 +
 tests/cp/cp-i      |   36 ++++++++++++++++++++++++++++++
 tests/mv/mv-n      |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 165 insertions(+), 7 deletions(-)
 create mode 100755 tests/mv/mv-n

diff --git a/NEWS b/NEWS
index f605330..f1b383e 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** New features
 
+  cp and mv accept a new option, --no-clobber (-n): silently refrain
+  from overwriting any existing destination file
+
   dd accepts iflag=cio and oflag=cio to open the file in CIO (concurrent I/O)
   mode where this feature is available.
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 1cc237c..d8df107 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7314,6 +7314,9 @@ description of @option{--remove-destination}.
 This option is independent of the @option{--interactive} or
 @option{-i} option: neither cancels the effect of the other.
 
+This option is redundant if the @option{--no-clobber} or @option{-n} option is
+used.
+
 @item -H
 @opindex -H
 If a command line argument specifies a symbolic link, then copy the
@@ -7326,7 +7329,8 @@ via recursive traversal.
 @opindex -i
 @opindex --interactive
 When copying a file other than a directory, prompt whether to
-overwrite an existing destination file.
+overwrite an existing destination file. The @option{-i} option overrides
+a previous @option{-n} option.
 
 @item -l
 @itemx --link
@@ -7340,6 +7344,14 @@ Make hard links instead of copies of non-directories.
 @opindex --dereference
 Follow symbolic links when copying from them.
 
+...@item -n
+...@itemx --no-clobber
+...@opindex -n
+...@opindex --no-clobber
+Do not overwrite an existing file. The @option{-n} option overrides a previous
+...@option{-i} option. This option is mutually exclusive with @option{-b} or
+...@option{--backup} option.
+
 @item -P
 @itemx --no-dereference
 @opindex -P
@@ -8099,6 +8111,11 @@ The program accepts the following options.  Also see @ref{Common options}.
 @opindex --force
 @cindex prompts, omitting
 Do not prompt the user before removing a destination file.
+...@macro mvOptsIfn
+If you specify more than one of the @option{-i}, @option{-f}, @option{-n}
+options, only the final one takes effect.
+...@end macro
+...@mvoptsifn
 
 @item -i
 @itemx --interactive
@@ -8108,6 +8125,16 @@ Do not prompt the user before removing a destination file.
 Prompt whether to overwrite each existing destination file, regardless
 of its permissions.
 If the response is not affirmative, the file is skipped.
+...@mvoptsifn
+
+...@item -n
+...@itemx --no-clobber
+...@opindex -n
+...@opindex --no-clobber
+...@cindex prompts, omitting
+Do not overwrite an existing file.
+...@mvoptsifn
+This option is mutually exclusive with @option{-b} or @option{--backup} option.
 
 @item -u
 @itemx --update
diff --git a/src/cp.c b/src/cp.c
index 8e34965..191f73e 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -1,5 +1,5 @@
 /* cp.c  -- file copying (main routines)
-   Copyright (C) 89, 90, 91, 1995-2008 Free Software Foundation, Inc.
+   Copyright (C) 89, 90, 91, 1995-2009 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
@@ -116,6 +116,7 @@ static struct option const long_opts[] =
   {"force", no_argument, NULL, 'f'},
   {"interactive", no_argument, NULL, 'i'},
   {"link", no_argument, NULL, 'l'},
+  {"no-clobber", no_argument, NULL, 'n'},
   {"no-dereference", no_argument, NULL, 'P'},
   {"no-preserve", required_argument, NULL, NO_PRESERVE_ATTRIBUTES_OPTION},
   {"no-target-directory", no_argument, NULL, 'T'},
@@ -167,8 +168,10 @@ Mandatory arguments to long options are mandatory for short options too.\n\
 "), stdout);
       fputs (_("\
   -f, --force                  if an existing destination file cannot be\n\
-                                 opened, remove it and try again\n\
-  -i, --interactive            prompt before overwrite\n\
+                                 opened, remove it and try again (redundant if\n\
+                                 the -n option is used)\n\
+  -i, --interactive            prompt before overwrite (overrides a previous -n\n\
+                                  option)\n\
   -H                           follow command-line symbolic links in SOURCE\n\
 "), stdout);
       fputs (_("\
@@ -176,6 +179,8 @@ Mandatory arguments to long options are mandatory for short options too.\n\
   -L, --dereference            always follow symbolic links in SOURCE\n\
 "), stdout);
       fputs (_("\
+  -n, --no-clobber             do not overwrite an existing file (overrides\n\
+                                 a previous -i option)\n\
   -P, --no-dereference         never follow symbolic links in SOURCE\n\
 "), stdout);
       fputs (_("\
@@ -894,7 +899,7 @@ main (int argc, char **argv)
      we'll actually use backup_suffix_string.  */
   backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
 
-  while ((c = getopt_long (argc, argv, "abdfHilLprst:uvxPRS:T",
+  while ((c = getopt_long (argc, argv, "abdfHilLnprst:uvxPRS:T",
 			   long_opts, NULL))
 	 != -1)
     {
@@ -950,6 +955,10 @@ main (int argc, char **argv)
 	  x.dereference = DEREF_ALWAYS;
 	  break;
 
+	case 'n':
+	  x.interactive = I_ALWAYS_NO;
+	  break;
+
 	case 'P':
 	  x.dereference = DEREF_NEVER;
 	  break;
@@ -1050,6 +1059,13 @@ main (int argc, char **argv)
       usage (EXIT_FAILURE);
     }
 
+  if (make_backups && x.interactive == I_ALWAYS_NO)
+    {
+      error (0, 0,
+	     _("options --backup and --no-clobber are mutually exclusive"));
+      usage (EXIT_FAILURE);
+    }
+
   if (backup_suffix_string)
     simple_backup_suffix = xstrdup (backup_suffix_string);
 
diff --git a/src/mv.c b/src/mv.c
index 98309a4..a5ab95d 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -1,5 +1,5 @@
 /* mv -- move or rename files
-   Copyright (C) 86, 89, 90, 91, 1995-2008 Free Software Foundation, Inc.
+   Copyright (C) 86, 89, 90, 91, 1995-2009 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
@@ -62,6 +62,7 @@ static struct option const long_options[] =
   {"backup", optional_argument, NULL, 'b'},
   {"force", no_argument, NULL, 'f'},
   {"interactive", no_argument, NULL, 'i'},
+  {"no-clobber", no_argument, NULL, 'n'},
   {"no-target-directory", no_argument, NULL, 'T'},
   {"strip-trailing-slashes", no_argument, NULL, STRIP_TRAILING_SLASHES_OPTION},
   {"suffix", required_argument, NULL, 'S'},
@@ -296,6 +297,8 @@ Mandatory arguments to long options are mandatory for short options too.\n\
   -b                           like --backup but does not accept an argument\n\
   -f, --force                  do not prompt before overwriting\n\
   -i, --interactive            prompt before overwrite\n\
+  -n, --no-clobber             do not overwrite an existing file\n\
+If you specify more than one of -i, -f, -n, only the final one takes effect.\n\
 "), stdout);
       fputs (_("\
       --strip-trailing-slashes  remove any trailing slashes from each SOURCE\n\
@@ -358,7 +361,7 @@ main (int argc, char **argv)
      we'll actually use backup_suffix_string.  */
   backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
 
-  while ((c = getopt_long (argc, argv, "bfit:uvS:T", long_options, NULL))
+  while ((c = getopt_long (argc, argv, "bfint:uvS:T", long_options, NULL))
 	 != -1)
     {
       switch (c)
@@ -374,6 +377,9 @@ main (int argc, char **argv)
 	case 'i':
 	  x.interactive = I_ASK_USER;
 	  break;
+	case 'n':
+	  x.interactive = I_ALWAYS_NO;
+	  break;
 	case STRIP_TRAILING_SLASHES_OPTION:
 	  remove_trailing_slashes = true;
 	  break;
@@ -446,6 +452,13 @@ main (int argc, char **argv)
 	       quote (file[n_files - 1]));
     }
 
+  if (make_backups && x.interactive == I_ALWAYS_NO)
+    {
+      error (0, 0,
+	     _("options --backup and --no-clobber are mutually exclusive"));
+      usage (EXIT_FAILURE);
+    }
+
   if (backup_suffix_string)
     simple_backup_suffix = xstrdup (backup_suffix_string);
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8dbbe9f..6dce9cd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -379,6 +379,7 @@ TESTS =						\
   mv/into-self-3				\
   mv/into-self-4				\
   mv/leak-fd					\
+  mv/mv-n					\
   mv/mv-special-1				\
   mv/no-target-dir				\
   mv/part-fail					\
diff --git a/tests/cp/cp-i b/tests/cp/cp-i
index 94284be..36bb758 100755
--- a/tests/cp/cp-i
+++ b/tests/cp/cp-i
@@ -31,4 +31,40 @@ fail=0
 # coreutils 6.2 cp would neglect to prompt in this case.
 echo n | cp -iR a b 2>/dev/null || fail=1
 
+# test miscellaneous combinations of -f -i -n parameters
+touch c d || framework_failure
+echo "\`c' -> \`d'" > out_copy
+> out_empty
+
+# ask for overwrite, answer no
+echo n | cp -vi  c d 2>/dev/null > out1 || fail=1
+compare out1 out_empty || fail=1
+
+# ask for overwrite, answer yes
+echo y | cp -vi  c d 2>/dev/null > out2 || fail=1
+compare out2 out_copy  || fail=1
+
+# -i wins over -n
+echo y | cp -vni c d 2>/dev/null > out3 || fail=1
+compare out3 out_copy  || fail=1
+
+# -n wins over -i
+echo y | cp -vin c d 2>/dev/null > out4 || fail=1
+compare out4 out_empty || fail=1
+
+# ask for overwrite, answer yes
+echo y | cp -vfi c d 2>/dev/null > out5 || fail=1
+compare out5 out_copy  || fail=1
+
+# do not ask, prevent from overwrite
+echo n | cp -vfn c d 2>/dev/null > out6 || fail=1
+compare out6 out_empty || fail=1
+
+# do not ask, prevent from overwrite
+echo n | cp -vnf c d 2>/dev/null > out7 || fail=1
+compare out7 out_empty || fail=1
+
+# options --backup and --no-clobber are mutually exclusive
+cp -bn c d 2>/dev/null && fail=1
+
 Exit $fail
diff --git a/tests/mv/mv-n b/tests/mv/mv-n
new file mode 100755
index 0000000..1b44fda
--- /dev/null
+++ b/tests/mv/mv-n
@@ -0,0 +1,62 @@
+#!/bin/sh
+# Test whether mv -n works as documented (not overwrite target).
+
+# Copyright (C) 2006-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/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  mv --version
+fi
+
+. $srcdir/test-lib.sh
+
+fail=0
+
+# test miscellaneous combinations of -f -i -n parameters
+touch a b || framework_failure
+echo "\`a' -> \`b'" > out_move
+> out_empty
+
+# ask for overwrite, answer no
+touch a b || framework_failure
+echo n | mv -vi a b 2>/dev/null > out1 || fail=1
+compare out1 out_empty || fail=1
+
+# ask for overwrite, answer yes
+touch a b || framework_failure
+echo y | mv -vi a b 2>/dev/null > out2 || fail=1
+compare out2 out_move || fail=1
+
+# -n wins (as the last option)
+touch a b || framework_failure
+echo y | mv -vin a b 2>/dev/null > out3 || fail=1
+compare out3 out_empty || fail=1
+
+# -n wins (as the last option)
+touch a b || framework_failure
+echo y | mv -vfn a b 2>/dev/null > out4 || fail=1
+compare out4 out_empty || fail=1
+
+# -n wins (as the last option)
+touch a b || framework_failure
+echo y | mv -vifn a b 2>/dev/null > out5 || fail=1
+compare out5 out_empty || fail=1
+
+# options --backup and --no-clobber are mutually exclusive
+touch a || framework_failure
+mv -bn a b 2>/dev/null && fail=1
+
+Exit $fail
-- 
1.5.4.3

_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Reply via email to