{"thread":{"id":"12614","subject":"\"git checkout\" branch switching safety broken in 'next'","startedAt":"2008-03-10T08:12:37Z","lastAt":"2008-03-11T10:01:25Z","messageCount":8,"participants":["Junio C Hamano","Jakub Narebski","Jay Soffian","Linus Torvalds","Maxim Fridental","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"71560","messageId":"7vmyp7j8ui.fsf@gitster.siamese.dyndns.org","threadId":"12614","inReplyTo":null,"subject":"\"git checkout\" branch switching safety broken in 'next'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-10T08:12:37Z","receivedAt":"2008-03-10T08:12:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus, please be careful when switching branches if you are using your\nunpack_trees() patch (in 'next') for production.  There is a breakage that\nmakes switching branches with \"git checkout\" lose your work-in-progress.\n\nTraditionally, when you have a change in the work tree, and switch to\nanother branch that has different contents in the modified path, we\nerrored out by saying:\n\n    error: Entry 'foo' not uptodate. Cannot merge.\n\nThe updated unpack_trees() notices the condition, issues this message\ncorrectly and tries to propagate the error up, but loses the error at a\nfew places in the call chain.  The end result is that the caller does not\nnotice the two-way merge failure, and after that all h*ll breaks loose.\n\nI think this should fix it.\n\n-- >8 --\ntraverse_trees_recursive(): propagate merge errors up\n\nThere were few places where merge errors detected deeper in the call chain\nwere ignored and not propagated up the callchain to the caller.\n\nMost notably, this caused \"git checkout\" to switch branches to internally\nnotice that a path modified in a work tree are different between the HEAD\nversion and the commit being switched to, but ignore that and switch\nbranches nevertheless, resulting in an incorrect two-way merge and loss of\nthe change in the work tree.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n unpack-trees.c |    7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex da68557..5a0f038 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -144,8 +144,7 @@ int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long df_conf\n \t\t\tsha1 = names[i].sha1;\n \t\tfill_tree_descriptor(t+i, sha1);\n \t}\n-\ttraverse_trees(n, t, &newinfo);\n-\treturn 0;\n+\treturn traverse_trees(n, t, &newinfo);\n }\n \n /*\n@@ -306,7 +305,9 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \t\t\tif (src[0])\n \t\t\t\tconflicts |= 1;\n \t\t}\n-\t\ttraverse_trees_recursive(n, dirmask, conflicts, names, info);\n+\t\tif (traverse_trees_recursive(n, dirmask, conflicts,\n+\t\t\t\t\t     names, info) < 0)\n+\t\t\treturn -1;\n \t\treturn mask;\n \t}\n \n"},{"id":"71566","messageId":"fr2v0s$cdb$1@ger.gmane.org","threadId":"12614","inReplyTo":"7vmyp7j8ui.fsf@gitster.siamese.dyndns.org","subject":"Re: \"git checkout\" branch switching safety broken in 'next'","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-10T09:29:00Z","receivedAt":"2008-03-10T09:29:00Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> Traditionally, when you have a change in the work tree, and switch to\n> another branch that has different contents in the modified path, we\n> errored out by saying:\n> \n>     error: Entry 'foo' not uptodate. Cannot merge.\n\nBy the way, could this error message be made less cryptic? It is there\nsince the very beginning, and was not changed during making error\nmessages more user friendly...\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"71583","messageId":"76718490803100656k6607e00ar1cce14aedffb3b27@mail.gmail.com","threadId":"12614","inReplyTo":"fr2v0s$cdb$1@ger.gmane.org","subject":"Re: \"git checkout\" branch switching safety broken in 'next'","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-03-10T13:56:40Z","receivedAt":"2008-03-10T13:56:40Z","isPatch":false,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"2008/3/10 Jakub Narebski <jnareb@gmail.com>:\n> Junio C Hamano wrote:\n> >     error: Entry 'foo' not uptodate. Cannot merge.\n>\n>  By the way, could this error message be made less cryptic? It is there\n>  since the very beginning, and was not changed during making error\n>  messages more user friendly...\n\n+1. Most concise is probably:\n\n * \"error: 'foo' has uncommitted changes.\"\n\nMore helpful messages that I can think of are probably too verbose. e.g.:\n\n * \"error: 'foo' differs in target branch but has uncommitted changes.\"\n\n * \"error: 'foo' has uncommitted changes. Commit, use -f to lose\n    changes, or -m to merge changes in target branch.\"\n\nj.\n"},{"id":"71587","messageId":"alpine.LFD.1.00.0803100735530.5896@woody.linux-foundation.org","threadId":"12614","inReplyTo":"7vmyp7j8ui.fsf@gitster.siamese.dyndns.org","subject":"Re: \"git checkout\" branch switching safety broken in 'next'","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-10T14:38:25Z","receivedAt":"2008-03-10T14:38:25Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 10 Mar 2008, Junio C Hamano wrote:\n>\n> Linus, please be careful when switching branches if you are using your\n> unpack_trees() patch (in 'next') for production.  There is a breakage that\n> makes switching branches with \"git checkout\" lose your work-in-progress.\n\nOuch. I've been using those patches \"in production\" since posting them, \nbut since I don't generally use branches on the kernel (it happens, but \nit's rare), I wouldn't have noticed.\n\nAnd in git, it probably wouldn't have triggered for me, since it looks \nlike it would only trigger for changes in subdirectories, not in the top \nlevel (which is probably also why it passed all the tests!).\n\nMy bad. Your patch looks obviously correct.\n\n\t\t\tLinus\n"},{"id":"71642","messageId":"7vk5kagexu.fsf_-_@gitster.siamese.dyndns.org","threadId":"12614","inReplyTo":"alpine.LFD.1.00.0803100735530.5896@woody.linux-foundation.org","subject":"\"git diff-index\" broken in 'next'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-11T02:41:33Z","receivedAt":"2008-03-11T02:41:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you are in the middle of conflicted merge, it is useful to view \"git\ndiff\" that compares the stage #2 and your work tree (which has conflict\nmarkers and both sides of the changes).  This still works.\n\nHowever, with the unpack_trees() update, another form of useful sanity\ncheck seems to be totally broken:\n\n      $ git diff HEAD -- $conflicted_file\n\nThis used to show the diff between the HEAD and the work tree, but not\nanymore.  It shows a diff as if the whole file is gone, and that is\nbecause diff-lib.c::oneway_diff() gets idx==NULL from the caller for\nunmerged entries. It never reaches show_modified() codepath.\n\nI think it is somewhere around ll. 280 in unpack_callback() that causes\nthis, but I don't have time to dig this further for now.\n"},{"id":"71646","messageId":"alpine.LFD.1.00.0803101958350.5896@woody.linux-foundation.org","threadId":"12614","inReplyTo":"7vk5kagexu.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: \"git diff-index\" broken in 'next'","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-11T03:20:12Z","receivedAt":"2008-03-11T03:20:12Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 10 Mar 2008, Junio C Hamano wrote:\n> \n> I think it is somewhere around ll. 280 in unpack_callback() that causes\n> this, but I don't have time to dig this further for now.\n\nYes indeed. I was confused. I even knew I was confused about the whole \ndifference between \"skip_unmerged\" and not skipping unmerged entries in \nthe tree unpacking, but nothing seemed to care, so I was lazy.\n\nNow that I remember what the heck that oneway_diff thing wanted, it all \nmakes sense to me again.\n\nThis should fix it.\n\nThat oneway_diff() thing is magic, and used to have a \"count_skip()\" \nfunction to skip over unmerged entries. In my confusion, I had instead \nskipped them in the generic unpack-trees.c code, and thus removed the \nskipping from oneway_diff().\n\nThat wasn't correct - we should pass the unmerged entries down and let the callback handle it.\n\n[ Side note: nobody else than the diff code should ever have them in the \n  source index anyway - that would be a bug, since we cannot merge with a \n  source index that has unmerged entries ]\n\nSorry about that. Holler if you notice anythign else.\n\n\t\tLinus\n\n---\n diff-lib.c     |   18 ++++++++++++++++++\n unpack-trees.c |    2 --\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 9520773..52dbac3 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -641,6 +641,21 @@ static void do_oneway_diff(struct unpack_trees_options *o,\n \tshow_modified(revs, tree, idx, 1, cached, match_missing);\n }\n \n+static inline void skip_same_name(struct cache_entry *ce, struct unpack_trees_options *o)\n+{\n+\tint len = ce_namelen(ce);\n+\tconst struct index_state *index = o->src_index;\n+\n+\twhile (o->pos < index->cache_nr) {\n+\t\tstruct cache_entry *next = index->cache[o->pos];\n+\t\tif (len != ce_namelen(next))\n+\t\t\tbreak;\n+\t\tif (memcmp(ce->name, next->name, len))\n+\t\t\tbreak;\n+\t\to->pos++;\n+\t}\n+}\n+\n /*\n  * The unpack_trees() interface is designed for merging, so\n  * the different source entries are designed primarily for\n@@ -662,6 +677,9 @@ static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)\n \tstruct cache_entry *tree = src[1];\n \tstruct rev_info *revs = o->unpack_data;\n \n+\tif (idx && ce_stage(idx))\n+\t\tskip_same_name(idx, o);\n+\n \t/*\n \t * Unpack-trees generates a DF/conflict entry if\n \t * there was a directory in the index and a tree\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex da68557..7a30361 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -116,7 +116,6 @@ static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_option\n \t\t\tadd_entry(o, ce, 0, 0);\n \t\t\treturn 0;\n \t\t}\n-\t\treturn 0;\n \t}\n \treturn call_unpack_fn(src, o);\n }\n@@ -287,7 +286,6 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \t\t\t\t\t\tadd_entry(o, ce, 0, 0);\n \t\t\t\t\t\treturn mask;\n \t\t\t\t\t}\n-\t\t\t\t\tcontinue;\n \t\t\t\t}\n \t\t\t\tsrc[0] = ce;\n \t\t\t}\n"},{"id":"71672","messageId":"23D34F9EB30CC241B8587BC90CA4D3793FECDD268D@silik.axinom.de","threadId":"12614","inReplyTo":"76718490803100656k6607e00ar1cce14aedffb3b27@mail.gmail.com","subject":"setting git-gui locale","fromName":"Maxim Fridental","fromEmail":"fridental@axinom.de","sentAt":"2008-03-11T08:57:35Z","receivedAt":"2008-03-11T08:57:35Z","isPatch":false,"sender":{"key":"fridental@axinom.de","avatar":null},"body":"Hi!\n\nCould someone point me please to a doc describing how to override language setting for git-gui tool (precompiled for Windows, downloaded from http://code.google.com/p/msysgit/downloads/list)? It is choosing German language for its menu items on any system I install it; and I personally find the German translations unusable.\n\nRegards\nMaxim Fridental\n"},{"id":"71684","messageId":"alpine.LSU.1.00.0803111100490.3873@racer.site","threadId":"12614","inReplyTo":"23D34F9EB30CC241B8587BC90CA4D3793FECDD268D@silik.axinom.de","subject":"Re: setting git-gui locale","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-11T10:01:25Z","receivedAt":"2008-03-11T10:01:25Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Mar 2008, Maxim Fridental wrote:\n\n> Could someone point me please to a doc describing how to override \n> language setting for git-gui tool (precompiled for Windows, downloaded \n> from http://code.google.com/p/msysgit/downloads/list)?\n\nTry setting LANG=C or LANG=en before calling git-gui.\n\n> It is choosing German language for its menu items on any system I \n> install it; and I personally find the German translations unusable.\n\nPatches welcome,\nDscho\n"}]}