merge-recursive: fix overwriting dirty files involved in renames

This fixes an issue that existed before my directory rename detection
patches that affects both normal renames and renames implied by
directory rename detection.  Additional codepaths that only affect
overwriting of dirty files that are involved in directory rename
detection will be added in a subsequent commit.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/merge-recursive.c b/merge-recursive.c
index 786990e27750e5ce3ff3138f674c58dec66a215e..0bd3f37c37bba6d7f40346372122131a4405e577 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -337,32 +337,37 @@ static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
        init_tree_desc(desc, tree->buffer, tree->size);
 }
 
        init_tree_desc(desc, tree->buffer, tree->size);
 }
 

-static int git_merge_trees(int index_only,
+static int git_merge_trees(struct merge_options *o,

                           struct tree *common,
                           struct tree *head,
                           struct tree *merge)
 {
        int rc;
        struct tree_desc t[3];
                           struct tree *common,
                           struct tree *head,
                           struct tree *merge)
 {
        int rc;
        struct tree_desc t[3];

-       struct unpack_trees_options opts;

 
 

-       memset(&opts, 0, sizeof(opts));
-       if (index_only)
-               opts.index_only = 1;
+       memset(&o->unpack_opts, 0, sizeof(o->unpack_opts));
+       if (o->call_depth)
+               o->unpack_opts.index_only = 1;

        else
        else

-               opts.update = 1;
-       opts.merge = 1;
-       opts.head_idx = 2;
-       opts.fn = threeway_merge;
-       opts.src_index = &the_index;
-       opts.dst_index = &the_index;
-       setup_unpack_trees_porcelain(&opts, "merge");
+               o->unpack_opts.update = 1;
+       o->unpack_opts.merge = 1;
+       o->unpack_opts.head_idx = 2;
+       o->unpack_opts.fn = threeway_merge;
+       o->unpack_opts.src_index = &the_index;
+       o->unpack_opts.dst_index = &the_index;
+       setup_unpack_trees_porcelain(&o->unpack_opts, "merge");

 
        init_tree_desc_from_tree(t+0, common);
        init_tree_desc_from_tree(t+1, head);
        init_tree_desc_from_tree(t+2, merge);
 
 
        init_tree_desc_from_tree(t+0, common);
        init_tree_desc_from_tree(t+1, head);
        init_tree_desc_from_tree(t+2, merge);
 

-       rc = unpack_trees(3, t, &opts);
+       rc = unpack_trees(3, t, &o->unpack_opts);
+       /*
+        * unpack_trees NULLifies src_index, but it's used in verify_uptodate,
+        * so set to the new index which will usually have modification
+        * timestamp info copied over.
+        */
+       o->unpack_opts.src_index = &the_index;

        cache_tree_free(&active_cache_tree);
        return rc;
 }
        cache_tree_free(&active_cache_tree);
        return rc;
 }


@@ -795,6 +800,20 @@ static int would_lose_untracked(const char *path)
        return !was_tracked(path) && file_exists(path);
 }
 
        return !was_tracked(path) && file_exists(path);
 }
 

