Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[demerphq]

The error in the subject line is generated if one has a git repo
checked out to a commit that adds a new file and one does something
like:

git reset HEAD^

and then a merge operation that involves going forward onto or past HEAD.

Why is this error generated when the file is *exactly* the same as the
file that would overwrite it?

Obviously it makes sense to throw this error when data would be lost,
but when they are identical what is the point?

In my experience when this happens they are almost always identical,
and that this is one of the most common sources of frustration and in
my shop a real source of confusion for my colleagues.

I think fixing it to detect they are identical and ignore them when
they are would really improve things for the uninitiated.

The following demonstrates the problem:

$ mkdir exp
$ cd exp
$ git init
Initialized empty Git repository in /home/demerphq/exp/.git/
$ echo woohoo > t.txt
$ git add t.txt
$ git commit -m'add t.txt'
[master (root-commit) dbe3cec] add t.txt
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 t.txt
$ echo boohoo > b.txt
$ git add b.txt
$ git commit -m'add b.txt'
[master 15a72ea] add b.txt
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 b.txt
$ git checkout -b other
Switched to a new branch 'other'
$ git reset HEAD^
$ git merge master
Updating dbe3cec..a61413d
error: Untracked working tree file 'b.txt' would be overwritten by merge.

Are there any reasons why this cant be changed?

Cheers,
yves



-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[demerphq]

On 16 November 2010 11:26, demerphq <demerphq@gmail.com> wrote:
> The error in the subject line is generated if one has a git repo
> checked out to a commit that adds a new file and one does something
> like:
>
> git reset HEAD^
>
> and then a merge operation that involves going forward onto or past HEAD.

In fact there are more operations where this can happen.  A
failed/aborted rebase. A failed/aborted merge.  etc.

All of them, so far in my experience, resulting in this bogus error
about overwriting a file with itself.

cheers,
Yves



-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[Nguyen Thai Ngoc Duy]

On Tue, Nov 16, 2010 at 11:26:19AM +0100, demerphq wrote:
> The error in the subject line is generated if one has a git repo
> checked out to a commit that adds a new file and one does something
> like:
> 
> git reset HEAD^
> 
> and then a merge operation that involves going forward onto or past HEAD.
> 
> Why is this error generated when the file is *exactly* the same as the
> file that would overwrite it?
> 
> Obviously it makes sense to throw this error when data would be lost,
> but when they are identical what is the point?

Something like this may help. Completely untested but could be a
starting point. See [1] for a more generic case, where users just want
git it ignore updating worktree and go ahead.
-- 
Duy

[1] http://thread.gmane.org/gmane.comp.version-control.git/160568/focus=160725

-- 8< --
diff --git a/unpack-trees.c b/unpack-trees.c
index 803445a..f9451fc 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -980,6 +980,12 @@ static int same(struct cache_entry *a, struct cache_entry *b)
 }
 
 
+static int identical_content(struct cache_entry *ce, struct stat *st)
+{
+	unsigned char sha1[20];
+	return !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);
+}
+
 /*
  * When a CE gets turned into an unmerged entry, we
  * want it to be up-to-date
@@ -1006,6 +1012,10 @@ static int verify_uptodate_1(struct cache_entry *ce,
 		 */
 		if (S_ISGITLINK(ce->ce_mode))
 			return 0;
+
+		if (identical_content(ce, &st))
+			return 0;
+
 		errno = 0;
 	}
 	if (errno == ENOENT)
@@ -1195,6 +1205,8 @@ static int verify_absent_1(struct cache_entry *ce,
 				return 0;
 		}
 
+		if (identical_content(ce, &st))
+			return 0;
 		return o->gently ? -1 :
 			add_rejected_path(o, error_type, ce->name);
 	}
-- 8< --

Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[Nguyen Thai Ngoc Duy]

On Tue, Nov 16, 2010 at 6:24 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> +static int identical_content(struct cache_entry *ce, struct stat *st)
> +{
> +       unsigned char sha1[20];
> +       return !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);

Even better, do a file size check here. If it's not equal, there's no
point in calling the expensive index_path().
-- 
Duy

Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[demerphq]

On 16 November 2010 12:34, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Tue, Nov 16, 2010 at 6:24 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>> +static int identical_content(struct cache_entry *ce, struct stat *st)
>> +{
>> +       unsigned char sha1[20];
>> +       return !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);
>
> Even better, do a file size check here. If it's not equal, there's no
> point in calling the expensive index_path().

I tried your patch, with the size check modification, and it seems to
work (meaning it passed all existing tests).

Ill try to put together tests and a proper patch soon.

Im hoping that if someone thinks that such a patch would be dismissed
out of hand that they will let me know....

cheers,
Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[Matthieu Moy]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Tue, Nov 16, 2010 at 6:24 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>> +static int identical_content(struct cache_entry *ce, struct stat *st)
>> +{
>> +       unsigned char sha1[20];
>> +       return !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);

I do like the idea.

> Even better, do a file size check here. If it's not equal, there's no
> point in calling the expensive index_path().

I think you also need to check mode changes here.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[Ævar Arnfjörð Bjarmason]

On Wed, Nov 17, 2010 at 09:37, demerphq <demerphq@gmail.com> wrote:

> Im hoping that if someone thinks that such a patch would be dismissed
> out of hand that they will let me know....

I don't think it should be dismissed, but it's interesting to look
forward a bit and think how clever we really want to be.

E.g. if I have a tracked file "tracked" in my repository and do:

    echo Uncommited >>tracked &&
    git rebase -i HEAD~3

We'll error out with:

    tracked: needs update
    Working tree is dirty

Although we could be smarter and scan HEAD~3..HEAD and figure out that
none of them changes "tracked" and go ahead with the rebase.

But arguably it would be more user friendly in the long term to lead
users in the direction of always having clean working trees before
they do operations like these. E.g. that users would turn your
original example into:

    git status &&                      # reports no changes / modified files
    git tag before-reset-and-merge &&  # for ease of getting back / safe from gc
    git reset --hard HEAD^ &&          # no untracked files with --hard
    git merge some-branch              # clean merge

In which case the user would be accustomed to a workflow where he's
guaranteed not to lose data, because Git is always tracking it.

But maybe we should just allow users to be more clever, and try really
hard to find out if they can be clever without screwing up their
working tree.

Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting

[Nguyen Thai Ngoc Duy]

On Wed, Nov 17, 2010 at 3:46 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>> Even better, do a file size check here. If it's not equal, there's no
>> point in calling the expensive index_path().
>
> I think you also need to check mode changes here.

Hmm.. ce_compare_data() does the same thing as identical_content() and
will be called by ie_match_stat() in racy condition. I think we should
add another flag to ie_match_stat() and let it call ce_compare_data(),
instead of creating a new function identical_content().

ie_match_stat() does check mode changes and other stuff so if it
returns 0 (with ce_compare_data() called), it's safe.
-- 
Duy