Re: "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"
- From
Junio C Hamano <junkio@cox.net>
- Date
- Oct 9, 2006, 05:20 UTC
- Message-ID
- <7vodsmdq0m.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <7v7izaf62c.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
Show 23 quoted lines
> 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" ;;