{"thread":{"id":"25760","subject":"Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","startedAt":"2010-11-16T10:26:19Z","lastAt":"2010-11-17T09:33:56Z","messageCount":8,"participants":["demerphq","Nguyen Thai Ngoc Duy","Matthieu Moy","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"155988","messageId":"AANLkTinv7XsXNR2MMbaGAUo2=WgL6SEWkuzwGU_7YVn0@mail.gmail.com","threadId":"25760","inReplyTo":null,"subject":"Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2010-11-16T10:26:19Z","receivedAt":"2010-11-16T10:26:19Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"The error in the subject line is generated if one has a git repo\nchecked out to a commit that adds a new file and one does something\nlike:\n\ngit reset HEAD^\n\nand then a merge operation that involves going forward onto or past HEAD.\n\nWhy is this error generated when the file is *exactly* the same as the\nfile that would overwrite it?\n\nObviously it makes sense to throw this error when data would be lost,\nbut when they are identical what is the point?\n\nIn my experience when this happens they are almost always identical,\nand that this is one of the most common sources of frustration and in\nmy shop a real source of confusion for my colleagues.\n\nI think fixing it to detect they are identical and ignore them when\nthey are would really improve things for the uninitiated.\n\nThe following demonstrates the problem:\n\n$ mkdir exp\n$ cd exp\n$ git init\nInitialized empty Git repository in /home/demerphq/exp/.git/\n$ echo woohoo > t.txt\n$ git add t.txt\n$ git commit -m'add t.txt'\n[master (root-commit) dbe3cec] add t.txt\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 t.txt\n$ echo boohoo > b.txt\n$ git add b.txt\n$ git commit -m'add b.txt'\n[master 15a72ea] add b.txt\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 b.txt\n$ git checkout -b other\nSwitched to a new branch 'other'\n$ git reset HEAD^\n$ git merge master\nUpdating dbe3cec..a61413d\nerror: Untracked working tree file 'b.txt' would be overwritten by merge.\n\nAre there any reasons why this cant be changed?\n\nCheers,\nyves\n\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"155989","messageId":"AANLkTimAgDDkPowrfF2QNDdLaxHEyOPVgFy=5rafPWoj@mail.gmail.com","threadId":"25760","inReplyTo":"AANLkTinv7XsXNR2MMbaGAUo2=WgL6SEWkuzwGU_7YVn0@mail.gmail.com","subject":"Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2010-11-16T10:38:44Z","receivedAt":"2010-11-16T10:38:44Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 16 November 2010 11:26, demerphq <demerphq@gmail.com> wrote:\n> The error in the subject line is generated if one has a git repo\n> checked out to a commit that adds a new file and one does something\n> like:\n>\n> git reset HEAD^\n>\n> and then a merge operation that involves going forward onto or past HEAD.\n\nIn fact there are more operations where this can happen.  A\nfailed/aborted rebase. A failed/aborted merge.  etc.\n\nAll of them, so far in my experience, resulting in this bogus error\nabout overwriting a file with itself.\n\ncheers,\nYves\n\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"155993","messageId":"20101116112406.GA18259@do","threadId":"25760","inReplyTo":"AANLkTinv7XsXNR2MMbaGAUo2=WgL6SEWkuzwGU_7YVn0@mail.gmail.com","subject":"Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T11:24:06Z","receivedAt":"2010-11-16T11:24:06Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Nov 16, 2010 at 11:26:19AM +0100, demerphq wrote:\n> The error in the subject line is generated if one has a git repo\n> checked out to a commit that adds a new file and one does something\n> like:\n> \n> git reset HEAD^\n> \n> and then a merge operation that involves going forward onto or past HEAD.\n> \n> Why is this error generated when the file is *exactly* the same as the\n> file that would overwrite it?\n> \n> Obviously it makes sense to throw this error when data would be lost,\n> but when they are identical what is the point?\n\nSomething like this may help. Completely untested but could be a\nstarting point. See [1] for a more generic case, where users just want\ngit it ignore updating worktree and go ahead.\n-- \nDuy\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/160568/focus=160725\n\n-- 8< --\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 803445a..f9451fc 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -980,6 +980,12 @@ static int same(struct cache_entry *a, struct cache_entry *b)\n }\n \n \n+static int identical_content(struct cache_entry *ce, struct stat *st)\n+{\n+\tunsigned char sha1[20];\n+\treturn !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);\n+}\n+\n /*\n  * When a CE gets turned into an unmerged entry, we\n  * want it to be up-to-date\n@@ -1006,6 +1012,10 @@ static int verify_uptodate_1(struct cache_entry *ce,\n \t\t */\n \t\tif (S_ISGITLINK(ce->ce_mode))\n \t\t\treturn 0;\n+\n+\t\tif (identical_content(ce, &st))\n+\t\t\treturn 0;\n+\n \t\terrno = 0;\n \t}\n \tif (errno == ENOENT)\n@@ -1195,6 +1205,8 @@ static int verify_absent_1(struct cache_entry *ce,\n \t\t\t\treturn 0;\n \t\t}\n \n+\t\tif (identical_content(ce, &st))\n+\t\t\treturn 0;\n \t\treturn o->gently ? -1 :\n \t\t\tadd_rejected_path(o, error_type, ce->name);\n \t}\n-- 8< --\n"},{"id":"155994","messageId":"AANLkTikeaj6kfq+pv+F0mOUSC7yJKnwyUgy9k8GPTpxg@mail.gmail.com","threadId":"25760","inReplyTo":"20101116112406.GA18259@do","subject":"Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-16T11:34:40Z","receivedAt":"2010-11-16T11:34:40Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Nov 16, 2010 at 6:24 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> +static int identical_content(struct cache_entry *ce, struct stat *st)\n> +{\n> +       unsigned char sha1[20];\n> +       return !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);\n\nEven better, do a file size check here. If it's not equal, there's no\npoint in calling the expensive index_path().\n-- \nDuy\n"},{"id":"156035","messageId":"AANLkTimp1ZvVLnRQa+LssbeO7F=9MrA56aZgFHQnbZCK@mail.gmail.com","threadId":"25760","inReplyTo":"AANLkTikeaj6kfq+pv+F0mOUSC7yJKnwyUgy9k8GPTpxg@mail.gmail.com","subject":"Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2010-11-17T08:37:13Z","receivedAt":"2010-11-17T08:37:13Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 16 November 2010 12:34, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> On Tue, Nov 16, 2010 at 6:24 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n>> +static int identical_content(struct cache_entry *ce, struct stat *st)\n>> +{\n>> +       unsigned char sha1[20];\n>> +       return !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);\n>\n> Even better, do a file size check here. If it's not equal, there's no\n> point in calling the expensive index_path().\n\nI tried your patch, with the size check modification, and it seems to\nwork (meaning it passed all existing tests).\n\nIll try to put together tests and a proper patch soon.\n\nIm hoping that if someone thinks that such a patch would be dismissed\nout of hand that they will let me know....\n\ncheers,\nYves\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"156036","messageId":"vpqtyjgxrqk.fsf@bauges.imag.fr","threadId":"25760","inReplyTo":"AANLkTikeaj6kfq+pv+F0mOUSC7yJKnwyUgy9k8GPTpxg@mail.gmail.com","subject":"Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-11-17T08:46:11Z","receivedAt":"2010-11-17T08:46:11Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> On Tue, Nov 16, 2010 at 6:24 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n>> +static int identical_content(struct cache_entry *ce, struct stat *st)\n>> +{\n>> +       unsigned char sha1[20];\n>> +       return !index_path(sha1, ce->name, st, 0) && !hashcmp(sha1, ce->sha1);\n\nI do like the idea.\n\n> Even better, do a file size check here. If it's not equal, there's no\n> point in calling the expensive index_path().\n\nI think you also need to check mode changes here.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"156037","messageId":"AANLkTin3vbW7LFjJrS8yAyrWxVU=zorawAQAs8F1siSF@mail.gmail.com","threadId":"25760","inReplyTo":"AANLkTimp1ZvVLnRQa+LssbeO7F=9MrA56aZgFHQnbZCK@mail.gmail.com","subject":"Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-11-17T09:22:02Z","receivedAt":"2010-11-17T09:22:02Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Nov 17, 2010 at 09:37, demerphq <demerphq@gmail.com> wrote:\n\n> Im hoping that if someone thinks that such a patch would be dismissed\n> out of hand that they will let me know....\n\nI don't think it should be dismissed, but it's interesting to look\nforward a bit and think how clever we really want to be.\n\nE.g. if I have a tracked file \"tracked\" in my repository and do:\n\n    echo Uncommited >>tracked &&\n    git rebase -i HEAD~3\n\nWe'll error out with:\n\n    tracked: needs update\n    Working tree is dirty\n\nAlthough we could be smarter and scan HEAD~3..HEAD and figure out that\nnone of them changes \"tracked\" and go ahead with the rebase.\n\nBut arguably it would be more user friendly in the long term to lead\nusers in the direction of always having clean working trees before\nthey do operations like these. E.g. that users would turn your\noriginal example into:\n\n    git status &&                      # reports no changes / modified files\n    git tag before-reset-and-merge &&  # for ease of getting back / safe from gc\n    git reset --hard HEAD^ &&          # no untracked files with --hard\n    git merge some-branch              # clean merge\n\nIn which case the user would be accustomed to a workflow where he's\nguaranteed not to lose data, because Git is always tracking it.\n\nBut maybe we should just allow users to be more clever, and try really\nhard to find out if they can be clever without screwing up their\nworking tree.\n"},{"id":"156038","messageId":"AANLkTinOFZ=qbCeeUmN7FcPFJT=SZqXEMsuof98ZW-Wh@mail.gmail.com","threadId":"25760","inReplyTo":"vpqtyjgxrqk.fsf@bauges.imag.fr","subject":"Re: Bogus error: Untracked working tree file '....' would be overwritten by merge. Aborting","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-11-17T09:33:56Z","receivedAt":"2010-11-17T09:33:56Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Nov 17, 2010 at 3:46 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n>> Even better, do a file size check here. If it's not equal, there's no\n>> point in calling the expensive index_path().\n>\n> I think you also need to check mode changes here.\n\nHmm.. ce_compare_data() does the same thing as identical_content() and\nwill be called by ie_match_stat() in racy condition. I think we should\nadd another flag to ie_match_stat() and let it call ce_compare_data(),\ninstead of creating a new function identical_content().\n\nie_match_stat() does check mode changes and other stuff so if it\nreturns 0 (with ce_compare_data() called), it's safe.\n-- \nDuy\n"}]}