Hi Duy
On 19/03/2019 09:39, Nguyễn Thái Ngọc Duy wrote:
If you have staged changes in path A and perform 'checkout
--merge' (which could result in conflicts in a totally unrelated path
B), changes in A will be gone. Which is unexpected. We are supposed
to keep all changes, or kick and scream otherwise.
This is the result of how --merge is implemented, from the very first
day in 1be0659efc (checkout: merge local modifications while switching
branches., 2006-01-12):
1. a merge is done, unmerged entries are collected
2. a hard switch to a new branch is done, then unmerged entries added
back
There is no trivial fix for this. Going with 3-way merge one file at a
time loses rename detection. Going with 3-way merge by trees requires
teaching the algorithm to pick up staged changes. And even if we detect
staged changes with --merge and abort for safety, an option to continue
--merge is very weird. Such an option would keep worktree changes, but
drop staged changes.
Because the problem has been with us since the introduction of --merge
and everybody has been pretty happy (except Phillip, who found this
problem), I'll just take a note here to acknowledge it and wait for
merge wizards to come in and work their magic. There may be a way
forward [1].
[1] CABPp-BFoL_U=bzON4SEMaQSKU2TKwnOgNqjt5MUaOejTKGUJxw@xxxxxxxxxxxxxx
Reported-by: Phillip Wood <phillip.wood123@xxxxxxxxx>
I try to use phillip.wood@xxxxxxxxxxxxx for git stuff as it shouldn't
change in the future.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
---
This is my "fix" for Phillip's second problem. I chose to reply here
because this is where an actual fix was discussed. The test script to
demonstate it is here
https://public-inbox.org/git/7d3742d6-73e4-2750-6ecb-9edf761d96dd@xxxxxxxxx/
Documentation/git-checkout.txt | 2 ++
builtin/checkout.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index f179b43732..877e5f503a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -242,6 +242,8 @@ should result in deletion of the path).
+
When checking out paths from the index, this option lets you recreate
the conflicted merge in the specified paths.
++
+When switching branches with `--merge`, staged changes may be lost.
--conflict=<style>::
The same as --merge option above, but changes the way the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0e6037b296..f95e7975f7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -726,6 +726,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
struct tree *result;
struct tree *work;
struct merge_options o;
+ struct strbuf sb = STRBUF_INIT;
+
if (!opts->merge)
return 1;
@@ -736,6 +738,13 @@ static int merge_working_tree(const struct checkout_opts *opts,
if (!old_branch_info->commit)
return 1;
+ if (repo_index_has_changes(the_repository,
+ get_commit_tree(old_branch_info->commit),
+ &sb))
+ warning(_("staged changes in the following files may be lost: %s"),
+ sb.buf);
+ strbuf_release(&sb);
Thanks for doing this, I think having some sort of warning is a good
idea, I wonder if this could be quite noisy though. I guess it depends
on how many staged changes people have that don't match the new index.
If we diff against the new tree and only print names that are in both
lists does that give a definitive list of what will be lost? If it does
then if there are a lot of files affected then it will still be noisy
(using columns may help) but at least it will not contain false
positives. It is more work though, maybe we should just say "staged
changes may be lost" and leave it at that.
Best Wishes
Phillip
+
/* Do more real merge */
/*
