git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"
@ 2006-10-09  0:11 Linus Torvalds
  2006-10-09  4:48 ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2006-10-09  0:11 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


Hmm. I'm getting this message annoyingly often, simply because a few files 
that used to be tracked are now generated, and so they exist in my tree 
but are no longer tracked.

However, they may be tracked in an older tree that I pull, because in that 
older tree they _do_ exist, and we get the

	fatal: Untracked working tree file 'so-and-so' would be overwritten by merge.

which is actually incorrect, because the merge result will not even 
_contain_ that untracked file any more.

So the message is misleading - we should only consider this a fatal thing 
if we actually do generate that file as part of a git-read-tree, but if a 
merge won't touch a file, it shouldn't be "overwritten".

It's true that if the _other_ end actually removed a file that we used to 
have (ie the file _disappears_ as part of the merge), then we should 
verify that that file matched what we're going to remove, but if the old 
index didn't contain the file at all, and the new index won't contain it 
either, it really should be a no-op.

IOW, I think there is one "verify_absent()" too many somewhere, where we 
check this unnecessarily. I think it's the one in "deleted_entry()" in 
unpack-trees.c, but I'm not sure.

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"
  2006-10-09  0:11 "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge" Linus Torvalds
@ 2006-10-09  4:48 ` Junio C Hamano
  2006-10-09  5:20   ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2006-10-09  4:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List

Linus Torvalds <torvalds@osdl.org> writes:

> Hmm. I'm getting this message annoyingly often, simply because a few files 
> that used to be tracked are now generated, and so they exist in my tree 
> but are no longer tracked.
>
> However, they may be tracked in an older tree that I pull, because in that 
> older tree they _do_ exist, and we get the
>
> 	fatal: Untracked working tree file 'so-and-so' would be overwritten by merge.
>
> which is actually incorrect, because the merge result will not even 
> _contain_ that untracked file any more.

> So the message is misleading - we should only consider this a fatal thing 
> if we actually do generate that file as part of a git-read-tree, but if a 
> merge won't touch a file, it shouldn't be "overwritten".
>
> It's true that if the _other_ end actually removed a file that we used to 
> have (ie the file _disappears_ as part of the merge), then we should 
> verify that that file matched what we're going to remove, but if the old 
> index didn't contain the file at all, and the new index won't contain it 
> either, it really should be a no-op.

True.

I think it is verify_absent() on l.665 in threeway_merge().

	if (index) {
		verify_uptodate(index, o);
	}
	else if (path)
		verify_absent(path, "overwritten", o);

	o->nontrivial_merge = 1;

We say "we know this path is involved in the non-trivial merge;
if the current index has it, it had better be up-to-date" (the
first "if").  I think that up to that check is fine.

However, we say that otherwise, the path should not exist in the
working tree; this should not be done unconditionally.  As you
say, the check should depend on the merge result.

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;
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"
  2006-10-09  4:48 ` Junio C Hamano
@ 2006-10-09  5:20   ` Junio C Hamano
  2006-10-09  5:48     ` [RFC/PATCH] merge: loosen overcautious "working file will be lost" check Junio C Hamano
  2006-10-09 16:03     ` "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge" Linus Torvalds
  0 siblings, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2006-10-09  5:20 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

Junio C Hamano <junkio@cox.net> 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"
 	;;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC/PATCH] merge: loosen overcautious "working file will be lost" check.
  2006-10-09  5:20   ` Junio C Hamano
