[PATCH] read-tree -m -u: avoid getting...

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:09

When switching from a branch with both x86_64/boot/Makefile and
i386/boot/Makefile to another branch that has x86_64/boot as a
symlink pointing at ../i386/boot, the code incorrectly removed
i386/boot/Makefile.

This was because we first removed everything under x86_64/boot
to make room to create a symbolic link x86_64/boot, then removed
x86_64/boot/Makefile which no longer exists but now is pointing
at i386/boot/Makefile, thanks to the symlink we just created.

This fixes it by using the has_symlink_leading_path() function
introduced previously for git-apply in the checkout codepath.

Signed-off-by: Junio C Hamano <redacted>
---

 * This comes on top of the previous git-apply patch.

 Makefile                        |    2 +-
 builtin-apply.c                 |   36 +-----------------------------------
 cache.h                         |    1 +
 symlinks.c                      |   35 +++++++++++++++++++++++++++++++++++
 t/t4122-apply-symlink-inside.sh |    1 -
 unpack-trees.c                  |    2 ++
 6 files changed, 40 insertions(+), 37 deletions(-)
 create mode 100644 symlinks.c

diff --git a/Makefile b/Makefile
index 7cf146b..29243c6 100644
--- a/Makefile
+++ b/Makefile

@@ -318,7 +318,7 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o
+	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \

diff --git a/builtin-apply.c b/builtin-apply.c
index 38d20ef..01acba8 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c

@@ -2009,40 +2009,6 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
 	return 0;
 }
 
-static int has_symlink_component(const char *new_name)
-{
-	char path[PATH_MAX];
-	const char *sp, *ep;
-	char *dp;
-
-	sp = new_name;
-	dp = path;
-
-	while (1) {
-		size_t len;
-		struct stat st;
-
-		ep = strchr(sp, '/');
-		if (!ep)
-			break;
-		len = ep - sp;
-		if (PATH_MAX <= dp + len - path + 2)
-			return 0; /* new name is longer than that??? */
-		memcpy(dp, sp, len);
-		dp[len] = 0;
-
-		if (lstat(path, &st))
-			return 0; /* why? we already lstat() new_name successfully. */
-		if (S_ISLNK(st.st_mode))
-			return 1;
-
-		dp[len++] = '/';
-		dp = dp + len;
-		sp = ep + 1;
-	}
-	return 0;
-}
-
 static int check_to_create_blob(const char *new_name, int ok_if_exists)
 {
 	struct stat nst;

@@ -2056,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_component(new_name))
+		if (has_symlink_leading_path(new_name))
 			return 0;
 
 		return error("%s: already exists in working directory", new_name);

diff --git a/cache.h b/cache.h
index 8e76152..ab66263 100644
--- a/cache.h
+++ b/cache.h

@@ -410,6 +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 struct alternate_object_database {
 	struct alternate_object_database *next;

diff --git a/symlinks.c b/symlinks.c
new file mode 100644
index 0000000..cfecfcf
--- /dev/null
+++ b/symlinks.c

@@ -0,0 +1,35 @@
+#include "cache.h"
+
+int has_symlink_leading_path(const char *name)
+{
+	char path[PATH_MAX];
+	const char *sp, *ep;
+	char *dp;
+
+	sp = name;
+	dp = path;
+
+	while (1) {
+		size_t len;
+		struct stat st;
+
+		ep = strchr(sp, '/');
+		if (!ep)
+			break;
+		len = ep - sp;
+		if (PATH_MAX <= dp + len - path + 2)
+			return 0; /* new name is longer than that??? */
+		memcpy(dp, sp, len);
+		dp[len] = 0;
+
+		if (lstat(path, &st))
+			return 0;
+		if (S_ISLNK(st.st_mode))
+			return 1;
+
+		dp[len++] = '/';
+		dp = dp + len;
+		sp = ep + 1;
+	}
+	return 0;
+}

diff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh
index 37c9a9f..3ddfe64 100755
--- a/t/t4122-apply-symlink-inside.sh
+++ b/t/t4122-apply-symlink-inside.sh

@@ -34,7 +34,6 @@ test_expect_success setup '
 test_expect_success apply '
 
 	git checkout test &&
-	git reset --hard && #### checkout seems to be buggy
 	git diff --exit-code test &&
 	git diff --exit-code --cached test &&
 	git apply --index test.patch

diff --git a/unpack-trees.c b/unpack-trees.c
index 675a999..a6fa32f 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c

@@ -268,6 +268,8 @@ static void unlink_entry(char *name)
 {
 	char *cp, *prev;
 
+	if (has_symlink_leading_path(name))
+		return;
 	if (unlink(name))
 		return;
 	prev = NULL;

-- 
1.5.2.rc3.706.g498a

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

From: Alex Riesen <hidden>
Date: 2016-06-15 22:43:09

On 5/11/07, Junio C Hamano [off-list ref] wrote:

quoted hunk

@@ -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?

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:09

"Alex Riesen" [off-list ref] writes:

On 5/11/07, Junio C Hamano [off-list ref] wrote:

quoted

@@ -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.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help