Junio C Hamano <junkio@xxxxxxx> writes:
> But that is a bit tricky. This is not on the aggressive path,
> and the merge result is decided by the policy implemented by the
> caller of read-tree. So in that sense we should not be doing
> the working tree check ourselves either. We just should leave
> that to the caller.
>
> Hence, I think removing the above "else if" part altogether is
> the right thing to do here.
>
> ---
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3ac0289..b1d78b8 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -661,8 +661,6 @@ int threeway_merge(struct cache_entry **
> if (index) {
> verify_uptodate(index, o);
> }
> - else if (path)
> - verify_absent(path, "overwritten", o);
>
> o->nontrivial_merge = 1;
>
Note note note. The above patch alone leaves merge risky to
remove an untracked working tree files, and needs to be
compensated by corresponding checks to the git-merge-xxx
strategies. The original code was overcautious, but was
protecting valid cases too.
For example, you and I recently independently did something
called show-refs (mine was actually called show-ref but I could
have picked a name that happened to conflict with yours), and it
was when I had an uncommitted, not even in index, work-in-progress
when I saw your version. If I pulled from you, the version of
read-tree without above check would have happily said it is OK
to do a three-way merge, and git-merge-one-file would have said
you added one while I haven't, and would have tried to overwrite
the file in my working tree.
But this still feels wrong, at two levels.
For one thing, the beauty of git merge was that if there is a
risk of local changes being lost, it was detected at read-tree
stage and we did not even touch index in that case. Not
detecting problems at read-tree time and leaving it to
merge-one-file feels wrong. Very wrong.
I suspect the other issue I have is easier to address -- if we
were to implement the check at merge-one-file level, it would be
something like the attached patch, but at the same time it
should take .gitignore file into account.
---
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index fba4b0c..25aedb7 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -23,6 +23,9 @@ #
"$1.." | "$1.$1" | "$1$1.")
if [ "$2" ]; then
echo "Removing $4"
+ elif test -f "$4"
+ echo "ERROR: untracked $4 is removed by the merge."
+ exit 1
fi
if test -f "$4"; then
rm -f -- "$4" &&
@@ -34,8 +37,16 @@ #
#
# Added in one.
#
-".$2." | "..$3" )
+".$2.")
+ # the other side did not add and we added so there is nothing
+ # to be done.
+ ;;
+"..$3")
echo "Adding $4"
+ test -f "$4" || {
+ echo "ERROR: untracked $4 is overwritten by the merge."
+ exit 1
+ }
git-update-index --add --cacheinfo "$6$7" "$2$3" "$4" &&
exec git-checkout-index -u -f -- "$4"
;;
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html