* [PATCHv4 1/6] Add additional testcases for D/F conflicts
2010-07-09 13:10 [PATCHv4 0/6] D/F conflict fixes Elijah Newren
@ 2010-07-09 13:10 ` Elijah Newren
2010-07-09 13:10 ` [PATCHv4 2/6] Add a rename + D/F conflict testcase Elijah Newren
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Elijah Newren @ 2010-07-09 13:10 UTC (permalink / raw)
To: gitster; +Cc: git, agladysh, Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t6035-merge-dir-to-symlink.sh | 64 ++++++++++++++++++++++++++++++++++++--
t/t9350-fast-export.sh | 24 ++++++++++++++
2 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh
index 3202e1d..761ad9d 100755
--- a/t/t6035-merge-dir-to-symlink.sh
+++ b/t/t6035-merge-dir-to-symlink.sh
@@ -48,7 +48,7 @@ test_expect_success 'setup for merge test' '
git tag baseline
'
-test_expect_success 'do not lose a/b-2/c/d in merge (resolve)' '
+test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolve)' '
git reset --hard &&
git checkout baseline^0 &&
git merge -s resolve master &&
@@ -56,7 +56,7 @@ test_expect_success 'do not lose a/b-2/c/d in merge (resolve)' '
test -f a/b-2/c/d
'
-test_expect_failure 'do not lose a/b-2/c/d in merge (recursive)' '
+test_expect_failure 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '
git reset --hard &&
git checkout baseline^0 &&
git merge -s recursive master &&
@@ -64,6 +64,54 @@ test_expect_failure 'do not lose a/b-2/c/d in merge (recursive)' '
test -f a/b-2/c/d
'
+test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (resolve)' '
+ git reset --hard &&
+ git checkout master^0 &&
+ git merge -s resolve baseline^0 &&
+ test -h a/b &&
+ test -f a/b-2/c/d
+'
+
+test_expect_failure 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' '
+ git reset --hard &&
+ git checkout master^0 &&
+ git merge -s recursive baseline^0 &&
+ test -h a/b &&
+ test -f a/b-2/c/d
+'
+
+test_expect_failure 'do not lose untracked in merge (resolve)' '
+ git reset --hard &&
+ git checkout baseline^0 &&
+ >a/b/c/e &&
+ test_must_fail git merge -s resolve master &&
+ test -f a/b/c/e &&
+ test -f a/b-2/c/d
+'
+
+test_expect_success 'do not lose untracked in merge (recursive)' '
+ git reset --hard &&
+ git checkout baseline^0 &&
+ >a/b/c/e &&
+ test_must_fail git merge -s recursive master &&
+ test -f a/b/c/e &&
+ test -f a/b-2/c/d
+'
+
+test_expect_success 'do not lose modifications in merge (resolve)' '
+ git reset --hard &&
+ git checkout baseline^0 &&
+ echo more content >>a/b/c/d &&
+ test_must_fail git merge -s resolve master
+'
+
+test_expect_success 'do not lose modifications in merge (recursive)' '
+ git reset --hard &&
+ git checkout baseline^0 &&
+ echo more content >>a/b/c/d &&
+ test_must_fail git merge -s recursive master
+'
+
test_expect_success 'setup a merge where dir a/b-2 changed to symlink' '
git reset --hard &&
git checkout start^0 &&
@@ -74,7 +122,7 @@ test_expect_success 'setup a merge where dir a/b-2 changed to symlink' '
git tag test2
'
-test_expect_success 'merge should not have conflicts (resolve)' '
+test_expect_success 'merge should not have D/F conflicts (resolve)' '
git reset --hard &&
git checkout baseline^0 &&
git merge -s resolve test2 &&
@@ -82,7 +130,7 @@ test_expect_success 'merge should not have conflicts (resolve)' '
test -f a/b/c/d
'
-test_expect_failure 'merge should not have conflicts (recursive)' '
+test_expect_failure 'merge should not have D/F conflicts (recursive)' '
git reset --hard &&
git checkout baseline^0 &&
git merge -s recursive test2 &&
@@ -90,4 +138,12 @@ test_expect_failure 'merge should not have conflicts (recursive)' '
test -f a/b/c/d
'
+test_expect_failure 'merge should not have F/D conflicts (recursive)' '
+ git reset --hard &&
+ git checkout -b foo test2 &&
+ git merge -s recursive baseline^0 &&
+ test -h a/b-2 &&
+ test -f a/b/c/d
+'
+
test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index d43f37c..69179c6 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -376,4 +376,28 @@ test_expect_success 'tree_tag-obj' 'git fast-export tree_tag-obj'
test_expect_success 'tag-obj_tag' 'git fast-export tag-obj_tag'
test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'
+test_expect_failure 'directory becomes symlink' '
+ git init dirtosymlink &&
+ git init result &&
+ (
+ cd dirtosymlink &&
+ mkdir foo &&
+ mkdir bar &&
+ echo hello > foo/world &&
+ echo hello > bar/world &&
+ git add foo/world bar/world &&
+ git commit -q -mone &&
+ git rm -r foo &&
+ ln -s bar foo &&
+ git add foo &&
+ git commit -q -mtwo
+ ) &&
+ (
+ cd dirtosymlink &&
+ git fast-export master -- foo |
+ (cd ../result && git fast-import --quiet)
+ ) &&
+ (cd result && git show master:foo)
+'
+
test_done
--
1.7.1.1.23.gafea6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCHv4 2/6] Add a rename + D/F conflict testcase
2010-07-09 13:10 [PATCHv4 0/6] D/F conflict fixes Elijah Newren
2010-07-09 13:10 ` [PATCHv4 1/6] Add additional testcases for D/F conflicts Elijah Newren
@ 2010-07-09 13:10 ` Elijah Newren
2010-07-09 13:10 ` [PATCHv4 3/6] merge-recursive: Fix D/F conflicts Elijah Newren
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Elijah Newren @ 2010-07-09 13:10 UTC (permalink / raw)
To: gitster; +Cc: git, agladysh, Elijah Newren
From: Alexander Gladysh <agladysh@gmail.com>
This is a simple testcase where both sides of the rename are paths involved
in (separate) D/F merge conflicts
Signed-off-by: Alexander Gladysh <agladysh@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio was also curious in the last round how well the resolve strategy
handled this testcase. It passes. I could add it as a testcase, but use
of the --strategy option for cherry-pick would make the series no longer
apply to maint. However, if that is wanted, I can add it.
t/t3509-cherry-pick-merge-df.sh | 35 +++++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)
create mode 100755 t/t3509-cherry-pick-merge-df.sh
diff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh
new file mode 100755
index 0000000..7c05e16
--- /dev/null
+++ b/t/t3509-cherry-pick-merge-df.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+test_description='Test cherry-pick with directory/file conflicts'
+. ./test-lib.sh
+
+test_expect_success 'Setup rename across paths each below D/F conflicts' '
+ mkdir a &&
+ >a/f &&
+ git add a &&
+ git commit -m a &&
+
+ mkdir b &&
+ ln -s ../a b/a &&
+ git add b &&
+ git commit -m b &&
+
+ git checkout -b branch &&
+ rm b/a &&
+ mv a b/a &&
+ ln -s b/a a &&
+ git add . &&
+ git commit -m swap &&
+
+ >f1 &&
+ git add f1 &&
+ git commit -m f1
+'
+
+test_expect_failure 'Cherry-pick succeeds with rename across D/F conflicts' '
+ git reset --hard &&
+ git checkout master^0 &&
+ git cherry-pick branch
+'
+
+test_done
--
1.7.1.1.23.gafea6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCHv4 3/6] merge-recursive: Fix D/F conflicts
2010-07-09 13:10 [PATCHv4 0/6] D/F conflict fixes Elijah Newren
2010-07-09 13:10 ` [PATCHv4 1/6] Add additional testcases for D/F conflicts Elijah Newren
2010-07-09 13:10 ` [PATCHv4 2/6] Add a rename + D/F conflict testcase Elijah Newren
@ 2010-07-09 13:10 ` Elijah Newren
2010-07-09 13:10 ` [PATCHv4 4/6] merge_recursive: Fix renames across paths below " Elijah Newren
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Elijah Newren @ 2010-07-09 13:10 UTC (permalink / raw)
To: gitster; +Cc: git, agladysh, Elijah Newren
The D/F conflicts that can be automatically resolved (file or directory
unmodified on one side of history), have the nice property that
process_entry() can correctly handle all subpaths of the D/F conflict. In
the case of D->F conversions, it will correctly delete all non-conflicting
files below the relevant directory and the directory itself (note that both
untracked and conflicting files below the directory will prevent its
removal). So if we handle D/F conflicts after all other conflicts, they
become fairly simple to handle -- we just need to check for whether or not
a path (file/directory) is in the way of creating the new content. We do
this by having process_entry() defer handling such entries to a subsequent
process_df_entry() step.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio pointed out that if there's a D->F conflict with unmerged or
untracked files below the directory, and the user wants to resolve the
conflict by choosing to keep the directory, then it is quite difficult
since all the non-conflicting files under that directory will have been
deleted. I intended to add another patch to this series to address
that, however, current git already has the same problem and I wanted to
get a cleaned up series resubmitted soon.
merge-recursive.c | 93 ++++++++++++++++++++++++++++++++-------
t/t6035-merge-dir-to-symlink.sh | 6 +-
2 files changed, 80 insertions(+), 19 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 206c103..7d0c27d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1072,6 +1072,7 @@ static int process_entry(struct merge_options *o,
unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
+ entry->processed = 1;
if (o_sha && (!a_sha || !b_sha)) {
/* Case A: Deleted in one */
if ((!a_sha && !b_sha) ||
@@ -1104,33 +1105,28 @@ static int process_entry(struct merge_options *o,
} else if ((!o_sha && a_sha && !b_sha) ||
(!o_sha && !a_sha && b_sha)) {
/* Case B: Added in one. */
- const char *add_branch;
- const char *other_branch;
unsigned mode;
const unsigned char *sha;
- const char *conf;
if (a_sha) {
- add_branch = o->branch1;
- other_branch = o->branch2;
mode = a_mode;
sha = a_sha;
- conf = "file/directory";
} else {
- add_branch = o->branch2;
- other_branch = o->branch1;
mode = b_mode;
sha = b_sha;
- conf = "directory/file";
}
if (string_list_has_string(&o->current_directory_set, path)) {
- const char *new_path = unique_path(o, path, add_branch);
- clean_merge = 0;
- output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
- "Adding %s as %s",
- conf, path, other_branch, path, new_path);
- remove_file(o, 0, path, 0);
- update_file(o, 0, sha, mode, new_path);
+ /* Handle D->F conflicts after all subfiles */
+ entry->processed = 0;
+ /* But get any file out of the way now, so conflicted
+ * entries below the directory of the same name can
+ * be put in the working directory.
+ */
+ if (a_sha)
+ output(o, 2, "Removing %s", path);
+ /* do not touch working file if it did not exist */
+ remove_file(o, 0, path, !a_sha);
+ return 1; /* Assume clean till processed */
} else {
output(o, 2, "Adding %s", path);
update_file(o, 1, sha, mode, path);
@@ -1178,6 +1174,64 @@ static int process_entry(struct merge_options *o,
return clean_merge;
}
+/*
+ * Per entry merge function for D/F conflicts, to be called only after
+ * all files below dir have been processed. We do this because in the
+ * cases we can cleanly resolve D/F conflicts, process_entry() can clean
+ * out all the files below the directory for us.
+ */
+static int process_df_entry(struct merge_options *o,
+ const char *path, struct stage_data *entry)
+{
+ int clean_merge = 1;
+ unsigned o_mode = entry->stages[1].mode;
+ unsigned a_mode = entry->stages[2].mode;
+ unsigned b_mode = entry->stages[3].mode;
+ unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);
+ unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
+ unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
+ const char *add_branch;
+ const char *other_branch;
+ unsigned mode;
+ const unsigned char *sha;
+ const char *conf;
+ struct stat st;
+
+ /* We currently only handle D->F cases */
+ assert((!o_sha && a_sha && !b_sha) ||
+ (!o_sha && !a_sha && b_sha));
+
+ entry->processed = 1;
+
+ if (a_sha) {
+ add_branch = o->branch1;
+ other_branch = o->branch2;
+ mode = a_mode;
+ sha = a_sha;
+ conf = "file/directory";
+ } else {
+ add_branch = o->branch2;
+ other_branch = o->branch1;
+ mode = b_mode;
+ sha = b_sha;
+ conf = "directory/file";
+ }
+ if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
+ const char *new_path = unique_path(o, path, add_branch);
+ clean_merge = 0;
+ output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
+ "Adding %s as %s",
+ conf, path, other_branch, path, new_path);
+ remove_file(o, 0, path, 0);
+ update_file(o, 0, sha, mode, new_path);
+ } else {
+ output(o, 2, "Adding %s", path);
+ update_file(o, 1, sha, mode, path);
+ }
+
+ return clean_merge;
+}
+
struct unpack_trees_error_msgs get_porcelain_error_msgs(void)
{
struct unpack_trees_error_msgs msgs = {
@@ -1249,6 +1303,13 @@ int merge_trees(struct merge_options *o,
&& !process_entry(o, path, e))
clean = 0;
}
+ for (i = 0; i < entries->nr; i++) {
+ const char *path = entries->items[i].string;
+ struct stage_data *e = entries->items[i].util;
+ if (!e->processed
+ && !process_df_entry(o, path, e))
+ clean = 0;
+ }
string_list_clear(re_merge, 0);
string_list_clear(re_head, 0);
diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh
index 761ad9d..f6972be 100755
--- a/t/t6035-merge-dir-to-symlink.sh
+++ b/t/t6035-merge-dir-to-symlink.sh
@@ -56,7 +56,7 @@ test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolv
test -f a/b-2/c/d
'
-test_expect_failure 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '
+test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '
git reset --hard &&
git checkout baseline^0 &&
git merge -s recursive master &&
@@ -130,7 +130,7 @@ test_expect_success 'merge should not have D/F conflicts (resolve)' '
test -f a/b/c/d
'
-test_expect_failure 'merge should not have D/F conflicts (recursive)' '
+test_expect_success 'merge should not have D/F conflicts (recursive)' '
git reset --hard &&
git checkout baseline^0 &&
git merge -s recursive test2 &&
@@ -138,7 +138,7 @@ test_expect_failure 'merge should not have D/F conflicts (recursive)' '
test -f a/b/c/d
'
-test_expect_failure 'merge should not have F/D conflicts (recursive)' '
+test_expect_success 'merge should not have F/D conflicts (recursive)' '
git reset --hard &&
git checkout -b foo test2 &&
git merge -s recursive baseline^0 &&
--
1.7.1.1.23.gafea6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCHv4 4/6] merge_recursive: Fix renames across paths below D/F conflicts
2010-07-09 13:10 [PATCHv4 0/6] D/F conflict fixes Elijah Newren
` (2 preceding siblings ...)
2010-07-09 13:10 ` [PATCHv4 3/6] merge-recursive: Fix D/F conflicts Elijah Newren
@ 2010-07-09 13:10 ` Elijah Newren
2010-07-09 13:10 ` [PATCHv4 5/6] fast-export: Fix output order of D/F changes Elijah Newren
2010-07-09 13:10 ` [PATCHv4 6/6] fast-import: Improve robustness when D->F changes provided in wrong order Elijah Newren
5 siblings, 0 replies; 7+ messages in thread
From: Elijah Newren @ 2010-07-09 13:10 UTC (permalink / raw)
To: gitster; +Cc: git, agladysh, Elijah Newren
The rename logic in process_renames() handles renames and merging of file
contents and then marks files as processed. However, there may be higher
stage entries left in the index for other reasons (e.g., due to D/F
conflicts). By checking for such cases and marking the entry as not
processed, it allows process_entry() later to look at it and handle those
higher stages.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
merge-recursive.c | 15 +++++++++++++--
t/t3509-cherry-pick-merge-df.sh | 2 +-
t/t6020-merge-df.sh | 2 +-
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 7d0c27d..8f4b7d4 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1019,14 +1019,25 @@ static int process_renames(struct merge_options *o,
if (mfi.clean &&
sha_eq(mfi.sha, ren1->pair->two->sha1) &&
- mfi.mode == ren1->pair->two->mode)
+ mfi.mode == ren1->pair->two->mode) {
/*
* This messaged is part of
* t6022 test. If you change
* it update the test too.
*/
output(o, 3, "Skipped %s (merged same as existing)", ren1_dst);
- else {
+
+ /* There may be higher stage entries left
+ * in the index (e.g. due to a D/F
+ * conflict) that need to be resolved.
+ */
+ for (i = 1; i <= 3; i++) {
+ if (!ren1->dst_entry->stages[i].mode)
+ continue;
+ ren1->dst_entry->processed = 0;
+ break;
+ }
+ } else {
if (mfi.merge || !mfi.clean)
output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst);
if (mfi.merge)
diff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh
index 7c05e16..6e7ef84 100755
--- a/t/t3509-cherry-pick-merge-df.sh
+++ b/t/t3509-cherry-pick-merge-df.sh
@@ -26,7 +26,7 @@ test_expect_success 'Setup rename across paths each below D/F conflicts' '
git commit -m f1
'
-test_expect_failure 'Cherry-pick succeeds with rename across D/F conflicts' '
+test_expect_success 'Cherry-pick succeeds with rename across D/F conflicts' '
git reset --hard &&
git checkout master^0 &&
git cherry-pick branch
diff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh
index e71c687..490d397 100755
--- a/t/t6020-merge-df.sh
+++ b/t/t6020-merge-df.sh
@@ -22,7 +22,7 @@ git commit -m "File: dir"'
test_expect_code 1 'Merge with d/f conflicts' 'git merge "merge msg" B master'
-test_expect_failure 'F/D conflict' '
+test_expect_success 'F/D conflict' '
git reset --hard &&
git checkout master &&
rm .git/index &&
--
1.7.1.1.23.gafea6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCHv4 5/6] fast-export: Fix output order of D/F changes
2010-07-09 13:10 [PATCHv4 0/6] D/F conflict fixes Elijah Newren
` (3 preceding siblings ...)
2010-07-09 13:10 ` [PATCHv4 4/6] merge_recursive: Fix renames across paths below " Elijah Newren
@ 2010-07-09 13:10 ` Elijah Newren
2010-07-09 13:10 ` [PATCHv4 6/6] fast-import: Improve robustness when D->F changes provided in wrong order Elijah Newren
5 siblings, 0 replies; 7+ messages in thread
From: Elijah Newren @ 2010-07-09 13:10 UTC (permalink / raw)
To: gitster; +Cc: git, agladysh, Elijah Newren
The fast-import stream format requires incremental changes which take place
immediately, meaning that for D->F conversions all files below the relevant
directory must be deleted before the resulting file of the same name is
created. Reversing the order can result in fast-import silently deleting
the file right after creating it, resulting in the file missing from the
resulting repository.
We correct this by first sorting the diff_queue_struct in depth-first
order.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Email from Junio:
> If all you want is to force a particular order of paths in the output
> (e.g. depth first) in this one single application, the cleanest way might
> be to let the diffcore do its work and at the very end sort the elements
> in the diff_queued_diff to your liking (c.f. diffcore_fix_diff_index()
> that uses diffnamecmp() to sort the list).
This patch is my attempt to do precisely that.
builtin/fast-export.c | 29 +++++++++++++++++++++++++++++
t/t9350-fast-export.sh | 2 +-
2 files changed, 30 insertions(+), 1 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c6dd71a..965e90e 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -147,10 +147,39 @@ static void handle_object(const unsigned char *sha1)
free(buf);
}
+static int depth_first(const void *a_, const void *b_)
+{
+ const struct diff_filepair *a = *((const struct diff_filepair **)a_);
+ const struct diff_filepair *b = *((const struct diff_filepair **)b_);
+ const char *name_a, *name_b;
+ int len_a, len_b, len;
+ int cmp;
+
+ name_a = a->one ? a->one->path : a->two->path;
+ name_b = b->one ? b->one->path : b->two->path;
+
+ len_a = strlen(name_a);
+ len_b = strlen(name_b);
+ len = (len_a < len_b) ? len_a : len_b;
+
+ /* strcmp will sort 'd' before 'd/e', we want 'd/e' before 'd' */
+ cmp = memcmp(name_a, name_b, len);
+ if (cmp)
+ return cmp;
+ return (len_b - len_a);
+}
+
static void show_filemodify(struct diff_queue_struct *q,
struct diff_options *options, void *data)
{
int i;
+
+ /*
+ * Handle files below a directory first, in case they are all deleted
+ * and the directory changes to a file or symlink.
+ */
+ qsort(q->queue, q->nr, sizeof(q->queue[0]), depth_first);
+
for (i = 0; i < q->nr; i++) {
struct diff_filespec *ospec = q->queue[i]->one;
struct diff_filespec *spec = q->queue[i]->two;
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 69179c6..1ee1461 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -376,7 +376,7 @@ test_expect_success 'tree_tag-obj' 'git fast-export tree_tag-obj'
test_expect_success 'tag-obj_tag' 'git fast-export tag-obj_tag'
test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'
-test_expect_failure 'directory becomes symlink' '
+test_expect_success 'directory becomes symlink' '
git init dirtosymlink &&
git init result &&
(
--
1.7.1.1.23.gafea6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCHv4 6/6] fast-import: Improve robustness when D->F changes provided in wrong order
2010-07-09 13:10 [PATCHv4 0/6] D/F conflict fixes Elijah Newren
` (4 preceding siblings ...)
2010-07-09 13:10 ` [PATCHv4 5/6] fast-export: Fix output order of D/F changes Elijah Newren
@ 2010-07-09 13:10 ` Elijah Newren
5 siblings, 0 replies; 7+ messages in thread
From: Elijah Newren @ 2010-07-09 13:10 UTC (permalink / raw)
To: gitster; +Cc: git, agladysh, Elijah Newren
When older versions of fast-export came across a directory changing to a
symlink (or regular file), it would output the changes in the form
M 120000 :239821 dir-changing-to-symlink
D dir-changing-to-symlink/filename1
When fast-import sees the first line, it deletes the directory named
dir-changing-to-symlink (and any files below it) and creates a symlink in
its place. When fast-import came across the second line, it was previously
trying to remove the file and relevant leading directories in
tree_content_remove(), and as a side effect it would delete the symlink
that was just created. This resulted in the symlink silently missing from
the resulting repository.
To improve robustness, we ignore file deletions underneath directory names
that correspond to non-directories. This can also be viewed as a minor
optimization: since there cannot be a file and a directory with the same
name in the same directory, the file clearly can't exist so nothing needs
to be done to delete it.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
fast-import.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 309f2c5..75ed738 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1528,6 +1528,14 @@ static int tree_content_remove(
for (i = 0; i < t->entry_count; i++) {
e = t->entries[i];
if (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) {
+ if (slash1 && !S_ISDIR(e->versions[1].mode))
+ /*
+ * If p names a file in some subdirectory, and a
+ * file or symlink matching the name of the
+ * parent directory of p exists, then p cannot
+ * exist and need not be deleted.
+ */
+ return 1;
if (!slash1 || !S_ISDIR(e->versions[1].mode))
goto del_entry;
if (!e->tree)
--
1.7.1.1.23.gafea6
^ permalink raw reply related [flat|nested] 7+ messages in thread