@ 2006-10-09  5:48     ` Junio C Hamano
  2006-10-09 17:20       ` Luben Tuikov
  2006-10-09 16:03     ` "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge" Linus Torvalds
  1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2006-10-09  5:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

The three-way merge complained unconditionally when a path that
does not exist in the index is involved in a merge when it
existed in the working tree.  If we are merging an old version
that had that path tracked, but the path is not tracked anymore,
and if we are merging that old version in, the result will be
that the path is not tracked.  In that case we should not
complain.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * Consolidated patch to summarize a few crapoids I sent out
   tonight.

   The change to merge-one-file still does not do .gitignore
   check but that is easy to add once we know this is the right
   direction to go, which I am not sure yet.  If we can convince
   ourselves that this is the right direction we should update
   merge-recursive as well.

 git-merge-one-file.sh       |   16 ++++++++++++-
 t/t1004-read-tree-m-u-wf.sh |   53 +++++++++++++++++++++++++++++++++++++++++++
 unpack-trees.c              |    2 -
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index fba4b0c..74ad4f2 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -23,6 +23,12 @@ #
 "$1.." | "$1.$1" | "$1$1.")
 	if [ "$2" ]; then
 		echo "Removing $4"
+	else
+		# read-tree checked that index matches HEAD already,
+		# so we know we do not have this path tracked.
+		# there may be an unrelated working tree file here,
+		# which we should just leave unmolested.
+		exit 0
 	fi
 	if test -f "$4"; then
 		rm -f -- "$4" &&
@@ -34,8 +40,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"
 	;;
diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh
new file mode 100755
index 0000000..018fbea
--- /dev/null
+++ b/t/t1004-read-tree-m-u-wf.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+test_description='read-tree -m -u checks working tree files'
+
+. ./test-lib.sh
+
+# two-tree test
+
+test_expect_success 'two-way setup' '
+
+	echo >file1 file one &&
+	echo >file2 file two &&
+	git update-index --add file1 file2 &&
+	git commit -m initial &&
+
+	git branch side &&
+	git tag -f branch-point &&
+
+	echo file2 is not tracked on the master anymore &&
+	rm -f file2 &&
+	git update-index --remove file2 &&
+	git commit -a -m "master removes file2"
+'
+
+test_expect_success 'two-way not clobbering' '
+
+	echo >file2 master creates untracked file2 &&
+	if err=`git read-tree -m -u master side 2>&1`
+	then
+		echo should have complained
+		false
+	else
+		echo "happy to see $err"
+	fi
+'
+
+# three-tree test
+
+test_expect_success 'three-way not complaining' '
+
+	rm -f file2 &&
+	git checkout side &&
+	echo >file3 file three &&
+	git update-index --add file3 &&
+	git commit -a -m "side adds file3" &&
+
+	git checkout master &&
+	echo >file2 file two is untracked on the master side &&
+
+	git-read-tree -m -u branch-point master side
+'
+
+test_done
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;
 
-- 
1.4.2.3.g2c59

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"
  2006-10-09  5:20   ` Junio C Hamano
  2006-10-09  5:48     ` [RFC/PATCH] merge: loosen overcautious "working file will be lost" check Junio C Hamano
@ 2006-10-09 16:03     ` Linus Torvalds
  2006-10-09 16:55       ` Junio C Hamano
  2006-10-10  6:32       ` Junio C Hamano
  1 sibling, 2 replies; 12+ messages in thread
From: Linus Torvalds @ 2006-10-09 16:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git



On Sun, 8 Oct 2006, Junio C Hamano wrote:
> 
> 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.

I think the difference _should_ be that we only remove the local file if 
it was removed _remotely_.

In  that case, it was there in the original tree, and removing it is 
correct.

> 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.")

So I actually think that we should NOT consider these three cases to be 
the same.

There are really two distinct cases:

 - "$1.." and "$1.$1"
	The file had already been removed locally, AND WE SHOULD NOT TOUCH 
	IT.

 - "$1$1.":
	The file was removed remotely, and we SHOULD remove it locally.

In fact, we already have that as a "special case":

>  	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" &&

That

	if [ "$2" ]; then
		echo "Removing $4"

is _exactly_ that case: it is the "$1$1." case, and we already treat it 
differently, but we actually treat it differently the wrong way: we only 
print out the message for that case, but the actual "touch working tree" 
code should _also_ be affected.

If the local index doesn't change, we should not print out the "Removing 
'so-and-so'" message, but we should also not even _touch_ that file, 
because it was already "gone" as far as the local tree was concerned.

Agreed?

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"
  2006-10-09 16:03     ` "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge" Linus Torvalds
@ 2006-10-09 16:55       ` Junio C Hamano
  2006-10-10  6:32       ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2006-10-09 16:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

Linus Torvalds <torvalds@osdl.org> writes:

> If the local index doesn't change, we should not print out the "Removing 
> 'so-and-so'" message, but we should also not even _touch_ that file, 
> because it was already "gone" as far as the local tree was concerned.
>
> Agreed?

Yes.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC/PATCH] merge: loosen overcautious "working file will be lost" check.
  2006-10-09  5:48     ` [RFC/PATCH] merge: loosen overcautious "working file will be lost" check Junio C Hamano
@ 2006-10-09 17:20       ` Luben Tuikov
  2006-10-09 22:47         ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Luben Tuikov @ 2006-10-09 17:20 UTC (permalink / raw)
  To: Junio C Hamano, Linus Torvalds; +Cc: git

I think this is a good thing.

How about this case I've noticed in my trees:

After branching out, a file is deleted, only to add
a different file with the same file name.