+static int was_dirty(struct merge_options *o, const char *path)
+{
+       struct cache_entry *ce;
+       int dirty = 1;
+
+       if (o->call_depth || !was_tracked(path))
+               return !dirty;
+
+       ce = cache_file_exists(path, strlen(path), ignore_case);
+       dirty = (ce->ce_stat_data.sd_mtime.sec > 0 &&
+                verify_uptodate(ce, &o->unpack_opts) != 0);
+       return dirty;
+}
+

 static int make_room_for_path(struct merge_options *o, const char *path)
 {
        int status, i;
 static int make_room_for_path(struct merge_options *o, const char *path)
 {
        int status, i;


@@ -2677,6 +2696,7 @@ static int handle_modify_delete(struct merge_options *o,
 
 static int merge_content(struct merge_options *o,
                         const char *path,
 
 static int merge_content(struct merge_options *o,
                         const char *path,

+                        int file_in_way,

                         struct object_id *o_oid, int o_mode,
                         struct object_id *a_oid, int a_mode,
                         struct object_id *b_oid, int b_mode,
                         struct object_id *o_oid, int o_mode,
                         struct object_id *a_oid, int a_mode,
                         struct object_id *b_oid, int b_mode,


@@ -2751,7 +2771,7 @@ static int merge_content(struct merge_options *o,
                                return -1;
        }
 
                                return -1;
        }
 

-       if (df_conflict_remains) {
+       if (df_conflict_remains || file_in_way) {

                char *new_path;
                if (o->call_depth) {
                        remove_file_from_cache(path);
                char *new_path;
                if (o->call_depth) {
                        remove_file_from_cache(path);


@@ -2785,6 +2805,30 @@ static int merge_content(struct merge_options *o,
        return mfi.clean;
 }
 
        return mfi.clean;
 }
 

+static int conflict_rename_normal(struct merge_options *o,
+                                 const char *path,
+                                 struct object_id *o_oid, unsigned int o_mode,
+                                 struct object_id *a_oid, unsigned int a_mode,
+                                 struct object_id *b_oid, unsigned int b_mode,
+                                 struct rename_conflict_info *ci)
+{
+       int clean_merge;
+       int file_in_the_way = 0;
+
+       if (was_dirty(o, path)) {
+               file_in_the_way = 1;
+               output(o, 1, _("Refusing to lose dirty file at %s"), path);
+       }
+
+       /* Merge the content and write it out */
+       clean_merge = merge_content(o, path, file_in_the_way,
+                                   o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
+                                   ci);
+       if (clean_merge > 0 && file_in_the_way)
+               clean_merge = 0;
+       return clean_merge;
+}
+

 /* Per entry merge function */
 static int process_entry(struct merge_options *o,
                         const char *path, struct stage_data *entry)
 /* Per entry merge function */
 static int process_entry(struct merge_options *o,
                         const char *path, struct stage_data *entry)


@@ -2804,9 +2848,12 @@ static int process_entry(struct merge_options *o,
                switch (conflict_info->rename_type) {
                case RENAME_NORMAL:
                case RENAME_ONE_FILE_TO_ONE:
                switch (conflict_info->rename_type) {
                case RENAME_NORMAL:
                case RENAME_ONE_FILE_TO_ONE:

-                       clean_merge = merge_content(o, path,
-                                                   o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
-                                                   conflict_info);
+                       clean_merge = conflict_rename_normal(o,
+                                                            path,
+                                                            o_oid, o_mode,
+                                                            a_oid, a_mode,
+                                                            b_oid, b_mode,
+                                                            conflict_info);

                        break;
                case RENAME_DIR:
                        clean_merge = 1;
                        break;
                case RENAME_DIR:
                        clean_merge = 1;


@@ -2902,7 +2949,7 @@ static int process_entry(struct merge_options *o,
        } else if (a_oid && b_oid) {
                /* Case C: Added in both (check for same permissions) and */
                /* case D: Modified in both, but differently. */
        } else if (a_oid && b_oid) {
                /* Case C: Added in both (check for same permissions) and */
                /* case D: Modified in both, but differently. */

-               clean_merge = merge_content(o, path,
+               clean_merge = merge_content(o, path, 0 /* file_in_way */,

                                            o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
                                            NULL);
        } else if (!o_oid && !a_oid && !b_oid) {
                                            o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
                                            NULL);
        } else if (!o_oid && !a_oid && !b_oid) {


@@ -2943,7 +2990,7 @@ int merge_trees(struct merge_options *o,
                return 1;
        }
 
                return 1;
        }
 

-       code = git_merge_trees(o->call_depth, common, head, merge);
+       code = git_merge_trees(o, common, head, merge);

 
        if (code != 0) {
                if (show(o, 4) || o->call_depth)
 
        if (code != 0) {
                if (show(o, 4) || o->call_depth)

diff --git a/merge-recursive.h b/merge-recursive.h
index 50a4e6af4e754bcda5cabfbcd5adf25a6243e972..d863cf88676ef321aad85d395f964efe5673a805 100644 (file)
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,6 +1,7 @@
 #ifndef MERGE_RECURSIVE_H
 #define MERGE_RECURSIVE_H
 
 #ifndef MERGE_RECURSIVE_H
 #define MERGE_RECURSIVE_H
 

+#include "unpack-trees.h"

 #include "string-list.h"
 
 struct merge_options {
 #include "string-list.h"
 
 struct merge_options {


@@ -27,6 +28,7 @@ struct merge_options {
        struct strbuf obuf;
        struct hashmap current_file_dir_set;
        struct string_list df_conflict_file_set;
        struct strbuf obuf;
        struct hashmap current_file_dir_set;
        struct string_list df_conflict_file_set;

+       struct unpack_trees_options unpack_opts;

 };
 
 /*
 };
 
 /*

diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 783bdbf59db07ebf2afac83165d31b86fd6b95db..0d89f6d0f651e46251c9c3c610f711353c701893 100755 (executable)
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' '
        test_cmp expect actual
 '
 
        test_cmp expect actual
 '
 

-test_expect_failure 'cherry-pick works with dirty renamed file' '
+test_expect_success 'cherry-pick works with dirty renamed file' '

        test_commit to-rename &&
        git checkout -b unrelated &&
        test_commit unrelated &&
        test_commit to-rename &&
        git checkout -b unrelated &&
        test_commit unrelated &&

diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index 0b60eb80533a3a7004f1696567c621d079a4d780..b94ba066feb894d18d61685de43bba5ea72b857c 100755 (executable)
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -3298,7 +3298,7 @@ test_expect_success '11a-setup: Avoid losing dirty contents with simple rename'
        )
 '
 
        )
 '
 

-test_expect_failure '11a-check: Avoid losing dirty contents with simple rename' '
+test_expect_success '11a-check: Avoid losing dirty contents with simple rename' '

        (
                cd 11a &&
 
        (
                cd 11a &&
 

diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
index 9c422bcd7cc8c1b2525579ca3a7ffa6ee412ba42..dd8ab7ede182fc3c3da840fee083bf23a94ce13c 100755 (executable)
--- a/t/t7607-merge-overwrite.sh
+++ b/t/t7607-merge-overwrite.sh
@@ -92,7 +92,7 @@ test_expect_success 'will not overwrite removed file with staged changes' '
        test_cmp important c1.c
 '
 
        test_cmp important c1.c
 '
 

-test_expect_failure 'will not overwrite unstaged changes in renamed file' '
+test_expect_success 'will not overwrite unstaged changes in renamed file' '

        git reset --hard c1 &&
        git mv c1.c other.c &&
        git commit -m rename &&
        git reset --hard c1 &&
        git mv c1.c other.c &&
        git commit -m rename &&

diff --git a/unpack-trees.c b/unpack-trees.c
index bf8b6029013c31000d83d6292f83ce0d0021d5d4..5b922c19392146f61cec1a9e137b7645bab6eca6 100644 (file)
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1486,8 +1486,8 @@ static int verify_uptodate_1(const struct cache_entry *ce,
                add_rejected_path(o, error_type, ce->name);
 }
 
                add_rejected_path(o, error_type, ce->name);
 }
 

-static int verify_uptodate(const struct cache_entry *ce,
-                          struct unpack_trees_options *o)
+int verify_uptodate(const struct cache_entry *ce,
+                   struct unpack_trees_options *o)

 {
        if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))
                return 0;
 {
        if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))
                return 0;

diff --git a/unpack-trees.h b/unpack-trees.h
index 6c48117b845fbf7b983852be302e4472c5e6d651..41178ada94a4b7c5222cab7dd17d9eeb7a1956e4 100644 (file)
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -1,6 +1,7 @@
 #ifndef UNPACK_TREES_H
 #define UNPACK_TREES_H
 
 #ifndef UNPACK_TREES_H
 #define UNPACK_TREES_H
 

+#include "tree-walk.h"

 #include "string-list.h"
 
 #define MAX_UNPACK_TREES 8
 #include "string-list.h"
 
 #define MAX_UNPACK_TREES 8


@@ -78,6 +79,9 @@ struct unpack_trees_options {
 extern int unpack_trees(unsigned n, struct tree_desc *t,
                struct unpack_trees_options *options);
 
 extern int unpack_trees(unsigned n, struct tree_desc *t,
                struct unpack_trees_options *options);
 

+int verify_uptodate(const struct cache_entry *ce,
+                   struct unpack_trees_options *o);
+

 int threeway_merge(const struct cache_entry * const *stages,
                   struct unpack_trees_options *o);
 int twoway_merge(const struct cache_entry * const *src,
 int threeway_merge(const struct cache_entry * const *stages,
                   struct unpack_trees_options *o);
 int twoway_merge(const struct cache_entry * const *src,