Re: [PATCH] read-tree -m -u: avoid getting confused by intermediate symlinks.

From

Junio C Hamano <junkio@cox.net>

Date

May 11, 2007, 20:38 UTC

Message-ID

<7vps579jtp.fsf@assigned-by-dhcp.cox.net>

In-Reply-To

<7vwszfb80y.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> "Alex Riesen" <raa.lkml@gmail.com> writes:
>
>> On 5/11/07, Junio C Hamano <junkio@cox.net> wrote:
>>> @@ -268,6 +268,8 @@ static void unlink_entry(char *name)
>>>  {
>>>         char *cp, *prev;
>>>
>>> +       if (has_symlink_leading_path(name))
>>> +               return;
>>
>> This can slow down the unlink case quiet considerably.
>> Maybe the symlink paths can be cached?
>
> Yes it can, and probably doable.
>
> This is called once per each path that disappears from the
> result, relative to the current tree.  The number of calls to
> this function is potentially quite large.

 * The caller optionally allocates a buffer to hold the symlink
   that caused the check to succeed and passes it in.  This is
   an in-out parameter that the check reuses the result from the
   previous round;

 * Because we call things in the index order, removed entries
   under what used to be a directory but now is a symlink
   cluster together.  The has_symlink_leading_path() function
   will return true because they are under the same symlink;

 * When we see a "this is updated/created" entry, we know we are
   no longer inside the directory that previous round of
   last_symlink is useful, so we can clear the cached value.

So the calling sequence becomes:

	char last_symlink[PATH_MAX];

        *last_symlink = '\0';
        for each index entry { 
		if (lose)
                	unlink_entry(it, last_symlink);
		else if (update) {
			checkout_entry(it);
                        *last_symlink = '\0';
	}

---

diff --git a/builtin-apply.c b/builtin-apply.c
index 01acba8..8b8705a 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2022,7 +2022,7 @@ static int check_to_create_blob(const char *new_name, int ok_if_exists)
 		 * In such a case, path "new_name" does not exist as
 		 * far as git is concerned.
 		 */
-		if (has_symlink_leading_path(new_name))
+		if (has_symlink_leading_path(new_name, NULL))
 			return 0;
 
 		return error("%s: already exists in working directory", new_name);
diff --git a/cache.h b/cache.h
index ab66263..aaeb04a 100644
--- a/cache.h
+++ b/cache.h
@@ -410,7 +410,7 @@ struct checkout {
 };
 
 extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
-extern int has_symlink_leading_path(const char *name);
+extern int has_symlink_leading_path(const char *name, char *last_symlink);
 
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
diff --git a/symlinks.c b/symlinks.c
index cfecfcf..ee3f914 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -1,6 +1,6 @@
 #include "cache.h"
 
-int has_symlink_leading_path(const char *name)
+int has_symlink_leading_path(const char *name, char *last_symlink)
 {
 	char path[PATH_MAX];
 	const char *sp, *ep;
@@ -9,6 +9,16 @@ int has_symlink_leading_path(const char *name)
 	sp = name;
 	dp = path;
 
+	if (last_symlink && *last_symlink) {
+		size_t last_len = strlen(last_symlink);
+		size_t len = strlen(name); 
+		if (last_len < len &&
+		    !strncmp(name, last_symlink, last_len) &&
+		    name[last_len] == '/')
+			return 1;
+		*last_symlink = '\0';
+	}
+
 	while (1) {
 		size_t len;
 		struct stat st;
@@ -24,8 +34,11 @@ int has_symlink_leading_path(const char *name)
 
 		if (lstat(path, &st))
 			return 0;
-		if (S_ISLNK(st.st_mode))
+		if (S_ISLNK(st.st_mode)) {
+			if (last_symlink)
+				strcpy(last_symlink, path);
 			return 1;
+		}
 
 		dp[len++] = '/';
 		dp = dp + len;
diff --git a/unpack-trees.c b/unpack-trees.c
index a6fa32f..906ce69 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -264,11 +264,11 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
  * directories, in case this unlink is the removal of the
  * last entry in the directory -- empty directories are removed.
  */
-static void unlink_entry(char *name)
+static void unlink_entry(char *name, char *last_symlink)
 {
 	char *cp, *prev;
 
-	if (has_symlink_leading_path(name))
+	if (has_symlink_leading_path(name, last_symlink))
 		return;
 	if (unlink(name))
 		return;
@@ -293,11 +293,12 @@ static void unlink_entry(char *name)
 
 static struct checkout state;
 static void check_updates(struct cache_entry **src, int nr,
-		struct unpack_trees_options *o)
+			struct unpack_trees_options *o)
 {
 	unsigned short mask = htons(CE_UPDATE);
 	unsigned cnt = 0, total = 0;
 	struct progress progress;
+	char last_symlink[PATH_MAX];
 
 	if (o->update && o->verbose_update) {
 		for (total = cnt = 0; cnt < nr; cnt++) {
@@ -311,6 +312,7 @@ static void check_updates(struct cache_entry **src, int nr,
 		cnt = 0;
 	}
 
+	*last_symlink = '\0';
 	while (nr--) {
 		struct cache_entry *ce = *src++;
 
@@ -319,13 +321,15 @@ static void check_updates(struct cache_entry **src, int nr,
 				display_progress(&progress, ++cnt);
 		if (!ce->ce_mode) {
 			if (o->update)
-				unlink_entry(ce->name);
+				unlink_entry(ce->name, last_symlink);
 			continue;
 		}
 		if (ce->ce_flags & mask) {
 			ce->ce_flags &= ~mask;
-			if (o->update)
+			if (o->update) {
 				checkout_entry(ce, &state, NULL);
+				*last_symlink = '\0';
+			}
 		}
 	}
 	if (total)