Then any time I pull in from the trunk to merge,
merge fails with git-diff-files showing all 0's and the
file name in question.  Picture:

  Branch B       +-----------------M1---->
                /                 /
               C2 <-- git-add A  /
              /                 /
             C1 <-- git-rm A   /
            /                 /
Trunk -----+-----------------+---->

Since the common ancestor precedes git-rm, any Merge M1,
complains that file A needs resolving with git-show-files
all 0's.  I don't mind that so much and was wondering
what you thought about it.

    Luben

--- Junio C Hamano <junkio@cox.net> wrote:

> The three-way merge complained unconditionally when a path that
> does not exist in the index is involved in a merge when it
> existed in the working tree.  If we are merging an old version
> that had that path tracked, but the path is not tracked anymore,
> and if we are merging that old version in, the result will be
> that the path is not tracked.  In that case we should not
> complain.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
> 
>  * Consolidated patch to summarize a few crapoids I sent out
>    tonight.
> 
>    The change to merge-one-file still does not do .gitignore
>    check but that is easy to add once we know this is the right
>    direction to go, which I am not sure yet.  If we can convince
>    ourselves that this is the right direction we should update
>    merge-recursive as well.
> 
>  git-merge-one-file.sh       |   16 ++++++++++++-
>  t/t1004-read-tree-m-u-wf.sh |   53 +++++++++++++++++++++++++++++++++++++++++++
>  unpack-trees.c              |    2 -
>  3 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
> index fba4b0c..74ad4f2 100755
> --- a/git-merge-one-file.sh
> +++ b/git-merge-one-file.sh
> @@ -23,6 +23,12 @@ #
>  "$1.." | "$1.$1" | "$1$1.")
>  	if [ "$2" ]; then
>  		echo "Removing $4"
> +	else
> +		# read-tree checked that index matches HEAD already,
> +		# so we know we do not have this path tracked.
> +		# there may be an unrelated working tree file here,
> +		# which we should just leave unmolested.
> +		exit 0
>  	fi
>  	if test -f "$4"; then
>  		rm -f -- "$4" &&
> @@ -34,8 +40,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"
>  	;;
> diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh
> new file mode 100755
> index 0000000..018fbea
> --- /dev/null
> +++ b/t/t1004-read-tree-m-u-wf.sh
> @@ -0,0 +1,53 @@
> +#!/bin/sh
> +
> +test_description='read-tree -m -u checks working tree files'
> +
> +. ./test-lib.sh
> +
> +# two-tree test
> +
> +test_expect_success 'two-way setup' '
> +
> +	echo >file1 file one &&
> +	echo >file2 file two &&
> +	git update-index --add file1 file2 &&
> +	git commit -m initial &&
> +
> +	git branch side &&
> +	git tag -f branch-point &&
> +
> +	echo file2 is not tracked on the master anymore &&
> +	rm -f file2 &&
> +	git update-index --remove file2 &&
> +	git commit -a -m "master removes file2"
> +'
> +
> +test_expect_success 'two-way not clobbering' '
> +
> +	echo >file2 master creates untracked file2 &&
> +	if err=`git read-tree -m -u master side 2>&1`
> +	then
> +		echo should have complained
> +		false
> +	else
> +		echo "happy to see $err"
> +	fi
> +'
> +
> +# three-tree test
> +
> +test_expect_success 'three-way not complaining' '
> +
> +	rm -f file2 &&
> +	git checkout side &&
> +	echo >file3 file three &&
> +	git update-index --add file3 &&
> +	git commit -a -m "side adds file3" &&
> +
> +	git checkout master &&
> +	echo >file2 file two is untracked on the master side &&
> +
> +	git-read-tree -m -u branch-point master side
> +'
> +
> +test_done
> 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;
>  
> -- 
> 1.4.2.3.g2c59
> 
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC/PATCH] merge: loosen overcautious "working file will be lost" check.
  2006-10-09 17:20       ` Luben Tuikov
@ 2006-10-09 22:47         ` Junio C Hamano
  2006-10-10  0:01           ` Luben Tuikov
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2006-10-09 22:47 UTC (permalink / raw)
  To: ltuikov; +Cc: git

Luben Tuikov <ltuikov@yahoo.com> writes:

> I think this is a good thing.
>
> How about this case I've noticed in my trees:
>
> After branching out, a file is deleted, only to add
> a different file with the same file name.
>
> Then any time I pull in from the trunk to merge,
> merge fails with git-diff-files showing all 0's and the
> file name in question.  Picture:
>
>   Branch B       +-----------------M1---->
>                 /                 /
>                C2 <-- git-add A  /
>               /                 /
>              C1 <-- git-rm A   /
>             /                 /
> Trunk -----+-----------------+---->

Sorry I do not understand the picture; where is the common
ancestor?  If it is the left plus sign on the Trunk line, and
you are talking about what happens when making the merge M1,
then before C1 A did not exist (so Trunk tip which is the right
plus sign on the Trunk line would not either), and the other
head (tip of branch B you are pulling the trunk into) would have
one already (from C2), so I would imagine it would be "one side
adds while the other did not touch" (net effect since branch B
forked from trunk is an addition of A, while Trunk did not do
anything with respect to path A), so I do not see where any
conflict can come from.  Puzzled.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC/PATCH] merge: loosen overcautious "working file will be lost" check.
  2006-10-09 22:47         ` Junio C Hamano
