Re: builtin-clone does not fallback to copy when link fails
- From
Brandon Casey <casey@nrlssc.navy.mil>
- Date
- May 20, 2008, 18:44 UTC
- Message-ID
- <2W5fKgEzyIWieZBezujpgWKgshbyTXW2uBoRhp11gtOL5nAZalnVfg@cipher.nrlssc.navy.mil>
- In-Reply-To
- <alpine.LNX.1.00.0805201311180.19665@iabervon.org>
Daniel Barkalow wrote:
Show 55 quoted lines
> On Tue, 20 May 2008, Brandon Casey wrote:
>
>> 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.
>
> 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 <barkalow@iabervon.org>
> 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 <barkalow@iabervon.org>
>
> 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);
> }
> }actually, I don't like that buried 'continue' either, but it looks like it would work just the same... It does.
-brandon