Re: ln can't backup directories

2009-09-15 Thread Jim Meyering
Bert Wesarg wrote:

 Hi Bert,

 Thanks for the patch!
 I'll review it Monday or Tuesday.
 Thanks.

 In the mean time, do you feel like writing the remaining
 parts of such a change?


  - mention this in NEWS: put it under Changes in behavior or Improvements

  - add tests of cp and ln that exercise the new behavior, i.e.,
  that fail with the current/old tools and pass with your changes

  - possibly adjust doc/coreutils.texi
 Sure I will, I just wanted to post the patch for discussion, as it
 seems to trivial from you first reply.

Great!  Then I'll defer the review, and do it when
you send the complete patch.




Re: ln can't backup directories

2009-09-13 Thread Bert Wesarg
On Fri, Sep 11, 2009 at 16:15, Jim Meyering j...@meyering.net wrote:
 cp and mv work the same way: even with --backup, they refuse to move
 aside a destination directory.
Which does not seem to be correct, mv backups target directories:

$ mkdir a b
$ touch c
$ \mv --no-target-directory --backup=numbered b a
$ \mv --no-target-directory --backup=numbered c a
$ \ls
a  a.~1~  a.~2~
$

But mv could not be used for the backup() alias:

$ \mv --no-target-directory --backup=numbered a a
mv: `a' and `a' are the same file
$ \mv --no-target-directory --backup=numbered -f a a
mv: `a' and `a' are the same file

 Maybe someone will volunteer to do the work.

However, the trivial patch below solves the case for cp and ln and
still passes the test suit:

---8---
Subject: [PATCH] cp, ln: backup target directories

Signed-off-by: Bert Wesarg bert.wes...@googlemail.com

