{"thread":{"id":"13588","subject":"builtin-clone does not fallback to copy when link fails","startedAt":"2008-05-20T16:28:52Z","lastAt":"2008-05-20T21:48:30Z","messageCount":6,"participants":["Brandon Casey","Daniel Barkalow","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"77325","messageId":"oguFi9b5ZAq84dsDzWpm0tpI_xzucMxL23YhxIjDS5NTdpslAnzo6w@cipher.nrlssc.navy.mil","threadId":"13588","inReplyTo":null,"subject":"builtin-clone does not fallback to copy when link fails","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-05-20T16:28:52Z","receivedAt":"2008-05-20T16:28:52Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\n\nWhen cloning with the new builtin-clone, if the src repo is not\non the same disk as the dest repo, cloning fails. This is because\nhard linking does not fall back to copying like the shell version\ndid.\n\nThe shell version also made a distinction between defaulting to\nhard linking and an explicit request to hard link. In the latter\ncase it would not fall back to copying, but would die.\n\nI'll also mention that the 'use_local_hardlinks' variable in cmd_clone\nis not used. It looks like there was some initial thought that this\nvariable would be used and then an alternative solution was found\nwhere option_no_hardlinks was accessed directly. So use_local_hardlinks\nshould probably be cleaned out.\n\n-brandon\n"},{"id":"77326","messageId":"C6tFzFdDycYRRcjxtVG00CVX-Nsu7-UblPCaZmTxmWIUTFAsgReYbQ@cipher.nrlssc.navy.mil","threadId":"13588","inReplyTo":"oguFi9b5ZAq84dsDzWpm0tpI_xzucMxL23YhxIjDS5NTdpslAnzo6w@cipher.nrlssc.navy.mil","subject":"Re: builtin-clone does not fallback to copy when link fails","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-05-20T16:45:14Z","receivedAt":"2008-05-20T16:45:14Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\nBrandon Casey wrote:\n> \n> When cloning with the new builtin-clone, if the src repo is not\n> on the same disk as the dest repo, cloning fails. This is because\n> hard linking does not fall back to copying like the shell version\n> did.\n> \n> The shell version also made a distinction between defaulting to\n> hard linking and an explicit request to hard link. In the latter\n> case it would not fall back to copying, but would die.\n\nSomething like this (if not too ugly) might do the trick:\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 8713128..1062371 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -208,11 +208,17 @@ static void copy_or_link_directory(char *src, char *dest)\n \t\tif (unlink(dest) && errno != ENOENT)\n \t\t\tdie(\"failed to unlink %s\\n\", dest);\n \t\tif (option_no_hardlinks) {\n+FALLBACK_TO_COPY:\n \t\t\tif (copy_file(dest, src, 0666))\n \t\t\t\tdie(\"failed to copy file to %s\\n\", dest);\n \t\t} else {\n-\t\t\tif (link(src, dest))\n+\t\t\tif (link(src, dest)) {\n+\t\t\t\tif (errno == EXDEV && !option_local) {\n+\t\t\t\t \toption_no_hardlinks = 1;\n+\t\t\t\t\tgoto FALLBACK_TO_COPY;\n+\t\t\t\t}\n \t\t\t\tdie(\"failed to create link %s\\n\", dest);\n+\t\t\t}\n \t\t}\n \t}\n }\n"},{"id":"77329","messageId":"huU-0mKnZACgQMuzg8VRUnBz_T6J3OdrjM9uWBVOZPtcbOk9EkBYlA@cipher.nrlssc.navy.mil","threadId":"13588","inReplyTo":"C6tFzFdDycYRRcjxtVG00CVX-Nsu7-UblPCaZmTxmWIUTFAsgReYbQ@cipher.nrlssc.navy.mil","subject":"Re: builtin-clone does not fallback to copy when link fails","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-05-20T17:22:04Z","receivedAt":"2008-05-20T17:22:04Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Brandon Casey wrote:\n> Brandon Casey wrote:\n>> When cloning with the new builtin-clone, if the src repo is not\n>> on the same disk as the dest repo, cloning fails. This is because\n>> hard linking does not fall back to copying like the shell version\n>> did.\n>>\n>> The shell version also made a distinction between defaulting to\n>> hard linking and an explicit request to hard link. In the latter\n>> case it would not fall back to copying, but would die.\n> \n> Something like this (if not too ugly) might do the trick:\n> \n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index 8713128..1062371 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -208,11 +208,17 @@ static void copy_or_link_directory(char *src, char *dest)\n> \t\tif (unlink(dest) && errno != ENOENT)\n> \t\t\tdie(\"failed to unlink %s\\n\", dest);\n> \t\tif (option_no_hardlinks) {\n> +FALLBACK_TO_COPY:\n> \t\t\tif (copy_file(dest, src, 0666))\n> \t\t\t\tdie(\"failed to copy file to %s\\n\", dest);\n> \t\t} else {\n> -\t\t\tif (link(src, dest))\n> +\t\t\tif (link(src, dest)) {\n> +\t\t\t\tif (errno == EXDEV && !option_local) {\n\nMaybe EPERM should be tested also.\n\nOr maybe it should be simplified to:\n\n\tif (!option_local)\n\t\tgoto FALLBACK_TO_COPY;\n\n-brandon\n"},{"id":"77330","messageId":"alpine.LNX.1.00.0805201311180.19665@iabervon.org","threadId":"13588","inReplyTo":"C6tFzFdDycYRRcjxtVG00CVX-Nsu7-UblPCaZmTxmWIUTFAsgReYbQ@cipher.nrlssc.navy.mil","subject":"Re: builtin-clone does not fallback to copy when link fails","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-05-20T18:16:56Z","receivedAt":"2008-05-20T18:16:56Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 20 May 2008, Brandon Casey wrote:\n\n> Brandon Casey wrote:\n> > \n> > When cloning with the new builtin-clone, if the src repo is not\n> > on the same disk as the dest repo, cloning fails. This is because\n> > hard linking does not fall back to copying like the shell version\n> > did.\n> > \n> > The shell version also made a distinction between defaulting to\n> > hard linking and an explicit request to hard link. In the latter\n> > case it would not fall back to copying, but would die.\n\nI think that the shell version's behavior changed at some point, too. I \nthink I tried at some point to figure out exactly what the specified \nbehavior was, and couldn't come up with anything that entirely matched.\n\n> Something like this (if not too ugly) might do the trick:\n\nI think that's good behavior, but it's kind of ugly. How about:\n\n-----\ncommit 83afef6a159365c1b9a7a1961cb4c95df24fbcac\nAuthor: Daniel Barkalow \nDate: Tue May 20 14:15:14 2008 -0400\n\n Fall back to copying if hardlinking fails\n \n Note that it stops trying hardlinks if any fail.\n \n Signed-off-by: Daniel Barkalow \n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 8713128..42633ae 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -207,13 +207,15 @@ static void copy_or_link_directory(char *src, char *dest)\n \n \t\tif (unlink(dest) && errno != ENOENT)\n \t\t\tdie(\"failed to unlink %s\\n\", dest);\n-\t\tif (option_no_hardlinks) {\n-\t\t\tif (copy_file(dest, src, 0666))\n-\t\t\t\tdie(\"failed to copy file to %s\\n\", dest);\n-\t\t} else {\n-\t\t\tif (link(src, dest))\n+\t\tif (!option_no_hardlinks) {\n+\t\t\tif (!link(src, dest))\n+\t\t\t\tcontinue;\n+\t\t\tif (option_local)\n \t\t\t\tdie(\"failed to create link %s\\n\", dest);\n+\t\t\toption_no_hardlinks = 1;\n \t\t}\n+\t\tif (copy_file(dest, src, 0666))\n+\t\t\tdie(\"failed to copy file to %s\\n\", dest);\n \t}\n }\n \n"},{"id":"77331","messageId":"2W5fKgEzyIWieZBezujpgWKgshbyTXW2uBoRhp11gtOL5nAZalnVfg@cipher.nrlssc.navy.mil","threadId":"13588","inReplyTo":"alpine.LNX.1.00.0805201311180.19665@iabervon.org","subject":"Re: builtin-clone does not fallback to copy when link fails","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-05-20T18:44:18Z","receivedAt":"2008-05-20T18:44:18Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Daniel Barkalow wrote:\n> On Tue, 20 May 2008, Brandon Casey wrote:\n> \n>> Brandon Casey wrote:\n>>> When cloning with the new builtin-clone, if the src repo is not\n>>> on the same disk as the dest repo, cloning fails. This is because\n>>> hard linking does not fall back to copying like the shell version\n>>> did.\n>>>\n>>> The shell version also made a distinction between defaulting to\n>>> hard linking and an explicit request to hard link. In the latter\n>>> case it would not fall back to copying, but would die.\n> \n> I think that the shell version's behavior changed at some point, too. I \n> think I tried at some point to figure out exactly what the specified \n> behavior was, and couldn't come up with anything that entirely matched.\n> \n>> Something like this (if not too ugly) might do the trick:\n> \n> I think that's good behavior, but it's kind of ugly. How about:\n> \n> -----\n> commit 83afef6a159365c1b9a7a1961cb4c95df24fbcac\n> Author: Daniel Barkalow \n> Date: Tue May 20 14:15:14 2008 -0400\n> \n> Fall back to copying if hardlinking fails\n> \n> Note that it stops trying hardlinks if any fail.\n> \n> Signed-off-by: Daniel Barkalow \n> \n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index 8713128..42633ae 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -207,13 +207,15 @@ static void copy_or_link_directory(char *src, char *dest)\n> \n> \t\tif (unlink(dest) && errno != ENOENT)\n> \t\t\tdie(\"failed to unlink %s\\n\", dest);\n> -\t\tif (option_no_hardlinks) {\n> -\t\t\tif (copy_file(dest, src, 0666))\n> -\t\t\t\tdie(\"failed to copy file to %s\\n\", dest);\n> -\t\t} else {\n> -\t\t\tif (link(src, dest))\n> +\t\tif (!option_no_hardlinks) {\n> +\t\t\tif (!link(src, dest))\n> +\t\t\t\tcontinue;\n> +\t\t\tif (option_local)\n> \t\t\t\tdie(\"failed to create link %s\\n\", dest);\n> +\t\t\toption_no_hardlinks = 1;\n> \t\t}\n> +\t\tif (copy_file(dest, src, 0666))\n> +\t\t\tdie(\"failed to copy file to %s\\n\", dest);\n> \t}\n> }\n\nactually, I don't like that buried 'continue' either, but it looks like it\nwould work just the same... It does.\n\n-brandon\n"},{"id":"77349","messageId":"alpine.DEB.1.00.0805202248000.30431@racer","threadId":"13588","inReplyTo":"alpine.LNX.1.00.0805201311180.19665@iabervon.org","subject":"Re: builtin-clone does not fallback to copy when link fails","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-20T21:48:30Z","receivedAt":"2008-05-20T21:48:30Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 20 May 2008, Daniel Barkalow wrote:\n\n> How about:\n> \n> -----\n> commit 83afef6a159365c1b9a7a1961cb4c95df24fbcac\n> Author: Daniel Barkalow \n> Date: Tue May 20 14:15:14 2008 -0400\n> \n> Fall back to copying if hardlinking fails\n> \n> Note that it stops trying hardlinks if any fail.\n> \n> Signed-off-by: Daniel Barkalow \n\nI like it.\n\nCiao,\nDscho\n"}]}