summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <junkio@cox.net>2007-05-11 22:26:08 -0700
committerJunio C Hamano <junkio@cox.net>2007-05-11 22:26:08 -0700
commit64cab59159d1308365d56e218a10bfc0d3cd0fd0 (patch)
tree8a02932b0aca5f8bab2d7cfcda7fce2bb4db43be
parentf859c846e90b385c7ef873df22403529208ade50 (diff)
downloadgit-64cab59159d1308365d56e218a10bfc0d3cd0fd0.tar.gz
apply: do not get confused by symlinks in the middle
HPA noticed that git-rebase fails when changes involve symlinks in the middle of the hierarchy. Consider: * The tree state before the patch is applied has arch/x86_64/boot as a symlink pointing at ../i386/boot/ * The patch tries to remove arch/x86_64/boot symlink, and create bunch of files there: .gitignore, Makefile, etc. git-apply tries to be careful while applying patches; it never touches the working tree until it is convinced that the patch would apply cleanly. One of the check it does is that when it knows a path is going to be created by the patch, it runs lstat() on the path to make sure it does not exist. This leads to a false alarm. Because we do not touch the working tree before all the check passes, when we try to make sure that arch/x86_64/boot/.gitignore does not exist yet, we haven't removed the arch/x86_64/boot symlink. The lstat() check ends up seeing arch/i386/boot/.gitignore through the yet-to-be-removed symlink, and says "Hey, you already have a file there, but what you fed me is a patch to create a new file. I am not going to clobber what you have in the working tree." We have similar checks to see a file we are going to modify does exist and match the preimage of the diff, which is done by directly opening and reading the file. For a file we are going to delete, we make sure that it does exist and matches what is going to be removed (a removal patch records the full preimage, so we check what you have in your working tree matches it in full -- otherwise we would risk losing your local changes), which again is done by directly opening and reading the file. These checks need to be adjusted so that they are not fooled by symlinks in the middle. - To make sure something does not exist, first lstat(). If it does not exist, it does not, so be happy. If it _does_, we might be getting fooled by a symlink in the middle, so break leading paths and see if there are symlinks involved. When we are checking for a path a/b/c/d, if any of a, a/b, a/b/c is a symlink, then a/b/c/d does _NOT_ exist, for the purpose of our test. This would fix this particular case you saw, and would not add extra overhead in the usual case. - To make sure something already exists, first lstat(). If it does not exist, barf (up to this, we already do). Even if it does seem to exist, we might be getting fooled by a symlink in the middle, so make sure leading paths are not symlinks. This would make the normal codepath much more expensive for deep trees, which is a bit worrisome. This patch implements the first side of the check "making sure it does not exist". The latter "making sure it exists" check is not done yet, so applying the patch in reverse would still fail, but we have to start from somewhere. Signed-off-by: Junio C Hamano <junkio@cox.net>
-rw-r--r--Makefile2
-rw-r--r--builtin-apply.c35
-rwxr-xr-xt/t4122-apply-symlink-inside.sh57
3 files changed, 84 insertions, 10 deletions
diff --git a/Makefile b/Makefile
index 7cf146ba7b..29243c6e8b 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 f94d0dbf48..8b8705a6c0 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2009,6 +2009,29 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
return 0;
}
+static int check_to_create_blob(const char *new_name, int ok_if_exists)
+{
+ struct stat nst;
+ if (!lstat(new_name, &nst)) {
+ if (S_ISDIR(nst.st_mode) || ok_if_exists)
+ return 0;
+ /*
+ * A leading component of new_name might be a symlink
+ * that is going to be removed with this patch, but
+ * still pointing at somewhere that has the path.
+ * In such a case, path "new_name" does not exist as
+ * far as git is concerned.
+ */
+ if (has_symlink_leading_path(new_name, NULL))
+ return 0;
+
+ return error("%s: already exists in working directory", new_name);
+ }
+ else if ((errno != ENOENT) && (errno != ENOTDIR))
+ return error("%s: %s", new_name, strerror(errno));
+ return 0;
+}
+
static int check_patch(struct patch *patch, struct patch *prev_patch)
{
struct stat st;
@@ -2095,15 +2118,9 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)
!ok_if_exists)
return error("%s: already exists in index", new_name);
if (!cached) {
- struct stat nst;
- if (!lstat(new_name, &nst)) {
- if (S_ISDIR(nst.st_mode) || ok_if_exists)
- ; /* ok */
- else
- return error("%s: already exists in working directory", new_name);
- }
- else if ((errno != ENOENT) && (errno != ENOTDIR))
- return error("%s: %s", new_name, strerror(errno));
+ int err = check_to_create_blob(new_name, ok_if_exists);
+ if (err)
+ return err;
}
if (!patch->new_mode) {
if (0 < patch->is_new)
diff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh
new file mode 100755
index 0000000000..37c9a9f254
--- /dev/null
+++ b/t/t4122-apply-symlink-inside.sh
@@ -0,0 +1,57 @@
+#!/bin/sh
+
+test_description='apply to deeper directory without getting fooled with symlink'
+. ./test-lib.sh
+
+lecho () {
+ for l_
+ do
+ echo "$l_"
+ done
+}
+
+test_expect_success setup '
+
+ mkdir -p arch/i386/boot arch/x86_64 &&
+ lecho 1 2 3 4 5 >arch/i386/boot/Makefile &&
+ ln -s ../i386/boot arch/x86_64/boot &&
+ git add . &&
+ test_tick &&
+ git commit -m initial &&
+ git branch test &&
+
+ rm arch/x86_64/boot &&
+ mkdir arch/x86_64/boot &&
+ lecho 2 3 4 5 6 >arch/x86_64/boot/Makefile &&
+ git add . &&
+ test_tick &&
+ git commit -a -m second &&
+
+ git format-patch --binary -1 --stdout >test.patch
+
+'
+
+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
+
+'
+
+test_expect_success 'check result' '
+
+ git diff --exit-code master &&
+ git diff --exit-code --cached master &&
+ test_tick &&
+ git commit -m replay &&
+ T1=$(git rev-parse "master^{tree}") &&
+ T2=$(git rev-parse "HEAD^{tree}") &&
+ test "z$T1" = "z$T2"
+
+'
+
+test_done
+