---
 src/copy.c |2 +-
 src/ln.c   |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index f3ff5a2..5b8fa2f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1434,7 +1434,7 @@ copy_internal (char const *src_name, char const *dst_name,
 {
   if (S_ISDIR (dst_sb.st_mode))
 {
-  if (x-move_mode  x-backup_type != no_backups)
+  if (x-backup_type != no_backups)
 {
   /* Moving a non-directory onto an existing
  directory is ok only with --backup.  */
diff --git a/src/ln.c b/src/ln.c
index 6a1dc32..50af02d 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -228,7 +228,7 @@ do_link (const char *source, const char *dest)

   if (dest_lstat_ok)
 {
-  if (S_ISDIR (dest_stats.st_mode))
+  if (S_ISDIR (dest_stats.st_mode)  backup_type == no_backups)
 {
   error (0, 0, _(%s: cannot overwrite directory), quote (dest));
   return false;




Re: ln can't backup directories

2009-09-13 Thread Jim Meyering
Bert Wesarg wrote:
 On Fri, Sep 11, 2009 at 16:15, Jim Meyering j...@meyering.net wrote:
 cp and mv work the same way: even with --backup, they refuse to move
 aside a destination directory.
 Which does not seem to be correct, mv backups target directories:

 $ mkdir a b
 $ touch c
 $ \mv --no-target-directory --backup=numbered b a
 $ \mv --no-target-directory --backup=numbered c a
 $ \ls
 a  a.~1~  a.~2~
 $

 But mv could not be used for the backup() alias:

 $ \mv --no-target-directory --backup=numbered a a
 mv: `a' and `a' are the same file
 $ \mv --no-target-directory --backup=numbered -f a a
 mv: `a' and `a' are the same file

 Maybe someone will volunteer to do the work.

 However, the trivial patch below solves the case for cp and ln and
 still passes the test suit:

 ---8---
 Subject: [PATCH] cp, ln: backup target directories

Hi Bert,

Thanks for the patch!
I'll review it Monday or Tuesday.
In the mean time, do you feel like writing the remaining
parts of such a change?

  - mention this in NEWS: put it under Changes in behavior or Improvements

  - add tests of cp and ln that exercise the new behavior, i.e.,
  that fail with the current/old tools and pass with your changes

  - possibly adjust doc/coreutils.texi




Re: ln can't backup directories

2009-09-13 Thread Bert Wesarg
 Hi Bert,

 Thanks for the patch!
 I'll review it Monday or Tuesday.
Thanks.

 In the mean time, do you feel like writing the remaining
 parts of such a change?


  - mention this in NEWS: put it under Changes in behavior or Improvements

  - add tests of cp and ln that exercise the new behavior, i.e.,
  that fail with the current/old tools and pass with your changes

  - possibly adjust doc/coreutils.texi
Sure I will, I just wanted to post the patch for discussion, as it
seems to trivial from you first reply.

Bert




Re: ln can't backup directories

2009-09-11 Thread Bert Wesarg
On Mon, Sep 7, 2009 at 18:25, Bert Wesarg bert.wes...@googlemail.com wrote:
 Hi all,

 I'm failing in creating a symlink where the destination is a directory
 and do a backup of this directory. This is what I tried:

 $ mkdir a
 $ ln -s --no-target-directory --backup=numbered b a
 ln: `a': cannot overwrite directory

 Looking at the code I can seen the reason, the backup rename() is
 never executed, because the check whether the target is a directory
 comes first.

 Any hints whether this is a bug of ln or how I can create the link
 while preserving the destination as an backup are more than welcomed.

 Thanks.
 Bert

Thanks to all for the release. But can someone please have a look at my problem?

Bert




Re: ln can't backup directories

2009-09-11 Thread Jim Meyering
Bert Wesarg wrote:
 On Mon, Sep 7, 2009 at 18:25, Bert Wesarg bert.wes...@googlemail.com wrote:
 Hi all,

 I'm failing in creating a symlink where the destination is a directory
 and do a backup of this directory. This is what I tried:

 $ mkdir a
 $ ln -s --no-target-directory --backup=numbered b a
 ln: `a': cannot overwrite directory

 Looking at the code I can seen the reason, the backup rename() is
 never executed, because the check whether the target is a directory
 comes first.

 Any hints whether this is a bug of ln or how I can create the link
 while preserving the destination as an backup are more than welcomed.

 Thanks.
 Bert

 Thanks to all for the release. But can someone please have a look at my 
 problem?

cp and mv work the same way: even with --backup, they refuse to move
aside a destination directory.

However, that's something that I've considered worth changing
for years.  I never got around to it.  If only for the testing
requirements, it will not be a trivial change, since for UI
consistency, it should affect all three at the same time.

Maybe someone will volunteer to do the work.




Re: ln can't backup directories

2009-09-11 Thread Bert Wesarg
On Fri, Sep 11, 2009 at 16:15, Jim Meyering j...@meyering.net wrote:
 Bert Wesarg wrote:
 On Mon, Sep 7, 2009 at 18:25, Bert Wesarg bert.wes...@googlemail.com wrote:
 Hi all,

 I'm failing in creating a symlink where the destination is a directory
 and do a backup of this directory. This is what I tried:

 $ mkdir a
 $ ln -s --no-target-directory --backup=numbered b a
 ln: `a': cannot overwrite directory

 Looking at the code I can seen the reason, the backup rename() is
 never executed, because the check whether the target is a directory
 comes first.

 Any hints whether this is a bug of ln or how I can create the link
 while preserving the destination as an backup are more than welcomed.

 Thanks.
 Bert

 Thanks to all for the release. But can someone please have a look at my 
 problem?

 cp and mv work the same way: even with --backup, they refuse to move
 aside a destination directory.

 However, that's something that I've considered worth changing
 for years.  I never got around to it.  If only for the testing
 requirements, it will not be a trivial change, since for UI
 consistency, it should affect all three at the same time.

 Maybe someone will volunteer to do the work.
Maybe a simpler solution would be a new dedicated tool which just
backups files, i.e. utilize the find_backup_file_name()/rename()
combination and call it 'mkbck'.

Regards
Bert




Re: ln can't backup directories

2009-09-11 Thread Jim Meyering
Bert Wesarg wrote:
 On Fri, Sep 11, 2009 at 16:15, Jim Meyering j...@meyering.net wrote:
 Bert Wesarg wrote:
 On Mon, Sep 7, 2009 at 18:25, Bert Wesarg bert.wes...@googlemail.com 
 wrote:
 Hi all,

 I'm failing in creating a symlink where the destination is a directory
 and do a backup of this directory. This is what I tried:

 $ mkdir a
 $ ln -s --no-target-directory --backup=numbered b a
 ln: `a': cannot overwrite directory

 Looking at the code I can seen the reason, the backup rename() is
 never executed, because the check whether the target is a directory
 comes first.

 Any hints whether this is a bug of ln or how I can create the link
 while preserving the destination as an backup are more than welcomed.

 Thanks.
 Bert

 Thanks to all for the release. But can someone please have a look at my 
 problem?

 cp and mv work the same way: even with --backup, they refuse to move
 aside a destination directory.

 However, that's something that I've considered worth changing
 for years.  I never got around to it.  If only for the testing
 requirements, it will not be a trivial change, since for UI
 consistency, it should affect all three at the same time.

 Maybe someone will volunteer to do the work.
 Maybe a simpler solution would be a new dedicated tool which just
 backups files, i.e. utilize the find_backup_file_name()/rename()
 combination and call it 'mkbck'.

We already have that ;-)
Here's a bash function named backup:

backup() { local i; for i in $@; do command cp -bf $i $i; done; }




Re: ln can't backup directories

2009-09-11 Thread Bert Wesarg
On Fri, Sep 11, 2009 at 17:00, Jim Meyering j...@meyering.net wrote:
 Bert Wesarg wrote:
 On Fri, Sep 11, 2009 at 16:15, Jim Meyering j...@meyering.net wrote:
 Bert Wesarg wrote:
 On Mon, Sep 7, 2009 at 18:25, Bert Wesarg bert.wes...@googlemail.com 
 wrote:
 Hi all,

 I'm failing in creating a symlink where the destination is a directory
 and do a backup of this directory. This is what I tried:

 $ mkdir a
 $ ln -s --no-target-directory --backup=numbered b a
 ln: `a': cannot overwrite directory

 Looking at the code I can seen the reason, the backup rename() is
 never executed, because the check whether the target is a directory
 comes first.

 Any hints whether this is a bug of ln or how I can create the link
 while preserving the destination as an backup are more than welcomed.

 Thanks.
 Bert

 Thanks to all for the release. But can someone please have a look at my 
 problem?

 cp and mv work the same way: even with --backup, they refuse to move
 aside a destination directory.

 However, that's something that I've considered worth changing
 for years.  I never got around to it.  If only for the testing
 requirements, it will not be a trivial change, since for UI
 consistency, it should affect all three at the same time.

 Maybe someone will volunteer to do the work.
 Maybe a simpler solution would be a new dedicated tool which just
 backups files, i.e. utilize the find_backup_file_name()/rename()
 combination and call it 'mkbck'.

 We already have that ;-)
 Here's a bash function named backup:

 backup() { local i; for i in $@; do command cp -bf $i $i; done; }
Which still does not work for directories?

Bert





Re: ln can't backup directories

2009-09-11 Thread Jim Meyering
Bert Wesarg wrote:
 Thanks to all for the release. But can someone please have a look at my 
 problem?

 cp and mv work the same way: even with --backup, they refuse to move
 aside a destination directory.

 However, that's something that I've considered worth changing
 for years.  I never got around to it.  If only for the testing
 requirements, it will not be a trivial change, since for UI
 consistency, it should affect all three at the same time.

 Maybe someone will volunteer to do the work.
 Maybe a simpler solution would be a new dedicated tool which just
 backups files, i.e. utilize the find_backup_file_name()/rename()
 combination and call it 'mkbck'.

 We already have that ;-)
 Here's a bash function named backup:

 backup() { local i; for i in $@; do command cp -bf $i $i; done; }
 Which still does not work for directories?

Correct.  It uses the same underlying code.

My point is that making the tools do what I suggest
will give you the tool (that alias) you'd like.
No need to duplicate all of copy.c.




ln can't backup directories

2009-09-07 Thread Bert Wesarg
Hi all,

I'm failing in creating a symlink where the destination is a directory
and do a backup of this directory. This is what I tried:

$ mkdir a
$ ln -s --no-target-directory --backup=numbered b a
ln: `a': cannot overwrite directory

Looking at the code I can seen the reason, the backup rename() is
never executed, because the check whether the target is a directory
comes first.

Any hints whether this is a bug of ln or how I can create the link
while preserving the destination as an backup are more than welcomed.

Thanks.
Bert