Re: Patch for "mv -s" option

[Brendan Byrd/SineSwiper]

Brendan Byrd/SineSwiper wrote:

> So, it looks like it's expected behavior.  One annoying thing I found 
> with that, is that it says that error message AFTER it's moved and tried 
> to create a symlink with the file.  So you end up with the file already 
> moved, and a broken symlink in tmp/a/b called [f -> ./f].
>
> This may be related to the un-implemented (or rather un-ported from 
> copy.c) "goto un_backup" routine.  At the very least, it should exit 
> before the symlink call.  I'm going to try to fix the original problem 
> by moving that current directory check to the beginning of the do_move sub.

Attached second patch.  Fixed bug with error after mv/symlink, which 
removed the need for one of the "goto un_backup" lines.  The only 
problem I can see now is the second one, to remove the backups if the 
symlink fails.  (It's highly unlikely, since the file was just moved out 
of that space.  Even if the move fails, there's still a !fail check in 
place before the symlink.)

Also, in this case, should it move the file back in place, or just not 
create the symlink?  If the latter, it may be better to forgo the goto 
line and replace with a "return fail;" one (which is already currently 
in place now).

--
Brendan Byrd/SineSwiper <address@hidden>
Computer techie, Perl hacker, and all-purpose Internet guru
Resonator Software (http://www.ResonatorSoft.org/)

*** src/mv.c.orig       2004-11-19 02:59:43.000000000 -0500
--- src/mv.c    2004-11-19 04:19:08.000000000 -0500
***************
*** 82,87 ****
--- 82,88 ----
    {"interactive", no_argument, NULL, 'i'},
    {"reply", required_argument, NULL, REPLY_OPTION},
    {"strip-trailing-slashes", no_argument, NULL, 
STRIP_TRAILING_SLASHES_OPTION},
+   {"symbolic-link", no_argument, NULL, 's'},
    {"suffix", required_argument, NULL, 'S'},
    {"target-directory", required_argument, NULL, TARGET_DIRECTORY_OPTION},
    {"update", no_argument, NULL, 'u'},
***************
*** 174,180 ****
        hash_init ();
      }
  
!   fail = copy (source, dest, 0, x, &copy_into_self, &rename_succeeded);
  
    if (!fail)
      {
--- 175,228 ----
        hash_init ();
      }
  
! #ifdef S_ISLNK
!   if (x->symbolic_link)
!     {
!       /* Symbolic link checks put here before we actually do anything... */
!       const char *src_path = dest;    /* Reversed variables mainly for 
readability */
!       const char *dst_path = source;  /* since this was yanked from copy.c */
! 
!       if (*src_path != '/')
!         {
!           /* Check that DST_PATH denotes a file in the current directory.  */
!           struct stat dot_sb;
!           struct stat dst_parent_sb;
!           char *dst_parent;
!           int in_current_dir;
! 
!           dst_parent = dir_name (dst_path);
!           if (dst_parent == NULL)
!             xalloc_die ();
! 
!           in_current_dir = (STREQ (".", dst_parent)
!                             /* If either stat call fails, it's ok not to 
report
!                                the failure and say dst_path is in the current
!                                directory.  Other things will fail later.  */
!                             || stat (".", &dot_sb)
!                             || stat (dst_parent, &dst_parent_sb)
!                             || SAME_INODE (dot_sb, dst_parent_sb));
!           free (dst_parent);
! 
!           if (! in_current_dir)
!             {
!               error (0, 0,
!            _("%s: can make relative symbolic links only in current 
directory"),
!                      quote (dst_path));
!               return 1;
!             }
!         }
! 
!       /* If we are creating symbolic links, create them -after- the move, not
!          within the copy. */
!       static struct cp_options x_tmp;
!       x_tmp = *x;
!       x_tmp.symbolic_link = 0;
! 
!       fail = copy (source, dest, 0, &x_tmp, &copy_into_self, 
&rename_succeeded);
!     }
!   else
! #endif
!     fail = copy (source, dest, 0, x, &copy_into_self, &rename_succeeded);
  
    if (!fail)
      {
***************
*** 241,246 ****
--- 289,311 ----
          if (status == RM_ERROR)
            fail = 1;
        }
+ 
+ #ifdef S_ISLNK
+       if (x->symbolic_link && !fail)
+         {
+           fail = symlink (dest, source);  /* Remember that these are reversed 
*/
+           if (fail)
+             {
+               error (0, errno, _("cannot create symbolic link %s to %s"),
+                      quote_n (0, source), quote_n (1, dest));
+               // goto un_backup;  /* FIXME: backup undo not handled */
+               return fail;
+             }
+ 
+           return 0;
+         }
+ #endif
+ 
      }
  
    return fail;
***************
*** 328,333 ****
--- 393,399 ----
                                   existing destination file\n\
        --strip-trailing-slashes remove any trailing slashes from each SOURCE\n\
                                   argument\n\
+   -s, --symbolic-link          make symbolic links after moving\n\
    -S, --suffix=SUFFIX          override the usual backup suffix\n\
  "), stdout);
        fputs (_("\
***************
*** 388,394 ****
  
    errors = 0;
  
!   while ((c = getopt_long (argc, argv, "bfiuvS:V:", long_options, NULL)) != 
-1)
      {
        switch (c)
        {
--- 454,460 ----
  
    errors = 0;
  
!   while ((c = getopt_long (argc, argv, "bfiuvsS:V:", long_options, NULL)) != 
-1)
      {
        switch (c)
        {
***************
*** 429,434 ****
--- 495,507 ----
        case 'v':
          x.verbose = 1;
          break;
+         case 's':
+ #ifdef S_ISLNK
+           x.symbolic_link = 1;
+ #else
+           error (1, 0, _("symbolic links are not supported on this system"));
+ #endif
+           break;
        case 'S':
          make_backups = 1;
          backup_suffix_string = optarg;
*** doc/coreutils.texi.orig      2004-11-15 07:43:51.000000000 -0500
--- doc/coreutils.texi  2004-11-15 08:00:26.000000000 -0500
***************
*** 6774,6779 ****
--- 6774,6790 ----
  Specify @option{--reply=query} to make @command{mv} prompt the user
  about each existing destination file.

+ @item -s
+ @itemx --symbolic-link
+ @opindex -s
+ @opindex --symbolic-link
+ @cindex symbolic links, moving with
+ Make symbolic links (pointing to the destination) after the file has been
+ moved.  (This is essentially the "opposite" of the @command{cp} -s option.)
+ All source file names must be absolute (starting with @samp{/}) unless the
+ destination files are in the current directory.  This option merely
+ results in an error message on systems that do not support symbolic links.
+
  @item -u
  @itemx --update
  @opindex -u