{"thread":{"id":"16285","subject":"Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","startedAt":"2008-11-12T14:36:54Z","lastAt":"2008-11-12T19:52:35Z","messageCount":7,"participants":["Bruce Stephens","Michael J Gruber","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"95567","messageId":"80wsf9ovsp.fsf@tiny.isode.net","threadId":"16285","inReplyTo":null,"subject":"Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","fromName":"Bruce Stephens","fromEmail":"bruce.stephens@isode.com","sentAt":"2008-11-12T14:36:54Z","receivedAt":"2008-11-12T14:36:54Z","isPatch":false,"sender":{"key":"bruce.stephens@isode.com","avatar":null},"body":"The following works fine with 1.6.0.2 and before, but not 1.6.0.3 or\nlater:\n\n\tgit clone -n git git-test\n        cd git-test\n        git checkout -b work v1.6.0.2\n\nWhen it breaks, the error is:\n\n\terror: Entry '.gitignore' would be overwritten by merge. Cannot merge.\n\nI'm guessing it's a bug rather than a deliberate change?\n"},{"id":"95590","messageId":"491B131D.2050501@drmicha.warpmail.net","threadId":"16285","inReplyTo":"80wsf9ovsp.fsf@tiny.isode.net","subject":"Re: Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2008-11-12T17:32:13Z","receivedAt":"2008-11-12T17:32:13Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Bruce Stephens venit, vidit, dixit 12.11.2008 15:36:\n> The following works fine with 1.6.0.2 and before, but not 1.6.0.3 or\n> later:\n> \n> \tgit clone -n git git-test\n>         cd git-test\n>         git checkout -b work v1.6.0.2\n> \n> When it breaks, the error is:\n> \n> \terror: Entry '.gitignore' would be overwritten by merge. Cannot merge.\n> \n> I'm guessing it's a bug rather than a deliberate change?\n\nBisecting gives:\n\n\n5521883490e85f4d973141972cf16f89a79f1979 is first bad commit\ncommit 5521883490e85f4d973141972cf16f89a79f1979\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Sun Sep 7 19:49:25 2008 -0700\n\n    checkout: do not lose staged removal\n\nCCing Junio...\n\nMichael\n"},{"id":"95592","messageId":"804p2cq1vc.fsf@tiny.isode.net","threadId":"16285","inReplyTo":"80wsf9ovsp.fsf@tiny.isode.net","subject":"Re: Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","fromName":"Bruce Stephens","fromEmail":"bruce.stephens@isode.com","sentAt":"2008-11-12T17:40:23Z","receivedAt":"2008-11-12T17:40:23Z","isPatch":false,"sender":{"key":"bruce.stephens@isode.com","avatar":null},"body":"Bruce Stephens <bruce.stephens@isode.com> writes:\n\n> The following works fine with 1.6.0.2 and before, but not 1.6.0.3 or\n> later:\n>\n> \tgit clone -n git git-test\n>         cd git-test\n>         git checkout -b work v1.6.0.2\n>\n> When it breaks, the error is:\n>\n> \terror: Entry '.gitignore' would be overwritten by merge. Cannot merge.\n>\n> I'm guessing it's a bug rather than a deliberate change?\n\nAccording to \"git bisect\", the commit that caused this is the\nfollowing one.  Perhaps \"git clone -n\" doesn't start out with the\nindex empty in the relevant sense?\n\ncommit 5521883490e85f4d973141972cf16f89a79f1979\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Sun Sep 7 19:49:25 2008 -0700\n\n    checkout: do not lose staged removal\n    \n    The logic to checkout a different commit implements the safety to never\n    lose user's local changes.  For example, switching from a commit to\n    another commit, when you have changed a path that is different between\n    them, need to merge your changes to the version from the switched-to\n    commit, which you may not necessarily be able to resolve easily.  By\n    default, \"git checkout\" refused to switch branches, to give you a chance\n    to stash your local changes (or use \"-m\" to merge, accepting the risks of\n    getting conflicts).\n    \n    This safety, however, had one deliberate hole since early June 2005.  When\n    your local change was to remove a path (and optionally to stage that\n    removal), the command checked out the path from the switched-to commit\n    nevertheless.\n    \n    This was to allow an initial checkout to happen smoothly (e.g. an initial\n    checkout is done by starting with an empty index and switching from the\n    commit at the HEAD to the same commit).  We can tighten the rule slightly\n    to allow this special case to pass, without losing sight of removal\n    explicitly done by the user, by noticing if the index is truly empty when\n    the operation begins.\n    \n    For historical background, see:\n    \n        http://thread.gmane.org/gmane.comp.version-control.git/4641/focus=4646\n    \n    This case is marked as *0* in the message, which both Linus and I said \"it\n    feels somewhat wrong but otherwise we cannot start from an empty index\".\n    \n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"95594","messageId":"80r65gon3m.fsf@tiny.isode.net","threadId":"16285","inReplyTo":"491B131D.2050501@drmicha.warpmail.net","subject":"Re: Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","fromName":"Bruce Stephens","fromEmail":"bruce.stephens@isode.com","sentAt":"2008-11-12T17:44:45Z","receivedAt":"2008-11-12T17:44:45Z","isPatch":false,"sender":{"key":"bruce.stephens@isode.com","avatar":null},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n[...]\n\n> Bisecting gives:\n>\n>\n> 5521883490e85f4d973141972cf16f89a79f1979 is first bad commit\n> commit 5521883490e85f4d973141972cf16f89a79f1979\n> Author: Junio C Hamano <gitster@pobox.com>\n> Date:   Sun Sep 7 19:49:25 2008 -0700\n>\n>     checkout: do not lose staged removal\n\nI got the same, which is reassuring.\n\nLooks like a deliberate change with (what seems to me to be) an\nunfortunate interaction with \"git clone -n\"\n"},{"id":"95616","messageId":"7vprl0oiw6.fsf@gitster.siamese.dyndns.org","threadId":"16285","inReplyTo":"80r65gon3m.fsf@tiny.isode.net","subject":"Re: Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-12T19:15:37Z","receivedAt":"2008-11-12T19:15:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bruce Stephens <bruce.stephens@isode.com> writes:\n\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n>\n> [...]\n>\n>> Bisecting gives:\n>>\n>>\n>> 5521883490e85f4d973141972cf16f89a79f1979 is first bad commit\n>> commit 5521883490e85f4d973141972cf16f89a79f1979\n>> Author: Junio C Hamano <gitster@pobox.com>\n>> Date:   Sun Sep 7 19:49:25 2008 -0700\n>>\n>>     checkout: do not lose staged removal\n>\n> I got the same, which is reassuring.\n>\n> Looks like a deliberate change with (what seems to me to be) an\n> unfortunate interaction with \"git clone -n\"\n\nYeah, it was meant to allow:\n\n\tgit clone -n $there $here\n        cd $here\n        git checkout\n\nand was not taking care of the case to switch branches when the initial\ncheckout is made.\n\nPerhaps this would help.\n\n builtin-checkout.c |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git c/builtin-checkout.c w/builtin-checkout.c\nindex 05eee4e..d2265df 100644\n--- c/builtin-checkout.c\n+++ w/builtin-checkout.c\n@@ -269,8 +269,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\t}\n \n \t\t/* 2-way merge to the new branch */\n-\t\ttopts.initial_checkout = (!active_nr &&\n-\t\t\t\t\t  (old->commit == new->commit));\n+\t\ttopts.initial_checkout = !active_nr;\n \t\ttopts.update = 1;\n \t\ttopts.merge = 1;\n \t\ttopts.gently = opts->merge;\n\n        \n"},{"id":"95621","messageId":"80od0ksp8w.fsf@tiny.isode.net","threadId":"16285","inReplyTo":"7vprl0oiw6.fsf@gitster.siamese.dyndns.org","subject":"Re: Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","fromName":"Bruce Stephens","fromEmail":"bruce.stephens@isode.com","sentAt":"2008-11-12T19:44:47Z","receivedAt":"2008-11-12T19:44:47Z","isPatch":false,"sender":{"key":"bruce.stephens@isode.com","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n[...]\n\n> Yeah, it was meant to allow:\n>\n> \tgit clone -n $there $here\n>         cd $here\n>         git checkout\n>\n> and was not taking care of the case to switch branches when the initial\n> checkout is made.\n\nThat specific sequence does work.  I guess that's why I hadn't noticed\nthe issue for so long (I guess git's test suite has some tests using\n\"clone -n\", and perhaps they're of that form).\n\n> Perhaps this would help.\n\nWorks for me.\n\n[...]\n"},{"id":"95623","messageId":"7vfxlwoh6k.fsf_-_@gitster.siamese.dyndns.org","threadId":"16285","inReplyTo":"7vprl0oiw6.fsf@gitster.siamese.dyndns.org","subject":"Re* Change in \"git checkout\" behaviour between 1.6.0.2 and 1.6.0.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-12T19:52:35Z","receivedAt":"2008-11-12T19:52:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> Looks like a deliberate change with (what seems to me to be) an\n>> unfortunate interaction with \"git clone -n\"\n>\n> Yeah, it was meant to allow:\n>\n> \tgit clone -n $there $here\n>         cd $here\n>         git checkout\n>\n> and was not taking care of the case to switch branches when the initial\n> checkout is made.\n>\n> Perhaps this would help.\n> ...\n\nHere is a more involved but hopefully more maintainable fix.\n\n-- >8 --\nSubject: checkout: Fix \"initial checkout\" detection\n\nEarlier commit 5521883 (checkout: do not lose staged removal, 2008-09-07)\ntightened the rule to prevent switching branches from losing local\nchanges, so that staged removal of paths can be protected, while\nattempting to keep a loophole to still allow a special case of switching\nout of an un-checked-out state.\n\nHowever, the loophole was made a bit too tight, and did not allow\nswitching from one branch (in an un-checked-out state) to check out\nanother branch.\n\nThe change to builtin-checkout.c in this commit loosens it to allow this,\nby not insisting the original commit and the new commit to be the same.\n\nIt also introduces a new function, is_index_unborn (and an associated\nmacro, is_cache_unborn), to check if the repository is truly in an\nun-checked-out state more reliably, by making sure that $GIT_INDEX_FILE\ndid not exist when populating the in-core index structure.  A few places\nthe earlier commit 5521883 added the check for the initial checkout\ncondition are updated to use this function.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-checkout.c  |    3 +--\n builtin-read-tree.c |    2 +-\n cache.h             |    2 ++\n read-cache.c        |    5 +++++\n 4 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git c/builtin-checkout.c w/builtin-checkout.c\nindex 05eee4e..25845cd 100644\n--- c/builtin-checkout.c\n+++ w/builtin-checkout.c\n@@ -269,8 +269,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\t}\n \n \t\t/* 2-way merge to the new branch */\n-\t\ttopts.initial_checkout = (!active_nr &&\n-\t\t\t\t\t  (old->commit == new->commit));\n+\t\ttopts.initial_checkout = is_cache_unborn();\n \t\ttopts.update = 1;\n \t\ttopts.merge = 1;\n \t\ttopts.gently = opts->merge;\ndiff --git c/builtin-read-tree.c w/builtin-read-tree.c\nindex 0706c95..38fef34 100644\n--- c/builtin-read-tree.c\n+++ w/builtin-read-tree.c\n@@ -206,7 +206,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t\tbreak;\n \t\tcase 2:\n \t\t\topts.fn = twoway_merge;\n-\t\t\topts.initial_checkout = !active_nr;\n+\t\t\topts.initial_checkout = is_cache_unborn();\n \t\t\tbreak;\n \t\tcase 3:\n \t\tdefault:\ndiff --git c/cache.h w/cache.h\nindex a1e4982..3960931 100644\n--- c/cache.h\n+++ w/cache.h\n@@ -255,6 +255,7 @@ static inline void remove_name_hash(struct cache_entry *ce)\n \n #define read_cache() read_index(&the_index)\n #define read_cache_from(path) read_index_from(&the_index, (path))\n+#define is_cache_unborn() is_index_unborn(&the_index)\n #define read_cache_unmerged() read_index_unmerged(&the_index)\n #define write_cache(newfd, cache, entries) write_index(&the_index, (newfd))\n #define discard_cache() discard_index(&the_index)\n@@ -360,6 +361,7 @@ extern int init_db(const char *template_dir, unsigned int flags);\n /* Initialize and use the cache information */\n extern int read_index(struct index_state *);\n extern int read_index_from(struct index_state *, const char *path);\n+extern int is_index_unborn(struct index_state *);\n extern int read_index_unmerged(struct index_state *);\n extern int write_index(const struct index_state *, int newfd);\n extern int discard_index(struct index_state *);\ndiff --git c/read-cache.c w/read-cache.c\nindex 967f483..525d138 100644\n--- c/read-cache.c\n+++ w/read-cache.c\n@@ -1239,6 +1239,11 @@ unmap:\n \tdie(\"index file corrupt\");\n }\n \n+int is_index_unborn(struct index_state *istate)\n+{\n+\treturn (!istate->cache_nr && !istate->alloc && !istate->timestamp);\n+}\n+\n int discard_index(struct index_state *istate)\n {\n \tistate->cache_nr = 0;\n"}]}