@ 2006-10-10  0:01           ` Luben Tuikov
  2006-10-10  0:19             ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Luben Tuikov @ 2006-10-10  0:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > I think this is a good thing.
> >
> > How about this case I've noticed in my trees:
> >
> > After branching out, a file is deleted, only to add
> > a different file with the same file name.
> >
> > Then any time I pull in from the trunk to merge,
> > merge fails with git-diff-files showing all 0's and the
> > file name in question.  Picture:
> >
> >   Branch B       +-----------------M1---->
> >                 /                 /
> >                C2 <-- git-add A  /
> >               /                 /
> >              C1 <-- git-rm A   /
> >             /                 /
> > Trunk -----+-----------------+---->
> 
> Sorry I do not understand the picture; where is the common
> ancestor?  If it is the left plus sign on the Trunk line, and

Yes, the left plus sign is the common ancestor.

> you are talking about what happens when making the merge M1,
> then before C1 A did not exist (so Trunk tip which is the right

File A exists before C1, it does not exist after C1.  C1 removes
it.

> plus sign on the Trunk line would not either), and the other
> head (tip of branch B you are pulling the trunk into) would have
> one already (from C2), so I would imagine it would be "one side
> adds while the other did not touch" (net effect since branch B

The trunk is free to change/update file A.  In fact this is what
most likely happens.

> forked from trunk is an addition of A, while Trunk did not do
> anything with respect to path A), so I do not see where any
> conflict can come from.  Puzzled.

>From branch B's point of view, we are not interested in
any updates/changes/etc of file A coming from the trunk.
Since we've deleted that file and added our own, albeit
with the same name.

   Luben
P.S. I'm not sure if this is tenable, since it is hard
for GIT to know... or is it?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC/PATCH] merge: loosen overcautious "working file will be lost" check.
  2006-10-10  0:01           ` Luben Tuikov
@ 2006-10-10  0:19             ` Junio C Hamano
  2006-10-10  0:59               ` Luben Tuikov
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2006-10-10  0:19 UTC (permalink / raw)
  To: ltuikov; +Cc: git

Luben Tuikov <ltuikov@yahoo.com> writes:

