builtin-clone does not fallback to copy when link fails

When cloning with the new builtin-clone, if the src repo is not
on the same disk as the dest repo, cloning fails. This is because
hard linking does not fall back to copying like the shell version
did.

The shell version also made a distinction between defaulting to
hard linking and an explicit request to hard link. In the latter
case it would not fall back to copying, but would die.

I'll also mention that the 'use_local_hardlinks' variable in cmd_clone
is not used. It looks like there was some initial thought that this
variable would be used and then an alternative solution was found
where option_no_hardlinks was accessed directly. So use_local_hardlinks
should probably be cleaned out.

-brandon

Brandon Casey wrote:

When cloning with the new builtin-clone, if the src repo is not
on the same disk as the dest repo, cloning fails. This is because
hard linking does not fall back to copying like the shell version
did.

The shell version also made a distinction between defaulting to
hard linking and an explicit request to hard link. In the latter
case it would not fall back to copying, but would die.

Something like this (if not too ugly) might do the trick:

diff --git a/builtin-clone.c b/builtin-clone.c
index 8713128..1062371 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c

@@ -208,11 +208,17 @@ static void copy_or_link_directory(char *src, char *dest)
 		if (unlink(dest) && errno != ENOENT)
 			die("failed to unlink %s\n", dest);
 		if (option_no_hardlinks) {
+FALLBACK_TO_COPY:
 			if (copy_file(dest, src, 0666))
 				die("failed to copy file to %s\n", dest);
 		} else {
-			if (link(src, dest))
+			if (link(src, dest)) {
+				if (errno == EXDEV && !option_local) {
+				    	option_no_hardlinks = 1;
+					goto FALLBACK_TO_COPY;
+				}
 				die("failed to create link %s\n", dest);
+			}
 		}
 	}
 }

Brandon Casey wrote:

Maybe EPERM should be tested also.

Or maybe it should be simplified to:

	if (!option_local)
		goto FALLBACK_TO_COPY;

-brandon

On Tue, 20 May 2008, Brandon Casey wrote:

Brandon Casey wrote:

quoted

When cloning with the new builtin-clone, if the src repo is not
on the same disk as the dest repo, cloning fails. This is because
hard linking does not fall back to copying like the shell version
did.

The shell version also made a distinction between defaulting to
hard linking and an explicit request to hard link. In the latter
case it would not fall back to copying, but would die.

I think that the shell version's behavior changed at some point, too. I 
think I tried at some point to figure out exactly what the specified 
behavior was, and couldn't come up with anything that entirely matched.

Something like this (if not too ugly) might do the trick:

I think that's good behavior, but it's kind of ugly. How about:

-----
commit 83afef6a159365c1b9a7a1961cb4c95df24fbcac
Author: Daniel Barkalow [off-list ref]
Date:   Tue May 20 14:15:14 2008 -0400

    Fall back to copying if hardlinking fails
    
    Note that it stops trying hardlinks if any fail.
    
    Signed-off-by: Daniel Barkalow [off-list ref]

diff --git a/builtin-clone.c b/builtin-clone.c
index 8713128..42633ae 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c

@@ -207,13 +207,15 @@ static void copy_or_link_directory(char *src, char *dest)
 
 		if (unlink(dest) && errno != ENOENT)
 			die("failed to unlink %s\n", dest);
-		if (option_no_hardlinks) {
-			if (copy_file(dest, src, 0666))
-				die("failed to copy file to %s\n", dest);
-		} else {
-			if (link(src, dest))
+		if (!option_no_hardlinks) {
+			if (!link(src, dest))
+				continue;
+			if (option_local)
 				die("failed to create link %s\n", dest);
+			option_no_hardlinks = 1;
 		}
+		if (copy_file(dest, src, 0666))
+			die("failed to copy file to %s\n", dest);
 	}
 }
 

Daniel Barkalow wrote:

actually, I don't like that buried 'continue' either, but it looks like it
would work just the same... It does.

-brandon

Hi,

On Tue, 20 May 2008, Daniel Barkalow wrote:

How about:

-----
commit 83afef6a159365c1b9a7a1961cb4c95df24fbcac
Author: Daniel Barkalow [off-list ref]
Date:   Tue May 20 14:15:14 2008 -0400

    Fall back to copying if hardlinking fails
    
    Note that it stops trying hardlinks if any fail.
    
    Signed-off-by: Daniel Barkalow [off-list ref]

I like it.

Ciao,
Dscho