{"thread":{"id":"19023","subject":"[PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","startedAt":"2009-04-23T10:53:37Z","lastAt":"2009-04-28T22:07:08Z","messageCount":38,"participants":["Johannes Schindelin","Johannes Sixt","Alex Riesen","Junio C Hamano","Linus Torvalds","Michael Gaber","Jay Soffian"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"112086","messageId":"alpine.DEB.1.00.0904231252080.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":null,"subject":"[PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-23T10:53:37Z","receivedAt":"2009-04-23T10:53:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt seems that accessing NTFS partitions with ufsd (at least on my EeePC)\nhas an unnerving bug: if you link() a file and unlink() it right away,\nthe target of the link() will have the correct size, but consist of NULs.\n\nIt seems as if the calls are simply not serialized correctly, as single-stepping\nthrough the function move_temp_to_file() works flawlessly.\n\nAs ufsd is \"Commertial software\", I cannot fix it, and have to work\naround it in Git.\n\nAt the same time, it seems that this fixes msysGit issues 222 and 229 to\nassume that Windows cannot handle link() && unlink().\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tI know this is pretty late in the -rc1 cycle, but this is \n\tsomething I need to apply for msysGit anyway.\n\n\tAnd it does not really seem too unobvious a fix, does it?\n\n Documentation/config.txt |    5 +++++\n Makefile                 |    8 ++++++++\n cache.h                  |    2 ++\n config.c                 |    5 +++++\n environment.c            |    4 ++++\n sha1_file.c              |    4 +++-\n 6 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 35056e1..62cd903 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -429,6 +429,11 @@ relatively high IO latencies.  With this set to 'true', git will do the\n index comparison to the filesystem data in parallel, allowing\n overlapping IO's.\n \n+core.unreliableHardlinks::\n+\tSome filesystem drivers cannot properly handle hardlinking a file\n+\tand deleting the source right away.  In such a case, you need to\n+\tset this config variable to 'true'.\n+\n alias.*::\n \tCommand aliases for the linkgit:git[1] command wrapper - e.g.\n \tafter defining \"alias.last = cat-file commit HEAD\", the invocation\ndiff --git a/Makefile b/Makefile\nindex 49f36f5..e3979e0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -171,6 +171,10 @@ all::\n # Define UNRELIABLE_FSTAT if your system's fstat does not return the same\n # information on a not yet closed file that lstat would return for the same\n # file after it was closed.\n+#\n+# Define UNRELIABLE_HARDLINKS if your operating systems has problems when\n+# hardlinking a file to another name and unlinking the original file right\n+# away (some NTFS drivers seem to zero the contents in that scenario).\n \n GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -835,6 +839,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tNO_NSEC = YesPlease\n \tUSE_WIN32_MMAP = YesPlease\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n+\tUNRELIABLE_HARDLINKS = UnfortunatelySometimes\n \tCOMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch\n \tCOMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n@@ -1018,6 +1023,9 @@ else\n \t\tCOMPAT_OBJS += compat/win32mmap.o\n \tendif\n endif\n+ifdef UNRELIABLE_HARDLINKS\n+\tCOMPAT_CFLAGS += -DUNRELIABLE_HARDLINKS=1\n+endif\n ifdef NO_PREAD\n \tCOMPAT_CFLAGS += -DNO_PREAD\n \tCOMPAT_OBJS += compat/pread.o\ndiff --git a/cache.h b/cache.h\nindex ab1294d..ff9e145 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -554,6 +554,8 @@ extern enum branch_track git_branch_track;\n extern enum rebase_setup_type autorebase;\n extern enum push_default_type push_default;\n \n+extern int unreliable_hardlinks;\n+\n #define GIT_REPO_VERSION 0\n extern int repository_format_version;\n extern int check_repository_format(void);\ndiff --git a/config.c b/config.c\nindex 8c1ae59..1750cfb 100644\n--- a/config.c\n+++ b/config.c\n@@ -495,6 +495,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.unreliablehardlinks\")) {\n+\t\tunreliable_hardlinks = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/environment.c b/environment.c\nindex 4696885..10578d2 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -43,6 +43,10 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n+#ifndef UNRELIABLE_HARDLINKS\n+#define UNRELIABLE_HARDLINKS 0\n+#endif\n+int unreliable_hardlinks = UNRELIABLE_HARDLINKS;\n \n /* Parallel index stat data preload? */\n int core_preload_index = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8fe135d..f5a7970 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n {\n \tint ret = 0;\n \n-\tif (link(tmpfile, filename))\n+\tif (unreliable_hardlinks)\n+\t\tret = ~EEXIST;\n+\telse if (link(tmpfile, filename))\n \t\tret = errno;\n \n \t/*\n-- \n1.6.2.1.613.g25746\n"},{"id":"112115","messageId":"200904232116.10769.j6t@kdbg.org","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904231252080.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-04-23T19:16:10Z","receivedAt":"2009-04-23T19:16:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Donnerstag, 23. April 2009, Johannes Schindelin wrote:\n> It seems that accessing NTFS partitions with ufsd (at least on my EeePC)\n> has an unnerving bug: if you link() a file and unlink() it right away,\n> the target of the link() will have the correct size, but consist of NULs.\n>\n> It seems as if the calls are simply not serialized correctly, as\n> single-stepping through the function move_temp_to_file() works flawlessly.\n>\n> As ufsd is \"Commertial software\", I cannot fix it, and have to work\n\n\"commercial software\"\n\n> around it in Git.\n>\n> At the same time, it seems that this fixes msysGit issues 222 and 229 to\n> assume that Windows cannot handle link() && unlink().\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n...\n> @@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char\n> *filename) {\n>  \tint ret = 0;\n>\n> -\tif (link(tmpfile, filename))\n> +\tif (unreliable_hardlinks)\n> +\t\tret = ~EEXIST;\n\nIt took me a while to see why we need a tilde here, but it's ok. Perhaps this \nhelps others:\n\n+\t\tret = ~EEXIST;\t/* anything but EEXIST */\n\nNevertheless:\n\nAcked-by: Johannes Sixt <j6t@kdbg.org>\n\n> +\telse if (link(tmpfile, filename))\n>  \t\tret = errno;\n\n-- Hannes\n"},{"id":"112120","messageId":"alpine.DEB.1.00.0904232132380.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"200904232116.10769.j6t@kdbg.org","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-23T19:33:53Z","receivedAt":"2009-04-23T19:33:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 23 Apr 2009, Johannes Sixt wrote:\n\n> On Donnerstag, 23. April 2009, Johannes Schindelin wrote:\n> > It seems that accessing NTFS partitions with ufsd (at least on my EeePC)\n> > has an unnerving bug: if you link() a file and unlink() it right away,\n> > the target of the link() will have the correct size, but consist of NULs.\n> >\n> > It seems as if the calls are simply not serialized correctly, as\n> > single-stepping through the function move_temp_to_file() works flawlessly.\n> >\n> > As ufsd is \"Commertial software\", I cannot fix it, and have to work\n> \n> \"commercial software\"\n\nI just quoted the license string of that wonderfully high-quality kernel \nmodule.\n\nMaybe I should have added the beloved \"[sic!]\".\n\n> > At the same time, it seems that this fixes msysGit issues 222 and 229 to\n> > assume that Windows cannot handle link() && unlink().\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ...\n> > @@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char\n> > *filename) {\n> >  \tint ret = 0;\n> >\n> > -\tif (link(tmpfile, filename))\n> > +\tif (unreliable_hardlinks)\n> > +\t\tret = ~EEXIST;\n> \n> It took me a while to see why we need a tilde here, but it's ok. Perhaps this \n> helps others:\n> \n> +\t\tret = ~EEXIST;\t/* anything but EEXIST */\n\nWill do.\n\n> Nevertheless:\n> \n> Acked-by: Johannes Sixt <j6t@kdbg.org>\n\nThanks.\n\nBut it will have to wait for Saturday.\n\nCiao,\nDscho\n"},{"id":"112122","messageId":"81b0412b0904231239qf317c02xbfa548d0011a0302@mail.gmail.com","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904231252080.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-04-23T19:39:38Z","receivedAt":"2009-04-23T19:39:38Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/4/23 Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n> -       if (link(tmpfile, filename))\n> +       if (unreliable_hardlinks)\n> +               ret = ~EEXIST;\n\nIt is more like \"broken_hardlinks\" or even \"no_hardlinks\"!\n"},{"id":"112139","messageId":"alpine.DEB.1.00.0904232358520.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"81b0412b0904231239qf317c02xbfa548d0011a0302@mail.gmail.com","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-23T21:59:44Z","receivedAt":"2009-04-23T21:59:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 23 Apr 2009, Alex Riesen wrote:\n\n> 2009/4/23 Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n> > -       if (link(tmpfile, filename))\n> > +       if (unreliable_hardlinks)\n> > +               ret = ~EEXIST;\n> \n> It is more like \"broken_hardlinks\" or even \"no_hardlinks\"!\n\nWrong.  As I wrote, single-stepping (i.e. leaving enough time between \nlink() and unlink()) works as expected.  So it is not even that the \nhardlinks are broken.  Just the serialization between the operations.\n\nCiao,\nDscho\n"},{"id":"112159","messageId":"81b0412b0904232244l15cd7347n23702d92b38ab7e5@mail.gmail.com","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904232358520.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-04-24T05:44:18Z","receivedAt":"2009-04-24T05:44:18Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/4/23 Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n> Hi,\n>\n> On Thu, 23 Apr 2009, Alex Riesen wrote:\n>\n>> 2009/4/23 Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n>> > -       if (link(tmpfile, filename))\n>> > +       if (unreliable_hardlinks)\n>> > +               ret = ~EEXIST;\n>>\n>> It is more like \"broken_hardlinks\" or even \"no_hardlinks\"!\n>\n> Wrong.  As I wrote, single-stepping (i.e. leaving enough time between\n> link() and unlink()) works as expected.  So it is not even that the\n> hardlinks are broken.  Just the serialization between the operations.\n>\n\nSince when does link(2) involve file _data_?\n"},{"id":"112288","messageId":"alpine.DEB.1.00.0904251155130.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"200904232116.10769.j6t@kdbg.org","subject":"[PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-25T09:57:14Z","receivedAt":"2009-04-25T09:57:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt seems that accessing NTFS partitions with ufsd (at least on my EeePC)\nhas an unnerving bug: if you link() a file and unlink() it right away,\nthe target of the link() will have the correct size, but consist of NULs.\n\nIt seems as if the calls are simply not serialized correctly, as single-stepping\nthrough the function move_temp_to_file() works flawlessly.\n\nAs ufsd is \"Commertial software\" (sic!), I cannot fix it, and have to work\naround it in Git.\n\nAt the same time, it seems that this fixes msysGit issues 222 and 229 to\nassume that Windows cannot handle link() && unlink().\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nAcked-by: Johannes Sixt <j6t@kdbg.org>\n---\n\n\tOn Thu, 23 Apr 2009, Johannes Sixt wrote:\n\n\t> +\t\tret = ~EEXIST;\t/* anything but EEXIST */\n\n\tIn addition to this, I also added the \"(sic!)\" for the \"Commertial \n\tsoftware\" tyop.  So I still dared to add your\n\n\t> Acked-by: Johannes Sixt <j6t@kdbg.org>\n\n Documentation/config.txt |    5 +++++\n Makefile                 |    8 ++++++++\n cache.h                  |    2 ++\n config.c                 |    5 +++++\n environment.c            |    4 ++++\n sha1_file.c              |    4 +++-\n 6 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3188569..d31adb6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -429,6 +429,11 @@ relatively high IO latencies.  With this set to 'true', git will do the\n index comparison to the filesystem data in parallel, allowing\n overlapping IO's.\n \n+core.unreliableHardlinks::\n+\tSome filesystem drivers cannot properly handle hardlinking a file\n+\tand deleting the source right away.  In such a case, you need to\n+\tset this config variable to 'true'.\n+\n alias.*::\n \tCommand aliases for the linkgit:git[1] command wrapper - e.g.\n \tafter defining \"alias.last = cat-file commit HEAD\", the invocation\ndiff --git a/Makefile b/Makefile\nindex 6f602c7..5c8e83a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -171,6 +171,10 @@ all::\n # Define UNRELIABLE_FSTAT if your system's fstat does not return the same\n # information on a not yet closed file that lstat would return for the same\n # file after it was closed.\n+#\n+# Define UNRELIABLE_HARDLINKS if your operating systems has problems when\n+# hardlinking a file to another name and unlinking the original file right\n+# away (some NTFS drivers seem to zero the contents in that scenario).\n \n GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -835,6 +839,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tNO_NSEC = YesPlease\n \tUSE_WIN32_MMAP = YesPlease\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n+\tUNRELIABLE_HARDLINKS = UnfortunatelySometimes\n \tCOMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch\n \tCOMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n@@ -1018,6 +1023,9 @@ else\n \t\tCOMPAT_OBJS += compat/win32mmap.o\n \tendif\n endif\n+ifdef UNRELIABLE_HARDLINKS\n+\tCOMPAT_CFLAGS += -DUNRELIABLE_HARDLINKS=1\n+endif\n ifdef NO_PREAD\n \tCOMPAT_CFLAGS += -DNO_PREAD\n \tCOMPAT_OBJS += compat/pread.o\ndiff --git a/cache.h b/cache.h\nindex ab1294d..ff9e145 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -554,6 +554,8 @@ extern enum branch_track git_branch_track;\n extern enum rebase_setup_type autorebase;\n extern enum push_default_type push_default;\n \n+extern int unreliable_hardlinks;\n+\n #define GIT_REPO_VERSION 0\n extern int repository_format_version;\n extern int check_repository_format(void);\ndiff --git a/config.c b/config.c\nindex 8c1ae59..1750cfb 100644\n--- a/config.c\n+++ b/config.c\n@@ -495,6 +495,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.unreliablehardlinks\")) {\n+\t\tunreliable_hardlinks = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/environment.c b/environment.c\nindex 4696885..10578d2 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -43,6 +43,10 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n+#ifndef UNRELIABLE_HARDLINKS\n+#define UNRELIABLE_HARDLINKS 0\n+#endif\n+int unreliable_hardlinks = UNRELIABLE_HARDLINKS;\n \n /* Parallel index stat data preload? */\n int core_preload_index = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8fe135d..bb6eecf 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n {\n \tint ret = 0;\n \n-\tif (link(tmpfile, filename))\n+\tif (unreliable_hardlinks)\n+\t\tret = ~EEXIST; /* anything but EEXIST */\n+\telse if (link(tmpfile, filename))\n \t\tret = errno;\n \n \t/*\n-- \n1.6.2.1.613.g25746\n"},{"id":"112307","messageId":"7vbpqkznjs.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904251155130.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-25T16:49:11Z","receivedAt":"2009-04-25T16:49:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> It seems that accessing NTFS partitions with ufsd (at least on my EeePC)\n> has an unnerving bug: if you link() a file and unlink() it right away,\n> the target of the link() will have the correct size, but consist of NULs.\n>\n> It seems as if the calls are simply not serialized correctly, as single-stepping\n> through the function move_temp_to_file() works flawlessly.\n>\n> As ufsd is \"Commertial software\" (sic!), I cannot fix it, and have to work\n> around it in Git.\n>\n> At the same time, it seems that this fixes msysGit issues 222 and 229 to\n> assume that Windows cannot handle link() && unlink().\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> Acked-by: Johannes Sixt <j6t@kdbg.org>\n\nHannes, are you ok with this?\n\n> diff --git a/environment.c b/environment.c\n> index 4696885..10578d2 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -43,6 +43,10 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n>  enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n>  enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n>  enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n> +#ifndef UNRELIABLE_HARDLINKS\n> +#define UNRELIABLE_HARDLINKS 0\n> +#endif\n> +int unreliable_hardlinks = UNRELIABLE_HARDLINKS;\n\nHmm, this ifndef/define/endif is somewhat yucky to see especially in a .c\nsource file.  Sorry, I do not think of a better alternative, though.\n\n\tint unreliable_hardlinks = defined(UNRELIABLE_HARDLINKS)\n\nwould not work either X-<.\n\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 8fe135d..bb6eecf 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n>  {\n>  \tint ret = 0;\n>  \n> -\tif (link(tmpfile, filename))\n> +\tif (unreliable_hardlinks)\n> +\t\tret = ~EEXIST; /* anything but EEXIST */\n\nIt is a bit too far away from the:\n\n\tif (ret && ret != EEXIST)\n\nyou are trying to trigger with this hack, and without seeing that \"if\" in\nthe context anybody would go \"Huh?\".  It is a good sign that this is\nfragile (the later \"if\" may be rewritten by somebody else without\nrealizing this hack exists).  Besides, it is (rather, \"happens to be at\nthis moment\") \"anything non-zero but EEXIST\".\n\nI have a feeling that it would be much less fragile to write it like this,\nas a label warns anybody touching the code to check where else the control\nflow may come from.\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8fe135d..11969fc 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n {\n \tint ret = 0;\n \n-\tif (link(tmpfile, filename))\n+\tif (unreliable_hardlinks)\n+\t\tgoto try_rename;\n+\telse if (link(tmpfile, filename))\n \t\tret = errno;\n \n \t/*\n@@ -2240,6 +2242,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n \t * left to unlink.\n \t */\n \tif (ret && ret != EEXIST) {\n+\ttry_rename:\n \t\tif (!rename(tmpfile, filename))\n \t\t\tgoto out;\n \t\tret = errno;\n"},{"id":"112308","messageId":"7vws98y886.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904251155130.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-25T17:05:29Z","receivedAt":"2009-04-25T17:05:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> It seems that accessing NTFS partitions with ufsd (at least on my EeePC)\n> has an unnerving bug: if you link() a file and unlink() it right away,\n> the target of the link() will have the correct size, but consist of NULs.\n>\n> It seems as if the calls are simply not serialized correctly, as single-stepping\n> through the function move_temp_to_file() works flawlessly.\n\nA few questions.\n\nWhen this problem triggers for you,\n\n (1) do we have an open file descriptor to the tmpfile?\n\n (2) if so have we fsync'ed (or better yet, closed) it?\n\n (3) if the answers to the above are \"yes, no\", does it help the situation\n     if we fsync the filedescriptor before calling move_temp_to_file()?\n\n    ... gitster digs after asking questions to find answers himself ...\n\nI realize that the answers seem to be \"no, and the fd that created the\ntempfile has been closed\".  Hmm.  Very curious.\n\nSo if you do:\n\n\tcat >corrupt-move.c <<\\EOF\n\t#include <unistd.h>\n\tint main(int ac, char **av)\n        {\n                return (link(av[1], av[2]) || unlink(av[1]));\n\t}\n\tEOF\n        cc -o corrupt-move corrupt-move.c\n        ./corrupt-move corrupt-move.c corrupt-move.c.new\n\nyou end up with a corrupt-move.c.new file that is full of NUL?\n\nVery curious...\n"},{"id":"112310","messageId":"alpine.LFD.2.00.0904251037200.3101@localhost.localdomain","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904251155130.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-25T17:39:19Z","receivedAt":"2009-04-25T17:39:19Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 25 Apr 2009, Johannes Schindelin wrote:\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 8fe135d..bb6eecf 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n>  {\n>  \tint ret = 0;\n>  \n> -\tif (link(tmpfile, filename))\n> +\tif (unreliable_hardlinks)\n> +\t\tret = ~EEXIST; /* anything but EEXIST */\n\nDon't do this. ~EEXIST could be 0 (admittedly only if EEXIST is -1 which \nis not reasonable, but who knows about odd operating systems). Which is \nnot a good return value either.\n\nSo why not just use an explicit error value like EIO? Don't play games \nwith this.\n\n\t\t\tLinus\n"},{"id":"112311","messageId":"alpine.LFD.2.00.0904251039460.3101@localhost.localdomain","threadId":"19023","inReplyTo":"7vbpqkznjs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-25T17:40:15Z","receivedAt":"2009-04-25T17:40:15Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 25 Apr 2009, Junio C Hamano wrote:\n> @@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n>  {\n>  \tint ret = 0;\n>  \n> -\tif (link(tmpfile, filename))\n> +\tif (unreliable_hardlinks)\n> +\t\tgoto try_rename;\n\nMuch better.\n\n\t\tLinus\n"},{"id":"112312","messageId":"alpine.LFD.2.00.0904251042490.3101@localhost.localdomain","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904231252080.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-25T17:56:01Z","receivedAt":"2009-04-25T17:56:01Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 23 Apr 2009, Johannes Schindelin wrote:\n> \n> It seems that accessing NTFS partitions with ufsd (at least on my EeePC)\n> has an unnerving bug: if you link() a file and unlink() it right away,\n> the target of the link() will have the correct size, but consist of NULs.\n\nSo I assume that the way ufsd works is that it implements a user-level \nNTFS driver and then exposes it as a NFS mount over local networking (and\nperhaps also remotely?)\n\n> It seems as if the calls are simply not serialized correctly, as single-stepping\n> through the function move_temp_to_file() works flawlessly.\n\nSo presumably there is some cached writes somewhere (a NFS client _should_ \nnot cache writes past a 'close()', but maybe there is a bug there and/or \nbuffering inside ufsd itself that means that the writes are still queued \nup). And when the unlink() happens, it loses the writes to the original \nfile, and thus to the new one too.\n\nIf you _don't_ do this patch, does \n\n\t[core]\n\t\tfsyncobjectfiles = true\n\nhide the bug? \n\nI don't disagree with your patch (apart from the error number games), but \nI'd like to understand what's going on. I also wonder if we should make \nthat fsync thign be the default.\n\n[ That said, I think the http walker and possibly others may be using \n  'move_temp_to_file()' without going through any of the paths that know \n  about fsync, so 'fsyncobjectfiles' wouldn't fix all cases anyway. ]\n\nHmm. I hate how we have problems with that \"link/unlink\" sequence, and \n\"rename()\" would be much better, but I'd hate overwriting existing objects \neven _more_, and the normal POSIX rename() behavior is to overwrite any \nold object. So link/unlink is supposed to be a lot safer, but it's clearly \nproblematic.\n\n\t\tLinus\n"},{"id":"112314","messageId":"49F3588A.4000707@gmx.net","threadId":"19023","inReplyTo":"alpine.LFD.2.00.0904251039460.3101@localhost.localdomain","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Michael Gaber","fromEmail":"michael.gaber@gmx.net","sentAt":"2009-04-25T18:38:02Z","receivedAt":"2009-04-25T18:38:02Z","isPatch":true,"sender":{"key":"michael.gaber@gmx.net","avatar":null},"body":"Linus Torvalds schrieb:\n> \n> On Sat, 25 Apr 2009, Junio C Hamano wrote:\n>> @@ -2225,7 +2225,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n>>  {\n>>  \tint ret = 0;\n>>  \n>> -\tif (link(tmpfile, filename))\n>> +\tif (unreliable_hardlinks)\n>> +\t\tgoto try_rename;\n> \n> Much better.\n> \n> \t\tLinus\n\nhttp://www.cs.utexas.edu/users/EWD/ewd02xx/EWD215.PDF\n\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n\n"},{"id":"112315","messageId":"alpine.LFD.2.00.0904251142150.3101@localhost.localdomain","threadId":"19023","inReplyTo":"49F3588A.4000707@gmx.net","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-25T18:43:10Z","receivedAt":"2009-04-25T18:43:10Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 25 Apr 2009, Michael Gaber wrote:\n> \n> http://www.cs.utexas.edu/users/EWD/ewd02xx/EWD215.PDF\n\nYeah, and people thought \"pascal\" was a good language because it didn't \ncontain \"break\" statements to break out of loops, or \"return\" statements \nto break out of functions early.\n\nToo bad. They were wrong.\n\n\t\t\tLinus\n"},{"id":"112316","messageId":"200904252050.10306.j6t@kdbg.org","threadId":"19023","inReplyTo":"7vbpqkznjs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-04-25T18:50:10Z","receivedAt":"2009-04-25T18:50:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 25. April 2009, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> > It seems that accessing NTFS partitions with ufsd (at least on my EeePC)\n> > has an unnerving bug: if you link() a file and unlink() it right away,\n> > the target of the link() will have the correct size, but consist of NULs.\n> >\n> > It seems as if the calls are simply not serialized correctly, as\n> > single-stepping through the function move_temp_to_file() works\n> > flawlessly.\n> >\n> > As ufsd is \"Commertial software\" (sic!), I cannot fix it, and have to\n> > work around it in Git.\n> >\n> > At the same time, it seems that this fixes msysGit issues 222 and 229 to\n> > assume that Windows cannot handle link() && unlink().\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > Acked-by: Johannes Sixt <j6t@kdbg.org>\n>\n> Hannes, are you ok with this?\n\nYes. We have been using rename() instead of link() on Windows until recently \nanyway (until link() was implemented, 7be401e06, 2009-01-24). There is no \nregression to be expected from this side.\n\n-- Hannes\n"},{"id":"112317","messageId":"200904252052.10327.j6t@kdbg.org","threadId":"19023","inReplyTo":"alpine.LFD.2.00.0904251042490.3101@localhost.localdomain","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-04-25T18:52:10Z","receivedAt":"2009-04-25T18:52:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 25. April 2009, Linus Torvalds wrote:\n> If you _don't_ do this patch, does\n>\n> \t[core]\n> \t\tfsyncobjectfiles = true\n>\n> hide the bug?\n\nMost likely not because our fsync() on Windows is a noop :(\n\n-- Hannes\n"},{"id":"112324","messageId":"7vhc0cw6w8.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"200904252052.10327.j6t@kdbg.org","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-26T01:17:11Z","receivedAt":"2009-04-26T01:17:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> On Samstag, 25. April 2009, Linus Torvalds wrote:\n>> If you _don't_ do this patch, does\n>>\n>> \t[core]\n>> \t\tfsyncobjectfiles = true\n>>\n>> hide the bug?\n>\n> Most likely not because our fsync() on Windows is a noop :(\n\nActually, the reason I CC'ed Linus (and I also was interested in the\nplatform bug itself) was because I think Dscho is accessing the NTFS\npartition from the Linux side.\n"},{"id":"112378","messageId":"alpine.DEB.1.00.0904261936011.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"alpine.LFD.2.00.0904251042490.3101@localhost.localdomain","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-26T17:38:08Z","receivedAt":"2009-04-26T17:38:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 25 Apr 2009, Linus Torvalds wrote:\n\n> If you _don't_ do this patch, does \n> \n> \t[core]\n> \t\tfsyncobjectfiles = true\n> \n> hide the bug?\n\nYes.  On my EeePC with that stupid ufsd driver.\n\nHowever, there is the other issue with Windows, so we still need half of \nmy patch.\n\nJunio, do you want me to throw out the core.unreliableHardlinks stuff, and \nonly keep it as a Makefile option?\n\nCiao,\nDscho\n"},{"id":"112399","messageId":"alpine.DEB.1.00.0904261939190.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"7vws98y886.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-26T17:39:54Z","receivedAt":"2009-04-26T17:39:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 25 Apr 2009, Junio C Hamano wrote:\n\n> So if you do:\n> \n> \tcat >corrupt-move.c <<\\EOF\n> \t#include <unistd.h>\n> \tint main(int ac, char **av)\n>         {\n>                 return (link(av[1], av[2]) || unlink(av[1]));\n> \t}\n> \tEOF\n>         cc -o corrupt-move corrupt-move.c\n>         ./corrupt-move corrupt-move.c corrupt-move.c.new\n> \n> you end up with a corrupt-move.c.new file that is full of NUL?\n\nI have not compiled and run this code, but I am _real_ sure that this is \nexactly the issue.\n\nCiao,\nDscho\n"},{"id":"112377","messageId":"alpine.DEB.1.00.0904261940170.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"7vhc0cw6w8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-26T17:40:26Z","receivedAt":"2009-04-26T17:40:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 25 Apr 2009, Junio C Hamano wrote:\n\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n> > On Samstag, 25. April 2009, Linus Torvalds wrote:\n> >> If you _don't_ do this patch, does\n> >>\n> >> \t[core]\n> >> \t\tfsyncobjectfiles = true\n> >>\n> >> hide the bug?\n> >\n> > Most likely not because our fsync() on Windows is a noop :(\n> \n> Actually, the reason I CC'ed Linus (and I also was interested in the\n> platform bug itself) was because I think Dscho is accessing the NTFS\n> partition from the Linux side.\n\nYep, correct.\n\nCiao,\nDscho\n"},{"id":"112344","messageId":"76718490904262037r5dc39225k2adb500cd855b4f2@mail.gmail.com","threadId":"19023","inReplyTo":"49F3588A.4000707@gmx.net","subject":"Re: [PATCH v2] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-04-27T03:37:53Z","receivedAt":"2009-04-27T03:37:53Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, Apr 25, 2009 at 2:38 PM, Michael Gaber <Michael.Gaber@gmx.net> wrote:\n> http://www.cs.utexas.edu/users/EWD/ewd02xx/EWD215.PDF\n\nhttp://www.bartleby.com/59/3/foolishconsi.html\n\nj.\n"},{"id":"112413","messageId":"alpine.DEB.1.00.0904271400180.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904261940170.10279@pacific.mpi-cbg.de","subject":"[PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-27T12:00:56Z","receivedAt":"2009-04-27T12:00:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt seems that accessing NTFS partitions with ufsd (at least on my EeePC)\nhas an unnerving bug: if you link() a file and unlink() it right away,\nthe target of the link() will have the correct size, but consist of\nNULs.\n\nIt seems as if the calls are simply not serialized correctly, as\nsingle-stepping through the function move_temp_to_file() works\nflawlessly.\n\nOn Linux, this issue can be fixed by setting core.fsyncobjects to true\n(thanks Linus), but the same is not true on Windows.\n\nSo, force the use of rename() instead of the link() && unlink()\nincantation on Windows, and for good measure, add a\ncore.unreliableHardlinks option to optionally force it on other\nplatforms, too.\n\nThis fixes msysGit issues 222 and 229.\n\nIt was substantially improved by the help of Junio.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n[TENTATIVE] Acked-by: Johannes Sixt <j6t@kdbg.org>\n---\n\n\tHannes, is it okay to remove the [TENTATIVE]?\n\n\tJunio, do you want me to remove the config variable?\n\n Documentation/config.txt |    5 +++++\n Makefile                 |    8 ++++++++\n cache.h                  |    2 ++\n config.c                 |    5 +++++\n environment.c            |    4 ++++\n sha1_file.c              |    3 +++\n 6 files changed, 27 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3188569..d31adb6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -429,6 +429,11 @@ relatively high IO latencies.  With this set to 'true', git will do the\n index comparison to the filesystem data in parallel, allowing\n overlapping IO's.\n \n+core.unreliableHardlinks::\n+\tSome filesystem drivers cannot properly handle hardlinking a file\n+\tand deleting the source right away.  In such a case, you need to\n+\tset this config variable to 'true'.\n+\n alias.*::\n \tCommand aliases for the linkgit:git[1] command wrapper - e.g.\n \tafter defining \"alias.last = cat-file commit HEAD\", the invocation\ndiff --git a/Makefile b/Makefile\nindex 6f602c7..5c8e83a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -171,6 +171,10 @@ all::\n # Define UNRELIABLE_FSTAT if your system's fstat does not return the same\n # information on a not yet closed file that lstat would return for the same\n # file after it was closed.\n+#\n+# Define UNRELIABLE_HARDLINKS if your operating systems has problems when\n+# hardlinking a file to another name and unlinking the original file right\n+# away (some NTFS drivers seem to zero the contents in that scenario).\n \n GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -835,6 +839,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tNO_NSEC = YesPlease\n \tUSE_WIN32_MMAP = YesPlease\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n+\tUNRELIABLE_HARDLINKS = UnfortunatelySometimes\n \tCOMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch\n \tCOMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n@@ -1018,6 +1023,9 @@ else\n \t\tCOMPAT_OBJS += compat/win32mmap.o\n \tendif\n endif\n+ifdef UNRELIABLE_HARDLINKS\n+\tCOMPAT_CFLAGS += -DUNRELIABLE_HARDLINKS=1\n+endif\n ifdef NO_PREAD\n \tCOMPAT_CFLAGS += -DNO_PREAD\n \tCOMPAT_OBJS += compat/pread.o\ndiff --git a/cache.h b/cache.h\nindex ab1294d..ff9e145 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -554,6 +554,8 @@ extern enum branch_track git_branch_track;\n extern enum rebase_setup_type autorebase;\n extern enum push_default_type push_default;\n \n+extern int unreliable_hardlinks;\n+\n #define GIT_REPO_VERSION 0\n extern int repository_format_version;\n extern int check_repository_format(void);\ndiff --git a/config.c b/config.c\nindex 8c1ae59..1750cfb 100644\n--- a/config.c\n+++ b/config.c\n@@ -495,6 +495,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.unreliablehardlinks\")) {\n+\t\tunreliable_hardlinks = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/environment.c b/environment.c\nindex 4696885..10578d2 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -43,6 +43,10 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n+#ifndef UNRELIABLE_HARDLINKS\n+#define UNRELIABLE_HARDLINKS 0\n+#endif\n+int unreliable_hardlinks = UNRELIABLE_HARDLINKS;\n \n /* Parallel index stat data preload? */\n int core_preload_index = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8fe135d..0d289f4 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2225,6 +2225,8 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n {\n \tint ret = 0;\n \n+\tif (unreliable_hardlinks)\n+\t\tgoto try_rename;\n \tif (link(tmpfile, filename))\n \t\tret = errno;\n \n@@ -2240,6 +2242,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n \t * left to unlink.\n \t */\n \tif (ret && ret != EEXIST) {\n+try_rename:\n \t\tif (!rename(tmpfile, filename))\n \t\t\tgoto out;\n \t\tret = errno;\n-- \n1.6.2.1.613.g25746\n"},{"id":"112423","messageId":"alpine.LFD.2.00.0904270806130.22156@localhost.localdomain","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904271400180.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-27T15:15:10Z","receivedAt":"2009-04-27T15:15:10Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 27 Apr 2009, Johannes Schindelin wrote:\n> \n> So, force the use of rename() instead of the link() && unlink()\n> incantation on Windows, and for good measure, add a\n> core.unreliableHardlinks option to optionally force it on other\n> platforms, too.\n\nOk, so:\n\n\tAcked-by: Linus Torvalds <torvalds@linux-foundation.org>\n\nbut I do think it could be improved. See below..\n\n> \tJunio, do you want me to remove the config variable?\n\nI'd keep it. But I'd suggest that the naming is odd. Why talk about \n\"unreliable hardlinks\", when that's just a particular symptom. Why not \njust talk about whether hardlinks should be used or not?\n\nAnd to avoid double negative, make it\n\n\t[core]\n\t\tusehardlinks = true/false\n\nand then default it to 'true' for Unix.\n\nThe thing is, maybe people would prefer to use 'rename' over the \nlink/unlink games even on some unixes, and not because of 'reliability' \nissues, but because they may have some filesystems that don't do \nhardlinks, and they'd just rather speed things up by avoiding the 'link()' \nsystem call that will just error out.\n\nSo naming matters. Calling it 'unreliablehardlinks' in that case would be \nodd. They're not unreliable - you just don't want to try to use them.\n\nI also do wonder if we could/should make this one of those options that \nget set automatically at 'git init' time, rather than silently hardcoded \nas a compile option. I thought hardlinks at least sometimes worked fine on \nWindows too, don't they? \n\nI do detest _hidden_ default values for config options, unless those \nhidden defaults are \"obviously always correct\" as a default. This one \nsmells a bit uncertain, and as a result I think it's ok to default to not \nusing hardlinks, but doing it with .gitconfig would be nicer.\n\nHmm?\n\n\t\tLinus\n"},{"id":"112429","messageId":"alpine.DEB.1.00.0904271800360.7741@intel-tinevez-2-302","threadId":"19023","inReplyTo":"alpine.LFD.2.00.0904270806130.22156@localhost.localdomain","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-27T16:11:50Z","receivedAt":"2009-04-27T16:11:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 27 Apr 2009, Linus Torvalds wrote:\n\n> On Mon, 27 Apr 2009, Johannes Schindelin wrote:\n> > \n> > So, force the use of rename() instead of the link() && unlink() \n> > incantation on Windows, and for good measure, add a \n> > core.unreliableHardlinks option to optionally force it on other \n> > platforms, too.\n> \n> Ok, so:\n> \n> \tAcked-by: Linus Torvalds <torvalds@linux-foundation.org>\n> \n> but I do think it could be improved. See below..\n\nSorry, I missed the fact that Junio already applied and pushed it to \n'next'.\n\n> > \tJunio, do you want me to remove the config variable?\n> \n> I'd keep it. But I'd suggest that the naming is odd. Why talk about \n> \"unreliable hardlinks\", when that's just a particular symptom. Why not \n> just talk about whether hardlinks should be used or not?\n> \n> And to avoid double negative, make it\n> \n> \t[core]\n> \t\tusehardlinks = true/false\n> \n> and then default it to 'true' for Unix.\n\nOr maybe core.preferRenameOverLink?  Then we have no negation either.\n\n> The thing is, maybe people would prefer to use 'rename' over the \n> link/unlink games even on some unixes, and not because of 'reliability' \n> issues, but because they may have some filesystems that don't do \n> hardlinks, and they'd just rather speed things up by avoiding the 'link()' \n> system call that will just error out.\n\nWe already fall back to renaming when another error than EEXIST is \nreturned from link(), so I think this case is covered.\n\n> So naming matters. Calling it 'unreliablehardlinks' in that case would \n> be odd. They're not unreliable - you just don't want to try to use them.\n> \n> I also do wonder if we could/should make this one of those options that \n> get set automatically at 'git init' time, rather than silently hardcoded \n> as a compile option. I thought hardlinks at least sometimes worked fine on \n> Windows too, don't they? \n\nI thought about that long and hard, and I decided against it.  Take my \nNTFS-formatted portable hard drive (for convenience with Windows users \n@work) for example: the ufsd driver is totally broken, but because it is a \nmajor investment of time to get my EeePC to work with a sane Linux \ndistribution, I'd rather keep using the ufsd driver.  Yet, when I use \nntfs-3g from the other laptop, it works fine.\n\nSee?  It is not a file system specific error, but a fs/os combo problem.\n\n> I do detest _hidden_ default values for config options, unless those \n> hidden defaults are \"obviously always correct\" as a default. This one \n> smells a bit uncertain, and as a result I think it's ok to default to \n> not using hardlinks, but doing it with .gitconfig would be nicer.\n\nI fully agree on hidden default values, albeit in this case, it is \nnecessary: the hard links work just fine on Windows XP here, but that \nmight just be a matter of not upgrading to a newer service pack.\n\nCiao,\nDscho\n"},{"id":"112438","messageId":"alpine.LFD.2.00.0904270952040.22156@localhost.localdomain","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904271800360.7741@intel-tinevez-2-302","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-27T16:53:06Z","receivedAt":"2009-04-27T16:53:06Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 27 Apr 2009, Johannes Schindelin wrote:\n> \n> > The thing is, maybe people would prefer to use 'rename' over the \n> > link/unlink games even on some unixes, and not because of 'reliability' \n> > issues, but because they may have some filesystems that don't do \n> > hardlinks, and they'd just rather speed things up by avoiding the 'link()' \n> > system call that will just error out.\n> \n> We already fall back to renaming when another error than EEXIST is \n> returned from link(), so I think this case is covered.\n\nYou didn't read what I wrote.\n\n  \"they'd just rather speed things up by avoiding the 'link()' system call \n   that will just error out.\"\n\nI know we fall back to rename(). The point is that if you know link \ndoesn't work, why not just skip it?\n\n\t\tLinus\n"},{"id":"112455","messageId":"7vljpl3m8i.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"alpine.LFD.2.00.0904270806130.22156@localhost.localdomain","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-27T19:55:25Z","receivedAt":"2009-04-27T19:55:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n>> \tJunio, do you want me to remove the config variable?\n>\n> I'd keep it. But I'd suggest that the naming is odd. Why talk about \n> \"unreliable hardlinks\", when that's just a particular symptom. Why not \n> just talk about whether hardlinks should be used or not?\n>\n> And to avoid double negative, make it\n>\n> \t[core]\n> \t\tusehardlinks = true/false\n>\n> and then default it to 'true' for Unix.\n\nI am a bit worried about this name, too.  It may lead people to a\nmisunderstanding that we would do something magical when they do this with\nthe configuration set:\n\n\twget http://some.where/huge-file.mpg 1.mpg\n        ln 1.mpg 2.mpg\n        git add 1.mpg 2.mpg\n        rm -f 1.mpg 2.mpg\n        git checkout-index -a\n\tls -i ?.mpg\n\n> The thing is, maybe people would prefer to use 'rename' over the\n> link/unlink games even on some unixes, and not because of 'reliability'\n> issues, but because they may have some filesystems that don't do\n> hardlinks, and they'd just rather speed things up by avoiding the\n> 'link()' system call that will just error out.\n\n> So naming matters. Calling it 'unreliablehardlinks' in that case would be \n> odd. They're not unreliable - you just don't want to try to use them.\n\nThis part I agree with.\n\n> I also do wonder if we could/should make this one of those options that \n> get set automatically at 'git init' time, rather than silently hardcoded \n> as a compile option. I thought hardlinks at least sometimes worked fine on \n> Windows too, don't they? \n>\n> I do detest _hidden_ default values for config options, unless those \n> hidden defaults are \"obviously always correct\" as a default. This one \n> smells a bit uncertain, and as a result I think it's ok to default to not \n> using hardlinks, but doing it with .gitconfig would be nicer.\n\nThe coda hack comment in move_temp_to_file() shows what we can do to\nautodetect (i.e. try cross directory hardlink), but I somehow thought that\nwe changed the code enough to ensure that we create the tmpfiles in the\nsame directory as their final destination?\n"},{"id":"112459","messageId":"alpine.LFD.2.00.0904271304300.22156@localhost.localdomain","threadId":"19023","inReplyTo":"7vljpl3m8i.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-27T20:13:53Z","receivedAt":"2009-04-27T20:13:53Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 27 Apr 2009, Junio C Hamano wrote:\n> \n> The coda hack comment in move_temp_to_file() shows what we can do to\n> autodetect (i.e. try cross directory hardlink)\n\nThe thing is, we cannot do it reliably across different systems.\n\nCoda simply doesn't _support_ hardlinks across directories at all. So it \nwill always return an error when you try, and you can see the error \ndirectly and easily.\n\n> but I somehow thought that we changed the code enough to ensure that we \n> create the tmpfiles in the same directory as their final destination?\n\nThis was for a totally different case - a certain kind of NFS client bug \nwith a certain kind of (arguably buggy, but I can understand it because \nNFS is just a bad protocol in this respect) NFS server, where you may be \nable to do cross-directory renames, but it caused problems later.\n\nNow, the reason cross-directory name movement matters is that it makes \nmany things much harder, and filesystems thus have a much harder time \ndoing them well (or decide to not support them at all, as in Coda). Within \na single directory, things are just simpler, and thus less likely to hit \nbugs.\n\nIOW, with cross-directory link/rename, you didn't get an error, you got \nsome unreliable behavior - very much like the thing we see with ufsd. But \nwith those problems, we could fix it by just always making the link and \nthe rename be within a single directory.\n\nNow, it seems, even being in the same directory isn't sufficient for that \nufsd thing (but rename works. Knock wood).\n\n\t\t\tLinus\n"},{"id":"112458","messageId":"alpine.LFD.2.00.0904271314130.22156@localhost.localdomain","threadId":"19023","inReplyTo":"7vljpl3m8i.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-27T20:18:19Z","receivedAt":"2009-04-27T20:18:19Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 27 Apr 2009, Junio C Hamano wrote:\n> >\n> > \t[core]\n> > \t\tusehardlinks = true/false\n> \n> I am a bit worried about this name, too.  It may lead people to a\n> misunderstanding that we would do something magical when they do this with\n> the configuration set:\n> \n> \twget http://some.where/huge-file.mpg 1.mpg\n>         ln 1.mpg 2.mpg\n>         git add 1.mpg 2.mpg\n>         rm -f 1.mpg 2.mpg\n>         git checkout-index -a\n> \tls -i ?.mpg\n\nBtw, I do agree that maybe 'usehardlinks' is not a good name either. Maybe \nwe should make it clear that we're talking about a specific case for \nobject creation.\n\nMaybe the config option shouldn't be a boolean, but a \"how to instantiate \nobjects\". IOW, we could do\n\n\t[core]\n\t\tcreateobject = {link|rename}\n\ninstead. Maybe we some day could allow \"inplace\", for some totally broken \nsystem that supports neither renames nor links, and just wants the object \nto be created with the final name to start with.\n\n(Ok, that sounds unlikely, but I mention it because it's an example of the \nconcept. Maybe somebody likes crazy databases, and would like to have a \n\"createobject = mysql\" for some DB-backed loose object crap).\n\n\t\tLinus\n"},{"id":"112466","messageId":"7vvdopwxxa.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"alpine.LFD.2.00.0904271314130.22156@localhost.localdomain","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-27T22:10:09Z","receivedAt":"2009-04-27T22:10:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> Maybe the config option shouldn't be a boolean, but a \"how to instantiate \n> objects\". IOW, we could do\n>\n> \t[core]\n> \t\tcreateobject = {link|rename}\n>\n> instead. Maybe we some day could allow \"inplace\", for some totally broken \n> system that supports neither renames nor links, and just wants the object \n> to be created with the final name to start with.\n>\n> (Ok, that sounds unlikely, but I mention it because it's an example of the \n> concept. Maybe somebody likes crazy databases, and would like to have a \n> \"createobject = mysql\" for some DB-backed loose object crap).\n>\n> \t\tLinus\n\nMore likely is \"bigtable\", I guess ;-)\n"},{"id":"112468","messageId":"alpine.DEB.1.00.0904280027540.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"7vvdopwxxa.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-27T22:28:23Z","receivedAt":"2009-04-27T22:28:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 27 Apr 2009, Junio C Hamano wrote:\n\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n> \n> > Maybe the config option shouldn't be a boolean, but a \"how to instantiate \n> > objects\". IOW, we could do\n> >\n> > \t[core]\n> > \t\tcreateobject = {link|rename}\n> >\n> > instead. Maybe we some day could allow \"inplace\", for some totally broken \n> > system that supports neither renames nor links, and just wants the object \n> > to be created with the final name to start with.\n> >\n> > (Ok, that sounds unlikely, but I mention it because it's an example of the \n> > concept. Maybe somebody likes crazy databases, and would like to have a \n> > \"createobject = mysql\" for some DB-backed loose object crap).\n> >\n> > \t\tLinus\n> \n> More likely is \"bigtable\", I guess ;-)\n\nAs I said, this is highly unlikely, as certain people made sure that the \nGoogle Code people do not like Git.\n\nCiao,\nDscho\n"},{"id":"112469","messageId":"alpine.DEB.1.00.0904280031100.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"alpine.LFD.2.00.0904271314130.22156@localhost.localdomain","subject":"[PATCH] Rename core.unreliableHardlinks to core.createObject","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-27T22:32:25Z","receivedAt":"2009-04-27T22:32:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n\"Unreliable hardlinks\" is a misleading description for what is happening.\nSo rename it to something less misleading.\n\nSuggested by Linus Torvalds.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Mon, 27 Apr 2009, Linus Torvalds wrote:\n\n\t> Maybe the config option shouldn't be a boolean, but a \"how to \n\t> instantiate objects\". IOW, we could do\n\t> \n\t> \t[core]\n\t> \t\tcreateobject = {link|rename}\n\t> \n\t> instead. Maybe we some day could allow \"inplace\", for some \n\t> totally broken system that supports neither renames nor links, and\n\t> just wants the object to be created with the final name to start\n\t> with.\n\n\tHere you go.  Only compile-tested.\n\n Documentation/config.txt |   12 ++++++++----\n Makefile                 |   10 +++++-----\n cache.h                  |    7 ++++++-\n config.c                 |    9 +++++++--\n environment.c            |    6 +++---\n sha1_file.c              |    2 +-\n 6 files changed, 30 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 83454c5..2c03162 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -429,10 +429,14 @@ relatively high IO latencies.  With this set to 'true', git will do the\n index comparison to the filesystem data in parallel, allowing\n overlapping IO's.\n \n-core.unreliableHardlinks::\n-\tSome filesystem drivers cannot properly handle hardlinking a file\n-\tand deleting the source right away.  In such a case, you need to\n-\tset this config variable to 'true'.\n+core.createObject::\n+\tYou can set this to 'link', in which case a hardlink followed by\n+\ta delete of the source are used to make sure that object creation\n+\twill not overwrite existing objects.\n++\n+On some file system/operating system combinations, this is unreliable.\n+Set this config setting to 'rename' there; However, This will remove the\n+check that makes sure that existing object files will not get overwritten.\n \n alias.*::\n \tCommand aliases for the linkgit:git[1] command wrapper - e.g.\ndiff --git a/Makefile b/Makefile\nindex 5c8e83a..9ca1826 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -172,8 +172,8 @@ all::\n # information on a not yet closed file that lstat would return for the same\n # file after it was closed.\n #\n-# Define UNRELIABLE_HARDLINKS if your operating systems has problems when\n-# hardlinking a file to another name and unlinking the original file right\n+# Define OBJECT_CREATION_USES_RENAMES if your operating systems has problems\n+# when hardlinking a file to another name and unlinking the original file right\n # away (some NTFS drivers seem to zero the contents in that scenario).\n \n GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n@@ -839,7 +839,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tNO_NSEC = YesPlease\n \tUSE_WIN32_MMAP = YesPlease\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n-\tUNRELIABLE_HARDLINKS = UnfortunatelySometimes\n+\tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tCOMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch\n \tCOMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n@@ -1023,8 +1023,8 @@ else\n \t\tCOMPAT_OBJS += compat/win32mmap.o\n \tendif\n endif\n-ifdef UNRELIABLE_HARDLINKS\n-\tCOMPAT_CFLAGS += -DUNRELIABLE_HARDLINKS=1\n+ifdef OBJECT_CREATION_USES_RENAMES\n+\tCOMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1\n endif\n ifdef NO_PREAD\n \tCOMPAT_CFLAGS += -DNO_PREAD\ndiff --git a/cache.h b/cache.h\nindex ff9e145..d0d48b4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -554,7 +554,12 @@ extern enum branch_track git_branch_track;\n extern enum rebase_setup_type autorebase;\n extern enum push_default_type push_default;\n \n-extern int unreliable_hardlinks;\n+enum object_creation_mode {\n+\tOBJECT_CREATION_USES_HARDLINKS = 0,\n+\tOBJECT_CREATION_USES_RENAMES = 1,\n+};\n+\n+extern enum object_creation_mode object_creation_mode;\n \n #define GIT_REPO_VERSION 0\n extern int repository_format_version;\ndiff --git a/config.c b/config.c\nindex 1750cfb..876f0ed 100644\n--- a/config.c\n+++ b/config.c\n@@ -495,8 +495,13 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.unreliablehardlinks\")) {\n-\t\tunreliable_hardlinks = git_config_bool(var, value);\n+\tif (!strcmp(var, \"core.createobject\")) {\n+\t\tif (!strcmp(value, \"rename\"))\n+\t\t\tobject_creation_mode = OBJECT_CREATION_USES_RENAMES;\n+\t\telse if (!strcmp(value, \"link\"))\n+\t\t\tobject_creation_mode = OBJECT_CREATION_USES_HARDLINKS;\n+\t\telse\n+\t\t\tdie (\"Invalid mode for object creation: %s\", value);\n \t\treturn 0;\n \t}\n \ndiff --git a/environment.c b/environment.c\nindex 10578d2..801a005 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -43,10 +43,10 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n-#ifndef UNRELIABLE_HARDLINKS\n-#define UNRELIABLE_HARDLINKS 0\n+#ifndef OBJECT_CREATION_MODE\n+#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS\n #endif\n-int unreliable_hardlinks = UNRELIABLE_HARDLINKS;\n+enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;\n \n /* Parallel index stat data preload? */\n int core_preload_index = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 11969fc..f708cf4 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2225,7 +2225,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n {\n \tint ret = 0;\n \n-\tif (unreliable_hardlinks)\n+\tif (object_creation_mode == OBJECT_CREATION_USES_RENAMES)\n \t\tgoto try_rename;\n \telse if (link(tmpfile, filename))\n \t\tret = errno;\n-- \n1.6.3.rc3.326.g039c1\n"},{"id":"112475","messageId":"alpine.LFD.2.00.0904271605250.22156@localhost.localdomain","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904280027540.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v3] Add an option not to use link(src, dest) && unlink(src) when that is unreliable","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-04-27T23:06:46Z","receivedAt":"2009-04-27T23:06:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 28 Apr 2009, Johannes Schindelin wrote:\n>\n> As I said, this is highly unlikely, as certain people made sure that the \n> Google Code people do not like Git.\n\nThere are actually lots of sane and educated people inside google. Many of \nthem freely acknowledge what a horrid thing SVN is.\n\n\t\t\tLinus\n"},{"id":"112485","messageId":"7vws95vete.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904280031100.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Rename core.unreliableHardlinks to core.createObject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-27T23:48:13Z","receivedAt":"2009-04-27T23:48:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> diff --git a/Makefile b/Makefile\n> index 5c8e83a..9ca1826 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -172,8 +172,8 @@ all::\n>  # information on a not yet closed file that lstat would return for the same\n>  # file after it was closed.\n>  #\n> -# Define UNRELIABLE_HARDLINKS if your operating systems has problems when\n> -# hardlinking a file to another name and unlinking the original file right\n> +# Define OBJECT_CREATION_USES_RENAMES if your operating systems has problems\n> +# when hardlinking a file to another name and unlinking the original file right\n\nWith the configuration variable for this relatively obscure feature in\nplace, I wonder if we can simply get rid of the hardcoded compilation\npreference.  After all, even on your eeepc, I presume that you have some\nfilesystems in native format where you do not have the breakages, and some\nothers mounted with unfsd breakage.  When diagnosing a possible issue on\nsomebody else's box, having to look into .git/config to see which codepath\nis used is bad enough, but it is even worse to have a default that can be\ndifferent with compilation switch.\n\nIt would essentially boil down to this hunk; instead of introducing\nOBJECT_CREATION_MODE, we default to hardlinks, and let the configuration\noverride it (and do nothing else).\n\n> diff --git a/environment.c b/environment.c\n> index 10578d2..801a005 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -43,10 +43,10 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n>  enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n>  enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n>  enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n> -#ifndef UNRELIABLE_HARDLINKS\n> -#define UNRELIABLE_HARDLINKS 0\n> +#ifndef OBJECT_CREATION_MODE\n> +#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS\n>  #endif\n> -int unreliable_hardlinks = UNRELIABLE_HARDLINKS;\n> +enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;\n"},{"id":"112500","messageId":"alpine.DEB.1.00.0904281022070.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"7vws95vete.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Rename core.unreliableHardlinks to core.createObject","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-28T08:23:08Z","receivedAt":"2009-04-28T08:23:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 27 Apr 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > diff --git a/Makefile b/Makefile\n> > index 5c8e83a..9ca1826 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -172,8 +172,8 @@ all::\n> >  # information on a not yet closed file that lstat would return for the same\n> >  # file after it was closed.\n> >  #\n> > -# Define UNRELIABLE_HARDLINKS if your operating systems has problems when\n> > -# hardlinking a file to another name and unlinking the original file right\n> > +# Define OBJECT_CREATION_USES_RENAMES if your operating systems has problems\n> > +# when hardlinking a file to another name and unlinking the original file right\n> \n> With the configuration variable for this relatively obscure feature in \n> place, I wonder if we can simply get rid of the hardcoded compilation \n> preference.\n\nI'd rather not, for Windows.  Remember, it fixes issues 222 and 229.  And \nfrom the comments in those issues I understand that more than 2 persons \nhad problems due to these issues.\n\nCiao,\nDscho\n"},{"id":"112502","messageId":"7v1vrdqi9i.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904281022070.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Rename core.unreliableHardlinks to core.createObject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-28T08:44:57Z","receivedAt":"2009-04-28T08:44:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> With the configuration variable for this relatively obscure feature in \n>> place, I wonder if we can simply get rid of the hardcoded compilation \n>> preference.\n>\n> I'd rather not, for Windows.  Remember, it fixes issues 222 and 229.\n\nWait a bit. Wasn't this about you accessing NTFS on your EeePC via unfs\nfrom the Linux side?\n"},{"id":"112521","messageId":"alpine.DEB.1.00.0904281647350.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"7v1vrdqi9i.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Rename core.unreliableHardlinks to core.createObject","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-28T14:50:01Z","receivedAt":"2009-04-28T14:50:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 28 Apr 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> With the configuration variable for this relatively obscure feature \n> >> in place, I wonder if we can simply get rid of the hardcoded \n> >> compilation preference.\n> >\n> > I'd rather not, for Windows.  Remember, it fixes issues 222 and 229.\n> \n> Wait a bit. Wasn't this about you accessing NTFS on your EeePC via unfs \n> from the Linux side?\n\nBoth.  I realized that there was a problem with the ufsd driver of the \nXandros Linux on my EeePC, accessing NTFS partitions.  (This is the issue \nthat made me add a config variable, but which was solved by Linus' \ncore.fsyncobjects suggestion.)\n\nLater I had a hunch that the issues 222 and 229 of msysGit might have \nexactly the same reason, let the reporters test, and indeed, the problems \nwent away.\n\nBut come to think of it, we can _easily_ just set core.createObject=rename \nin msysGit, so I agree that there is no longer a need for the Makefile \nvariable.\n\nWant me to resend?\n\nCiao,\nDscho\n"},{"id":"112574","messageId":"7vk554jxzm.fsf@gitster.siamese.dyndns.org","threadId":"19023","inReplyTo":"alpine.DEB.1.00.0904281647350.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Rename core.unreliableHardlinks to core.createObject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-28T20:59:25Z","receivedAt":"2009-04-28T20:59:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Wait a bit. Wasn't this about you accessing NTFS on your EeePC via unfs \n>> from the Linux side?\n>\n> Both.  I realized that there was a problem with the ufsd driver of the \n> Xandros Linux on my EeePC, accessing NTFS partitions.  (This is the issue \n> that made me add a config variable, but which was solved by Linus' \n> core.fsyncobjects suggestion.)\n>\n> Later I had a hunch that the issues 222 and 229 of msysGit might have \n> exactly the same reason, let the reporters test, and indeed, the problems \n> went away.\n>\n> But come to think of it, we can _easily_ just set core.createObject=rename \n> in msysGit, so I agree that there is no longer a need for the Makefile \n> variable.\n>\n> Want me to resend?\n\nIf it helps msys, I think we should allow compiling things in, but this\n\"compiled in default for the platform, and possible per-repository\noverride\" made me a bit confused:\n\n (1) in your \"This is Linux and on sane filesystems I do not weaken it to\n     rename but on this one filesystem I do\" case can be handled by adding\n     .git/config in that repository;\n\n (2) problems with msysgit can be handled by compiled-in defaults as long\n     as the user does not have .git/config entry to say \"link\";\n\n (3) if you use the same repository from both sides with (1), presumably\n     by dual-booting, so having .git/config that says \"rename\" happens to\n     work;\n\n (4) if somebody has a dual-boot setup and shares a repository hosted\n     natively on the Linux side by mounting it on the Windows side (Ext2\n     IFS?), I wonder what should happen.  While you are using the\n     repository from the Linux side, you may not want to weaken it to use\n     \"rename\" (so you do not add .git/config that says \"rename\").  When\n     you are accessing it over Ext2 IFS, perhaps you would want to use\n     \"rename\" (I do not know about the details of #222 and #229, so it may\n     not applicable, though).\n\nSo,... as long as you do not have a triple-boot setup, third system among\nwhich wants to use \"link\" on a repository where both Linux and Windows\nside want to use \"rename\", I think you are Ok.\n"},{"id":"112567","messageId":"alpine.DEB.1.00.0904290005010.10279@pacific.mpi-cbg.de","threadId":"19023","inReplyTo":"7vk554jxzm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Rename core.unreliableHardlinks to core.createObject","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-28T22:07:08Z","receivedAt":"2009-04-28T22:07:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 28 Apr 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> Wait a bit. Wasn't this about you accessing NTFS on your EeePC via \n> >> unfs from the Linux side?\n> >\n> > Both.  I realized that there was a problem with the ufsd driver of the \n> > Xandros Linux on my EeePC, accessing NTFS partitions.  (This is the \n> > issue that made me add a config variable, but which was solved by \n> > Linus' core.fsyncobjects suggestion.)\n> >\n> > Later I had a hunch that the issues 222 and 229 of msysGit might have \n> > exactly the same reason, let the reporters test, and indeed, the \n> > problems went away.\n> >\n> > But come to think of it, we can _easily_ just set \n> > core.createObject=rename in msysGit, so I agree that there is no \n> > longer a need for the Makefile variable.\n> >\n> > Want me to resend?\n> \n> If it helps msys, I think we should allow compiling things in, but this \n> \"compiled in default for the platform, and possible per-repository \n> override\" made me a bit confused:\n> \n>  (1) in your \"This is Linux and on sane filesystems I do not weaken it \n>      to rename but on this one filesystem I do\" case can be handled by \n>      adding .git/config in that repository;\n> \n>  (2) problems with msysgit can be handled by compiled-in defaults as \n>      long as the user does not have .git/config entry to say \"link\";\n> \n>  (3) if you use the same repository from both sides with (1), presumably\n>      by dual-booting, so having .git/config that says \"rename\" happens \n>      to work;\n> \n>  (4) if somebody has a dual-boot setup and shares a repository hosted\n>      natively on the Linux side by mounting it on the Windows side (Ext2 \n>      IFS?), I wonder what should happen.  While you are using the \n>      repository from the Linux side, you may not want to weaken it to \n>      use \"rename\" (so you do not add .git/config that says \"rename\").  \n>      When you are accessing it over Ext2 IFS, perhaps you would want to \n>      use \"rename\" (I do not know about the details of #222 and #229, so \n>      it may not applicable, though).\n> \n> So,... as long as you do not have a triple-boot setup, third system \n> among which wants to use \"link\" on a repository where both Linux and \n> Windows side want to use \"rename\", I think you are Ok.\n\nWell, my idea was to \"weaken\" the repository by using rename() always, \neven if the fs/OS combo happens to handle the case gracefully.\n\nBut actually, what I will use is core.fsyncObjects, as suggested by Linus, \nwhich should not hurt sane fs/OS combos either.\n\nCiao,\nDscho\n"}]}