> --- Junio C Hamano <junkio@cox.net> wrote:
>> Luben Tuikov <ltuikov@yahoo.com> writes:
>> 
>> > I think this is a good thing.
>> >
>> > How about this case I've noticed in my trees:
>> >
>> > After branching out, a file is deleted, only to add
>> > a different file with the same file name.
>> >
>> > Then any time I pull in from the trunk to merge,
>> > merge fails with git-diff-files showing all 0's and the
>> > file name in question.  Picture:
>> >
>> >   Branch B       +-----------------M1---->
>> >                 /                 /
>> >                C2 <-- git-add A  /
>> >               /                 /
>> >              C1 <-- git-rm A   /
>> >             /                 /
>> > Trunk -----+-----------------+---->
>> 
>> Sorry I do not understand the picture; where is the common
>> ancestor?  If it is the left plus sign on the Trunk line, and
>
> Yes, the left plus sign is the common ancestor.
>
>> you are talking about what happens when making the merge M1,
>> then before C1 A did not exist (so Trunk tip which is the right
>
> File A exists before C1, it does not exist after C1.  C1 removes
> it.

Ah, I misunderstood.  But then I do not understand why you see
all 0 anywhere.  Merge base has the path, branch B has it, and
trunk has it too; wouldn't it result in regular 3-way merge?

I understand you do _not_ even want a regular 3-way merge in
this case, but that is a separate issue.  You could write a new
merge strategy to traverse ancestry chain between the merge base
and each heads you are merging and notice disappearance and
reappearance of the path, but that would slow things down
tremendously for normal case and I do not think it is worth it.

You would also have exactly the same issue if you do not remove
and then add the file, but if your work on the branch involves a
significant rewrite.  Depending on how good the rewrite is,
bugfixes that happened on the trunk based on an ancient code may
not even be needed (in other words, it would not apply cleanly
anyway, but it does not matter -- the branch work is much better
or has different set of problems that the trunk fix is
irrelevant).

At that point, M1 would involve significant merge conflicts (and
not all-0 which I am still puzzled about), but I suspect that
the situation is obvious enough to the human (inspect git log
branch...trunk output and the log somewhere had better indicate
that the are unrelated), and the solution very much is different
case-by-case (most likely the person who pulls branch into trunk
would say "keep ours" for the path which would mean running "git
cat-file :2:$path >$path", or say "we are not really ready to merge
yet" and abort the entire merge; somebody on the trunk pulling
from that branch might say "I want other smaller fixes but this
total rewrite is not ready yet -- keep ours", or "now we know
this is total rewrite and the small updates on the trunk does
not matter -- take theirs and from now on we will improve on the
work done on the branch").

So in short, I do not think there is a clear-cut improvement we
can do to the tools to solve this.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC/PATCH] merge: loosen overcautious "working file will be lost" check.
  2006-10-10  0:19             ` Junio C Hamano
@ 2006-10-10  0:59               ` Luben Tuikov
  0 siblings, 0 replies; 12+ messages in thread
From: Luben Tuikov @ 2006-10-10  0:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

--- Junio C Hamano <junkio@cox.net> wrote:
> Ah, I misunderstood.  But then I do not understand why you see
> all 0 anywhere.  Merge base has the path, branch B has it, and
> trunk has it too; wouldn't it result in regular 3-way merge?
> 
> I understand you do _not_ even want a regular 3-way merge in
> this case, but that is a separate issue.  You could write a new
> merge strategy to traverse ancestry chain between the merge base
> and each heads you are merging and notice disappearance and
> reappearance of the path, but that would slow things down
> tremendously for normal case and I do not think it is worth it.
> 
> You would also have exactly the same issue if you do not remove
> and then add the file, but if your work on the branch involves a
> significant rewrite.  Depending on how good the rewrite is,
> bugfixes that happened on the trunk based on an ancient code may
> not even be needed (in other words, it would not apply cleanly
> anyway, but it does not matter -- the branch work is much better
> or has different set of problems that the trunk fix is
> irrelevant).
> 
> At that point, M1 would involve significant merge conflicts (and
> not all-0 which I am still puzzled about), but I suspect that
> the situation is obvious enough to the human (inspect git log
> branch...trunk output and the log somewhere had better indicate
> that the are unrelated), and the solution very much is different
> case-by-case (most likely the person who pulls branch into trunk
> would say "keep ours" for the path which would mean running "git
> cat-file :2:$path >$path", or say "we are not really ready to merge
> yet" and abort the entire merge; somebody on the trunk pulling
> from that branch might say "I want other smaller fixes but this
> total rewrite is not ready yet -- keep ours", or "now we know
> this is total rewrite and the small updates on the trunk does
> not matter -- take theirs and from now on we will improve on the
> work done on the branch").
> 
> So in short, I do not think there is a clear-cut improvement we
> can do to the tools to solve this.

Yes, I agree.

   Luben

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"
  2006-10-09 16:03     ` "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge" Linus Torvalds
  2006-10-09 16:55       ` Junio C Hamano
@ 2006-10-10  6:32       ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2006-10-10  6:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 8 Oct 2006, Junio C Hamano wrote:
>> 
>> 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.
>
> I think the difference _should_ be that we only remove the local file if 
> it was removed _remotely_.
>...
> Agreed?

I reviewed the patch I sent out, the one after the one you
responded to, and I think it (which is the one I have in "pu")
is in line with the reasoning you outlined in your message.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2006-10-10  6:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-09  0:11 "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge" Linus Torvalds
2006-10-09  4:48 ` Junio C Hamano
2006-10-09  5:20   ` Junio C Hamano
2006-10-09  5:48     ` [RFC/PATCH] merge: loosen overcautious "working file will be lost" check Junio C Hamano
2006-10-09 17:20       ` Luben Tuikov
2006-10-09 22:47         ` Junio C Hamano
2006-10-10  0:01           ` Luben Tuikov
2006-10-10  0:19             ` Junio C Hamano
2006-10-10  0:59               ` Luben Tuikov
2006-10-09 16:03     ` "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge" Linus Torvalds
2006-10-09 16:55       ` Junio C Hamano
2006-10-10  6:32       ` Junio C Hamano

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).