{"thread":{"id":"26861","subject":"Why git silently replaces untracked files?","startedAt":"2011-03-25T14:52:34Z","lastAt":"2011-03-25T18:13:31Z","messageCount":8,"participants":["igor.mikushkin","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"164298","messageId":"1301064754576-6207950.post@n2.nabble.com","threadId":"26861","inReplyTo":null,"subject":"Why git silently replaces untracked files?","fromName":"igor.mikushkin","fromEmail":"igor.mikushkin@gmail.com","sentAt":"2011-03-25T14:52:34Z","receivedAt":"2011-03-25T14:52:34Z","isPatch":false,"sender":{"key":"igor.mikushkin@gmail.com","avatar":null},"body":"Why git silently replaces untracked files?\n\n# mkdir test.git\n# mkdir 1\n# mkdir 2\n# echo 1 > 1/test\n# echo 2 > 2/test\n# cd test.git\n# git init --bare\n# cd ..\n# git clone test.git\n# cp -r test/.git 1\n# cp -r test/.git 2\n# cd 1\n# git add test\n# git commit -am 1\n# git push origin master\n# cd ../2\n# git pull\n# cat test\n1\n\nIn my opinion it is wrong behavior.\nI've just lost important file due to it.\n\nShould not \"git pull\" fail here?\nAnyhow it looks more correct then silent replacing.\n\nThanks,\nIgor\n\n\n\n\n\n--\nView this message in context: http://git.661346.n2.nabble.com/Why-git-silently-replaces-untracked-files-tp6207950p6207950.html\nSent from the git mailing list archive at Nabble.com.\n"},{"id":"164308","messageId":"20110325165811.GB25851@sigill.intra.peff.net","threadId":"26861","inReplyTo":"1301064754576-6207950.post@n2.nabble.com","subject":"Re: Why git silently replaces untracked files?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-25T16:58:11Z","receivedAt":"2011-03-25T16:58:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 25, 2011 at 07:52:34AM -0700, igor.mikushkin wrote:\n\n> Why git silently replaces untracked files?\n> \n> # mkdir test.git\n> # mkdir 1\n> # mkdir 2\n> # echo 1 > 1/test\n> # echo 2 > 2/test\n> # cd test.git\n> # git init --bare\n> # cd ..\n> # git clone test.git\n> # cp -r test/.git 1\n> # cp -r test/.git 2\n> # cd 1\n> # git add test\n> # git commit -am 1\n> # git push origin master\n> # cd ../2\n> # git pull\n> # cat test\n> 1\n> \n> In my opinion it is wrong behavior.\n> I've just lost important file due to it.\n> \n> Should not \"git pull\" fail here?\n\nIck, definitely it's wrong behavior. The culprit seems to be a special\ncode path for the initial pull which doesn't merge at all, but calls\nread-tree --reset. It should probably be:\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex a3159c3..fb9e2df 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -253,7 +253,7 @@ esac\n if test -z \"$orig_head\"\n then\n \tgit update-ref -m \"initial pull\" HEAD $merge_head \"$curr_head\" &&\n-\tgit read-tree --reset -u HEAD || exit 1\n+\tgit read-tree -m -u HEAD || exit 1\n \texit\n fi\n\nThough I don't know if there are any cases where the --reset would be\nbeneficial over \"-m\". I couldn't think of any.\n\n-Peff\n"},{"id":"164313","messageId":"1301075628970-6208585.post@n2.nabble.com","threadId":"26861","inReplyTo":"20110325165811.GB25851@sigill.intra.peff.net","subject":"Re: Why git silently replaces untracked files?","fromName":"igor.mikushkin","fromEmail":"igor.mikushkin@gmail.com","sentAt":"2011-03-25T17:53:48Z","receivedAt":"2011-03-25T17:53:48Z","isPatch":false,"sender":{"key":"igor.mikushkin@gmail.com","avatar":null},"body":"\nJeff King wrote:\n> \n> On Fri, Mar 25, 2011 at 07:52:34AM -0700, igor.mikushkin wrote:\n> \n> > Why git silently replaces untracked files?\n> > \n> > # mkdir test.git\n> > # mkdir 1\n> > # mkdir 2\n> > # echo 1 > 1/test\n> > # echo 2 > 2/test\n> > # cd test.git\n> > # git init --bare\n> > # cd ..\n> > # git clone test.git\n> > # cp -r test/.git 1\n> > # cp -r test/.git 2\n> > # cd 1\n> > # git add test\n> > # git commit -am 1\n> > # git push origin master\n> > # cd ../2\n> > # git pull\n> > # cat test\n> > 1\n> > \n> > In my opinion it is wrong behavior.\n> > I've just lost important file due to it.\n> > \n> > Should not "git pull" fail here?\n> \n> Ick, definitely it's wrong behavior. The culprit seems to be a special\n> code path for the initial pull which doesn't merge at all, but calls\n> read-tree --reset. It should probably be:\n> \n> diff --git a/git-pull.sh b/git-pull.sh\n> index a3159c3..fb9e2df 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -253,7 +253,7 @@ esac\n> if test -z "$orig_head"\n> then\n> \tgit update-ref -m "initial pull" HEAD $merge_head\n> "$curr_head" &&\n> -\tgit read-tree --reset -u HEAD || exit 1\n> +\tgit read-tree -m -u HEAD || exit 1\n> \texit\n> fi\n> \n> Though I don't know if there are any cases where the --reset would be\n> beneficial over "-m". I couldn't think of any.\n> \n\nThanks Jeff,\nMy opinion is that you are right and merging is best here\n(Though just fail would be probably OK either).\nLove one line fixes.\n\nIgor\n\n--\nView this message in context: http://git.661346.n2.nabble.com/Why-git-silently-replaces-untracked-files-tp6207950p6208585.html\nSent from the git mailing list archive at Nabble.com.\n"},{"id":"164316","messageId":"20110325180644.GA30838@sigill.intra.peff.net","threadId":"26861","inReplyTo":"1301075628970-6208585.post@n2.nabble.com","subject":"Re: Why git silently replaces untracked files?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-25T18:06:44Z","receivedAt":"2011-03-25T18:06:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 25, 2011 at 10:53:48AM -0700, igor.mikushkin wrote:\n\n> > diff --git a/git-pull.sh b/git-pull.sh\n> > index a3159c3..fb9e2df 100755\n> > --- a/git-pull.sh\n> > +++ b/git-pull.sh\n> > @@ -253,7 +253,7 @@ esac\n> > if test -z "$orig_head"\n> > then\n> > \tgit update-ref -m "initial pull" HEAD $merge_head\n> > "$curr_head" &&\n> > -\tgit read-tree --reset -u HEAD || exit 1\n> > +\tgit read-tree -m -u HEAD || exit 1\n> > \texit\n> > fi\n> > \n> > Though I don't know if there are any cases where the --reset would be\n> > beneficial over "-m". I couldn't think of any.\n> > \n> \n> Thanks Jeff,\n> My opinion is that you are right and merging is best here\n> (Though just fail would be probably OK either).\n> Love one line fixes.\n\nIt will fail with \"untracked file 'test' would be overwritten...\"; it's\njust that --reset turns off the safety features of read-tree, which I\ndon't see a point in doing.\n\nWhile looking at this, I also noticed that \"git merge\" behaves in a\nfunny way on this case. So I came up with this patch series:\n\n [1/4]: t7607: mark known breakage in test 11 as fixed\n [2/4]: t7607: clean up stray untracked file\n [3/4]: merge: merge unborn index before setting ref\n [4/4]: pull: do not clobber untracked files on initial pull\n\n-Peff\n"},{"id":"164317","messageId":"20110325180836.GA14898@sigill.intra.peff.net","threadId":"26861","inReplyTo":"20110325180644.GA30838@sigill.intra.peff.net","subject":"[PATCH 1/4] t7607: mark known breakage in test 11 as fixed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-25T18:08:36Z","receivedAt":"2011-03-25T18:08:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This was fixed by 1d718a51 (do not overwrite untracked\nsymlinks, 2011-02-20).\n\nSigned-off-by: Jeff King \n---\n t/t7607-merge-overwrite.sh | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex 5f731a1..691c5fd 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -122,7 +122,7 @@ test_expect_success 'will not overwrite untracked file in leading path' '\n \trm -f sub sub2\n '\n \n-test_expect_failure SYMLINKS 'will not overwrite untracked symlink in leading path' '\n+test_expect_success SYMLINKS 'will not overwrite untracked symlink in leading path' '\n \tgit reset --hard c0 &&\n \trm -rf sub &&\n \tmkdir sub2 &&\n-- \n1.7.4.33.gb8855.dirty\n"},{"id":"164318","messageId":"20110325180903.GB14898@sigill.intra.peff.net","threadId":"26861","inReplyTo":"20110325180644.GA30838@sigill.intra.peff.net","subject":"[PATCH 2/4] t7607: clean up stray untracked file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-25T18:09:03Z","receivedAt":"2011-03-25T18:09:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This file ends up conflicting with the test just after it\n(causing the \"git merge\" to fail). Neither test is to blame\nfor the bug, though. It looks like the merge in 1a9fe45\n(Merge branch 'tr/merge-unborn-clobber', 2011-02-09) is what\ncaused the conflict.\n\nWe didn't notice because the follow-on test is already\nmarked as expect_failure (even though it has since been\nfixed, and now succeeds once the untracked file is moved out\nof the way).\n\nSigned-off-by: Jeff King \n---\n t/t7607-merge-overwrite.sh | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex 691c5fd..c86e298 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -150,6 +150,7 @@ test_expect_success 'will not overwrite untracked file on unborn branch' '\n \tgit rm -fr . &&\n \tgit checkout --orphan new &&\n \tcp important c0.c &&\n+\ttest_when_finished \"rm c0.c\" &&\n \ttest_must_fail git merge c0 2>out &&\n \ttest_cmp out expect &&\n \ttest_path_is_missing .git/MERGE_HEAD &&\n@@ -164,7 +165,7 @@ test_expect_success 'set up unborn branch and content' '\n \techo bar > untracked-file\n '\n \n-test_expect_failure 'will not clobber WT/index when merging into unborn' '\n+test_expect_success 'will not clobber WT/index when merging into unborn' '\n \tgit merge master &&\n \tgrep foo tracked-file &&\n \tgit show :tracked-file >expect &&\n-- \n1.7.4.33.gb8855.dirty\n"},{"id":"164319","messageId":"20110325181038.GC14898@sigill.intra.peff.net","threadId":"26861","inReplyTo":"20110325180644.GA30838@sigill.intra.peff.net","subject":"[PATCH 3/4] merge: merge unborn index before setting ref","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-25T18:10:38Z","receivedAt":"2011-03-25T18:10:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we merge into an unborn branch, there are basically two\nsteps:\n\n 1. Write the sha1 of the new commit into the ref pointed\n to by HEAD.\n\n 2. Update the index with the new content, and check it out\n to the working tree.\n\nWe currently do them in this order. However, (2) is the step\nthat is much more likely to fail, since it can be blocked by\nthings like untracked working tree files. When it does, the\nmerge fails and we are left with an empty index but an\nupdated HEAD.\n\nThis patch switches the order, so that a failure in updating\nthe index leaves us unchanged. Of course, a failure in\nupdating the ref now leaves us with an updated index and\nmis-matched HEAD. That is arguably not much better, but it\nis probably less likely to actually happen.\n\nSigned-off-by: Jeff King \n---\nI noticed this while diagnosing the pull problem fixed in 4/4. As\ndiscused, this is just trading one set of error conditions for another.\nThe \"right\" thing to do is probably to rollback, but of course that can\nfail, too, and it's more effort. I think in practice this is fine.\n\n builtin/merge.c | 2 +-\n t/t7607-merge-overwrite.sh | 4 ++++\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex aa3453c..c8d028c 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1063,9 +1063,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tremote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);\n \t\tif (!remote_head)\n \t\t\tdie(\"%s - not something we can merge\", argv[0]);\n+\t\tread_empty(remote_head->sha1, 0);\n \t\tupdate_ref(\"initial pull\", \"HEAD\", remote_head->sha1, NULL, 0,\n \t\t\t\tDIE_ON_ERR);\n-\t\tread_empty(remote_head->sha1, 0);\n \t\treturn 0;\n \t} else {\n \t\tstruct strbuf merge_names = STRBUF_INIT;\ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex c86e298..b54e840 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -157,6 +157,10 @@ test_expect_success 'will not overwrite untracked file on unborn branch' '\n \ttest_cmp important c0.c\n '\n \n+test_expect_success 'failed merge leaves unborn branch in the womb' '\n+\ttest_must_fail git rev-parse --verify HEAD\n+'\n+\n test_expect_success 'set up unborn branch and content' '\n \tgit symbolic-ref HEAD refs/heads/unborn &&\n \trm -f .git/index &&\n-- \n1.7.4.33.gb8855.dirty\n"},{"id":"164320","messageId":"20110325181331.GD14898@sigill.intra.peff.net","threadId":"26861","inReplyTo":"20110325180644.GA30838@sigill.intra.peff.net","subject":"[PATCH 4/4] pull: do not clobber untracked files on initial pull","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-25T18:13:31Z","receivedAt":"2011-03-25T18:13:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"For a pull into an unborn branch, we do not use \"git merge\"\nat all. Instead, we call read-tree directly. However, we\nused the --reset parameter instead of \"-m\", which turns off\nthe safety features.\n\nSigned-off-by: Jeff King \n---\nThis blames all the way back to d09e79c (git-pull: allow pulling into an\nempty repository, 2006-11-16) by Linus. I couldn't think of a good\nreason to use \"--reset\" instead of \"-m\".\n\n git-pull.sh | 2 +-\n t/t5520-pull.sh | 11 +++++++++++\n 2 files changed, 12 insertions(+), 1 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 63b063a..e31226b 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -274,7 +274,7 @@ esac\n if test -z \"$orig_head\"\n then\n \tgit update-ref -m \"initial pull\" HEAD $merge_head \"$curr_head\" &&\n-\tgit read-tree --reset -u HEAD || exit 1\n+\tgit read-tree -m -u HEAD || exit 1\n \texit\n fi\n \ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 0470a81..0e5eb67 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -46,6 +46,17 @@ test_expect_success 'pulling into void using master:master' '\n \ttest_cmp file cloned-uho/file\n '\n \n+test_expect_success 'pulling into void does not overwrite untracked files' '\n+\tgit init cloned-untracked &&\n+\t(\n+\t\tcd cloned-untracked &&\n+\t\techo untracked >file &&\n+\t\ttest_must_fail git pull .. master &&\n+\t\techo untracked >expect &&\n+\t\ttest_cmp expect file\n+\t)\n+'\n+\n test_expect_success 'test . as a remote' '\n \n \tgit branch copy master &&\n-- \n1.7.4.33.gb8855.dirty\n"}]}