On Thu, 19 Apr 2007, Alex Riesen wrote:
> On 4/19/07, Alex Riesen <raa.lkml@xxxxxxxxx> wrote:
> > >
> > > So here's a suggested and totally untested patch. It makes the code more
> > > readable, and probably fixes *two* bugs in the process. It also simply
> > > doesn't really even care what the error actually was - the important part
> > > was not that it was a directory, but that the unlink didn't succeed!
> > >
> >
> > Well, it is a bit tested now. I'll repeat the testing tomorrow on that
> > windows box.
>
> Tested on windows too. Works.
Good. Junio, I'd really suggest applying it. The old code was literally
wrong, and depended on an error return that seems to be Linux-specific. It
was also pretty ugly.
Here's the patch again with proper sign-off and a commentary..
As mentioned, maybe this wants expanding in the future, but regardless,
the patch not only fixes a git problem on windows (and quite possibly
other unixes too), any extensions will be much easier on top of this.
Linus
---
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Subject: Fix working directory errno handling when unlinking a directory
Alex Riesen noticed that the case where a file replaced a directory entry
in the working tree was broken on cygwin. It turns out that the code made
some Linux-specific assumptions, and also ignored errors entirely for the
case where the entry was a symlink rather than a file.
This cleans it up by separating out the common case into a function of its
own, so that both regular files and symlinks can share it, and by making
the error handling more obvious (and not depend on any Linux-specific
behaviour).
Acked-by: Alex Riesen <raa.lkml@xxxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
merge-recursive.c | 54 ++++++++++++++++++++++++++++------------------------
1 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 595b022..cea6c87 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -574,6 +574,31 @@ static void flush_buffer(int fd, const char *buf, unsigned long size)
}
}
+static int make_room_for_path(const char *path)
+{
+ int status;
+ const char *msg = "failed to create path '%s'%s";
+
+ status = mkdir_p(path, 0777);
+ if (status) {
+ if (status == -3) {
+ /* something else exists */
+ error(msg, path, ": perhaps a D/F conflict?");
+ return -1;
+ }
+ die(msg, path, "");
+ }
+
+ /* Successful unlink is good.. */
+ if (!unlink(path))
+ return 0;
+ /* .. and so is no existing file */
+ if (errno == ENOENT)
+ return 0;
+ /* .. but not some other error (who really cares what?) */
+ return error(msg, path, ": perhaps a D/F conflict?");
+}
+
static void update_file_flags(const unsigned char *sha,
unsigned mode,
const char *path,
@@ -594,33 +619,12 @@ static void update_file_flags(const unsigned char *sha,
if (type != OBJ_BLOB)
die("blob expected for %s '%s'", sha1_to_hex(sha), path);
+ if (make_room_for_path(path) < 0) {
+ update_wd = 0;
+ goto update_index;
+ }
if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
int fd;
- int status;
- const char *msg = "failed to create path '%s'%s";
-
- status = mkdir_p(path, 0777);
- if (status) {
- if (status == -3) {
- /* something else exists */
- error(msg, path, ": perhaps a D/F conflict?");
- update_wd = 0;
- goto update_index;
- }
- die(msg, path, "");
- }
- if (unlink(path)) {
- if (errno == EISDIR) {
- /* something else exists */
- error(msg, path, ": perhaps a D/F conflict?");
- update_wd = 0;
- goto update_index;
- }
- if (errno != ENOENT)
- die("failed to unlink %s "
- "in preparation to update: %s",
- path, strerror(errno));
- }
if (mode & 0100)
mode = 0777;
else
-
To unsubscribe from this list: 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