From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: sbeller@google.com, gitster@pobox.com, Elijah Newren <newren@gmail.com>
Subject: [PATCH v4 30/34] merge-recursive: fix overwriting dirty files involved in renames
Date: Tue, 28 Nov 2017 17:42:33 -0800 [thread overview]
Message-ID: <20171129014237.32570-31-newren@gmail.com> (raw)
In-Reply-To: <20171129014237.32570-1-newren@gmail.com>
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 directy files that are involved in directory rename
detection will be added in a subsequent commit.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
merge-recursive.c | 85 ++++++++++++++++++++++++++++---------
merge-recursive.h | 2 +
t/t3501-revert-cherry-pick.sh | 2 +-
t/t6043-merge-rename-directories.sh | 2 +-
t/t7607-merge-overwrite.sh | 2 +-
unpack-trees.c | 4 +-
unpack-trees.h | 4 ++
7 files changed, 77 insertions(+), 24 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index e77d2b043d..2b8a5ca03b 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -334,32 +334,37 @@ static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
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 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
- 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);
- 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;
}
@@ -792,6 +797,20 @@ static int would_lose_untracked(const char *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;
@@ -2645,6 +2664,7 @@ static int handle_modify_delete(struct merge_options *o,
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,
@@ -2719,7 +2739,7 @@ static int merge_content(struct merge_options *o,
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);
@@ -2753,6 +2773,30 @@ static int merge_content(struct merge_options *o,
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)
@@ -2772,9 +2816,12 @@ static int process_entry(struct merge_options *o,
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;
@@ -2870,7 +2917,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. */
- 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) {
@@ -2904,7 +2951,7 @@ int merge_trees(struct merge_options *o,
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)
diff --git a/merge-recursive.h b/merge-recursive.h
index e1be27f57c..a557201a50 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,6 +1,7 @@
#ifndef MERGE_RECURSIVE_H
#define MERGE_RECURSIVE_H
+#include "unpack-trees.h"
#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 unpack_trees_options unpack_opts;
};
struct dir_rename_entry {
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 783bdbf59d..0d89f6d0f6 100755
--- 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_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 &&
diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index b1ca3d18e5..9b16280d5e 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -3157,7 +3157,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 &&
diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
index 00617dadf8..e44fb50173 100755
--- 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_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 &&
diff --git a/unpack-trees.c b/unpack-trees.c
index bf8b602901..5b922c1939 100644
--- 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);
}
-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;
diff --git a/unpack-trees.h b/unpack-trees.h
index 6c48117b84..41178ada94 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -1,6 +1,7 @@
#ifndef UNPACK_TREES_H
#define UNPACK_TREES_H
+#include "tree-walk.h"
#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);
+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,
--
2.15.0.408.g850bc54b15
next prev parent reply other threads:[~2017-11-29 1:44 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-29 1:42 [PATCH v4 00/34] Add directory rename detection to git Elijah Newren
2017-11-29 1:42 ` [PATCH v4 01/34] Tighten and correct a few testcases for merging and cherry-picking Elijah Newren
2017-11-29 1:42 ` [PATCH v4 02/34] merge-recursive: fix logic ordering issue Elijah Newren
2017-11-29 1:42 ` [PATCH v4 03/34] merge-recursive: add explanation for src_entry and dst_entry Elijah Newren
2017-11-29 1:42 ` [PATCH v4 04/34] directory rename detection: basic testcases Elijah Newren
2017-11-29 1:42 ` [PATCH v4 05/34] directory rename detection: directory splitting testcases Elijah Newren
2017-11-29 1:42 ` [PATCH v4 06/34] directory rename detection: testcases to avoid taking detection too far Elijah Newren
2017-11-29 1:42 ` [PATCH v4 07/34] directory rename detection: partially renamed directory testcase/discussion Elijah Newren
2017-11-29 1:42 ` [PATCH v4 08/34] directory rename detection: files/directories in the way of some renames Elijah Newren
2017-11-29 1:42 ` [PATCH v4 09/34] directory rename detection: testcases checking which side did the rename Elijah Newren
2017-11-29 1:42 ` [PATCH v4 10/34] directory rename detection: more involved edge/corner testcases Elijah Newren
2017-11-29 1:42 ` [PATCH v4 11/34] directory rename detection: testcases exploring possibly suboptimal merges Elijah Newren
2017-11-29 1:42 ` [PATCH v4 12/34] directory rename detection: miscellaneous testcases to complete coverage Elijah Newren
2017-11-29 1:42 ` [PATCH v4 13/34] directory rename detection: tests for handling overwriting untracked files Elijah Newren
2017-11-29 1:42 ` [PATCH v4 14/34] directory rename detection: tests for handling overwriting dirty files Elijah Newren
2017-11-29 1:42 ` [PATCH v4 15/34] merge-recursive: move the get_renames() function Elijah Newren
2017-11-29 1:42 ` [PATCH v4 16/34] merge-recursive: introduce new functions to handle rename logic Elijah Newren
2017-11-29 1:42 ` [PATCH v4 17/34] merge-recursive: fix leaks of allocated renames and diff_filepairs Elijah Newren
2017-11-29 1:42 ` [PATCH v4 18/34] merge-recursive: make !o->detect_rename codepath more obvious Elijah Newren
2017-11-29 1:42 ` [PATCH v4 19/34] merge-recursive: split out code for determining diff_filepairs Elijah Newren
2017-11-29 1:42 ` [PATCH v4 20/34] merge-recursive: add a new hashmap for storing directory renames Elijah Newren
2017-11-29 1:42 ` [PATCH v4 21/34] merge-recursive: make a helper function for cleanup for handle_renames Elijah Newren
2017-11-29 1:42 ` [PATCH v4 22/34] merge-recursive: add get_directory_renames() Elijah Newren
2017-11-29 1:42 ` [PATCH v4 23/34] merge-recursive: check for directory level conflicts Elijah Newren
2017-11-29 1:42 ` [PATCH v4 24/34] merge-recursive: add a new hashmap for storing file collisions Elijah Newren
2017-11-29 1:42 ` [PATCH v4 25/34] merge-recursive: add computation of collisions due to dir rename & merging Elijah Newren
2017-11-29 1:42 ` [PATCH v4 26/34] merge-recursive: check for file level conflicts then get new name Elijah Newren
2017-11-29 1:42 ` [PATCH v4 27/34] merge-recursive: when comparing files, don't include trees Elijah Newren
2017-11-29 1:42 ` [PATCH v4 28/34] merge-recursive: apply necessary modifications for directory renames Elijah Newren
2017-11-29 1:42 ` [PATCH v4 29/34] merge-recursive: avoid clobbering untracked files with " Elijah Newren
2017-11-29 1:42 ` Elijah Newren [this message]
2017-11-29 1:42 ` [PATCH v4 31/34] merge-recursive: fix remaining directory rename + dirty overwrite cases Elijah Newren
2017-11-29 1:42 ` [PATCH v4 32/34] directory rename detection: new testcases showcasing a pair of bugs Elijah Newren
2017-11-29 1:42 ` [PATCH v4 33/34] merge-recursive: avoid spurious rename/rename conflict from dir renames Elijah Newren
2017-11-29 1:42 ` [PATCH v4 34/34] merge-recursive: ensure we write updates for directory-renamed file Elijah Newren
2017-12-13 1:06 ` [PATCH v4 00/34] Add directory rename detection to git Junio C Hamano
2017-12-13 2:01 ` Junio C Hamano
2017-12-13 15:38 ` Elijah Newren
2017-12-13 18:15 ` Ramsay Jones
2017-12-13 19:05 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171129014237.32570-31-newren@gmail.com \
--to=newren@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).