Linus Torvalds <torvalds@xxxxxxxx> wrote:
> On Tue, 11 Jul 2006, Junio C Hamano wrote:
>
> > Shawn Pearce <spearce@xxxxxxxxxxx> writes:
> >
> > > Has anyone else seen this type of behavior before? Any suggestions
> > > on debugging this issue?
> >
> > I would suggest raising this (politely) to Cygwin people.
>
> Well, since it apparently works with W2000, and breaks with XP, I suspect
> it's actually Windows that just returns the wrong error code.
>
> It's entirely possible that we should just make that whole
>
> if (ret == ENOENT)
>
> go away. Yes, it's the right error code if a subdirectory is missing, and
> yes, POSIX requires it, and yes, WXP is probably just a horrible piece of
> sh*t, but on the other hand, I don't think git really has any serious
> reason to even care.
>
> So we might as well say that if the link() fails for _any_ reason, we'll
> try to see if doing the mkdir() and re-trying the link helps.
Hmm. Its a single mkdir call before we give up and tell the user
something is wrong. The following change appears to work OK here on
a reasonably POSIX compliant system (OK meaning it reports errors reasonably).
Given that this type of error (failed link) shouldn't happen
that often, except for on Coda or FAT (according to a comment in
move_temp_to_file), I guess the change is OK and comes with little
penalty. But for Coda and FAT users things are going to slow down a
little bit as we try mkdir for every new loose object being created
before we try rename.
Tomorrow when I get access to my Cygwin system again I'll try to
write up a tiny test case which shows the error behavior we are
seeing and send it to the Cygwin mailing list, as this really does
seem to be a Cygwin or Windows issue. But of course having GIT
handle this case slightly better wouldn't be bad either. :-)
diff --git a/sha1_file.c b/sha1_file.c
index 8734d50..db4bddc 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1336,26 +1336,23 @@ static int link_temp_to_file(const char
return 0;
/*
- * Try to mkdir the last path component if that failed
- * with an ENOENT.
+ * Try to mkdir the last path component if that failed.
*
* Re-try the "link()" regardless of whether the mkdir
* succeeds, since a race might mean that somebody
* else succeeded.
*/
ret = errno;
- if (ret == ENOENT) {
- char *dir = strrchr(filename, '/');
- if (dir) {
- *dir = 0;
- mkdir(filename, 0777);
- if (adjust_shared_perm(filename))
- return -2;
- *dir = '/';
- if (!link(tmpfile, filename))
- return 0;
- ret = errno;
- }
+ char *dir = strrchr(filename, '/');
+ if (dir) {
+ *dir = 0;
+ mkdir(filename, 0777);
+ if (adjust_shared_perm(filename))
+ return -2;
+ *dir = '/';
+ if (!link(tmpfile, filename))
+ return 0;
+ ret = errno;
}
return ret;
}
-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html