{"thread":{"id":"845","subject":"new read-tree questions.","startedAt":"2005-06-06T08:43:03Z","lastAt":"2005-06-06T19:59:58Z","messageCount":3,"participants":["Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"4640","messageId":"7v64wrvpt4.fsf@assigned-by-dhcp.cox.net","threadId":"845","inReplyTo":null,"subject":"new read-tree questions.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-06T08:43:03Z","receivedAt":"2005-06-06T08:43:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I am trying to understand the new git-read-tree, by using\ngit-resolve-script as an example and also reading read-tree.c; I\nam somewhat confused.\n\n* two-way merge (git-read-tree -m $H $M)\n\nMy understanding is that the current index is allowed to be\nempty, but if it is not, they are kept at stage0, and each of\nthem must match $H and must be up-to-date if the merge involves\nthem.\n\nTo summarize my understanding of what should happen for each\npath:\n\n  stage0 (index)  stage1 ($H)             stage2 ($M)\n------------------------------------------------------------\n  no such path    no such path            no such path\n  * this does not happen (the code would not see such thing).\n  ----------------------------------------------------------\n  no such path    no such path            exists\n  * take $M without complaining.\n  ----------------------------------------------------------\n  no such path    exists                  (does not matter)   *0*\n  * although index does not match $H, we do not reject, so\n    that a merge can happen on an empty cache.  We take $M.\n  ----------------------------------------------------------\n  exists          no such path            no such path\n  * reject, because index does not match $H.\n  ----------------------------------------------------------\n  exists          no such path            exists (index!=$M)\n  * reject, because index does not match $H.\n  ----------------------------------------------------------\n  exists          no such path            exists (index=$M)   *1*\n  * take $M (same as \"keep stage0\").\n  ----------------------------------------------------------\n  exists          exists (index!=$H)      (does not matter)\n  * reject, because index does not match $H.\n  ----------------------------------------------------------\n  exists          exists (index=$H)       no such path        *2*\n  * path is removed.\n  ----------------------------------------------------------\n  exists          exists (index=$H)       exists\n  * take stage2.\n  ----------------------------------------------------------\n\nDoes the above matrix represent the intended behaviour?\n\nI think I understand why we would want *0*, but this asymmetry\nfeels wrong.\n\nI am having trouble with the case *1*.  This would call\ntwoway_check with !seen_stage1 and it says OK, because the\nmerged tree has the same contents as what we started with.  Is\nit to help the case where\" the merged tree changes things the\nsame way we already have as our local change\" case?\n\nAlso I am not sure if the code does the right thing for case\n*2*.  If I am reading the code right, for such a path, we will\nsee stage0 and stage1, and at that point say seen_stage1 = 1 and\nkeep stage0 entry in \"old\".  Then we continue on to the next\npath.  When it happens to be:\n\n    - stage0: we barf because we still have our \"old\".\n\n    - stage1: we barf because our \"old\" does not match the new\n      path; !path_matches(old,ce) triggers.\n\n    - stage2: we barf because our \"old\" does not match the new\n      path; twoway_check(old, seen_stage1, ce) triggers.\n\nOnly when such a \"exists-exists-removed\" were the last entry,\nthe control falls out of the loop and \"unmatched with a new\nentry?\"  check takes care of it without barfing.  The path is\nremoved which is what I understand you want to happen in the\ncase *2*.\n\nMaybe my version of intended behaviour for case *2* is wrong,\nbut then I do not understand why.\n\nI'll do a similar matrix for three-way merge case later and\nprobably ask more questions.\n\n"},{"id":"4645","messageId":"Pine.LNX.4.58.0506060748390.1876@ppc970.osdl.org","threadId":"845","inReplyTo":"7v64wrvpt4.fsf@assigned-by-dhcp.cox.net","subject":"Re: new read-tree questions.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-06T15:04:22Z","receivedAt":"2005-06-06T15:04:22Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 6 Jun 2005, Junio C Hamano wrote:\n>\n> I am trying to understand the new git-read-tree, by using\n> git-resolve-script as an example and also reading read-tree.c; I\n> am somewhat confused.\n\nIt _is_ a confusing piece of code. You don't see it in the revision \nhistory, but I threw away a lot of trials that were crapola.\n\n> * two-way merge (git-read-tree -m $H $M)\n> \n> My understanding is that the current index is allowed to be\n> empty, but if it is not, they are kept at stage0, and each of\n> them must match $H and must be up-to-date if the merge involves\n> them.\n\nThe rule really boils down to: \"no information that only existed in the\nold 'stag0' must be lost\".\n\nSo that means that an empty old stage0 is always fine.\n\nIt also means that if the old stage0 exactly matches the result, then \nthat's also always fine (regardless of the state of the actual checked out \nfile: we've not _changed_ anything).\n\nBut it means that if the old stage0 gets deleted or over-written, then the \ninformation that the old stage0 _used_ to have needs to be encoded either \nin the result _or_ it needs to have been there in the original tree, ie \nthe \"loss\" of information needs to be in the merge itself.\n\nSee? It's ok to drop information if that drop is encoded in the merge\nitself (\"we're merging a delete\"), but it's _not_ ok to drop information\nthat wasn't part of the merge (\"we're merging a delete where the index\nfile described something more than what was in the original tree\")\n\nDoes that clarify what I'm aiming for? It may not clarify the code (which \nmay or may not match my aims ;), but at least it hopefully clarifies what \nthe _point_ of the code was supposed to be.\n\n> To summarize my understanding of what should happen for each\n> path:\n> \n>   stage0 (index)  stage1 ($H)             stage2 ($M)\n> ------------------------------------------------------------\n>   no such path    no such path            no such path\n>   * this does not happen (the code would not see such thing).\n\nYeah, I htink I get this right ;)\n\n>   ----------------------------------------------------------\n>   no such path    no such path            exists\n>   * take $M without complaining.\n\nYes.\n\n>   ----------------------------------------------------------\n>   no such path    exists                  (does not matter)   *0*\n>   * although index does not match $H, we do not reject, so\n>     that a merge can happen on an empty cache.  We take $M.\n\nRight. We didn't lose anything hugely important. \n\nIn theory this could be a delete that we've missed, and we could add a \nflag to actually reject this case. However, it's always easy to \"recover\" \ndeletes (just delete it again ;), so the loss of information is absolutely \nminimal, and it allows starting from an empty index file.\n\nBut this is debatable. We could reject it if you prefer that, and if you \nwant to make it a command line flag I'll definitely apply the patch.\n\n>   ----------------------------------------------------------\n>   exists          no such path            no such path\n>   * reject, because index does not match $H.\n\nExactly. The end result would have dropped the existing data, and the \n_merge_ didn't contain that drop, so we reject that.\n\n>   ----------------------------------------------------------\n>   exists          no such path            exists (index!=$M)\n>   * reject, because index does not match $H.\n\nYes.\n\n>   ----------------------------------------------------------\n>   exists          no such path            exists (index=$M)   *1*\n>   * take $M (same as \"keep stage0\").\n\nThis one is questionable. I think it should be accepted simply because \nthere's no point in not accepting it.\n\n>   ----------------------------------------------------------\n>   exists          exists (index!=$H)      (does not matter)\n>   * reject, because index does not match $H.\n\nYes.\n\n>   ----------------------------------------------------------\n>   exists          exists (index=$H)       no such path        *2*\n>   * path is removed.\n\nYes. This drops the information, but since the merge contained that \n_exact_ drop, that's what we want it to do. This one is not questionable \nat all (except I got it wrong at one stage ;)\n\n>   ----------------------------------------------------------\n>   exists          exists (index=$H)       exists\n>   * take stage2.\n\nYes. \n\n> Does the above matrix represent the intended behaviour?\n> \n> I think I understand why we would want *0*, but this asymmetry\n> feels wrong.\n\nYup, you got it.\n\n> I am having trouble with the case *1*.\n\nThe fact that you're having trouble is good, it means you realize that \nreversing the merge doesn't get you the original state.\n\n> Also I am not sure if the code does the right thing for case\n> *2*.  If I am reading the code right, for such a path, we will\n> see stage0 and stage1, and at that point say seen_stage1 = 1 and\n> keep stage0 entry in \"old\".  Then we continue on to the next\n> path.  When it happens to be:\n\nYou're right. I messed up. Again. And the test-case I used for this \nhappened to not care (I think it may have been the only file, and as such \nthe next path never happened).\n\nAnyway, you definitely get the idea.\n\nAnother way of encoding the rules: we _should_ eventually aim for a\nsituation where if a git-read-tree succeeds (even if it leaves crap\nentries in the tree), we should be able to do a\n\n\tgit-read-tree -u -m <OLDHEAD>\n\nand get back to the old state - or at least \"close enough\" (right now \"-u\"  \ndoesn't do anything for the single-tree case, and \"-m\" complains about\nunmerged entries, but the point isn't that it works today, the point is \nwhat should be possible at some point ;)\n\n\t\tLinus\n"},{"id":"4651","messageId":"7voeajp875.fsf@assigned-by-dhcp.cox.net","threadId":"845","inReplyTo":"Pine.LNX.4.58.0506061210490.1876@ppc970.osdl.org","subject":"Re: clarifying two tree merge semantics","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-06T19:59:58Z","receivedAt":"2005-06-06T19:59:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"By the way, there is one case that you need to keep an eye on if\nyou are making further fixes to \"git-read-tree -m $H $M\",\nespecially now we are talking about keeping what we slurp from\nstage0 sometimes:\n\n (1) index has DF (file)\n (2) $H has DF (file); the index matches it.\n (3) $M has DF/DF (file), implicitly making DF a directory.\n\nFor path DF, this rule from the earlier matrix applies:\n\n  exists          exists (index=$H)       no such path        *2*\n  * path is removed.\n\nFor path DF/DF, this rule from the earlier matrix applies:\n\n  no such path    no such path            exists\n  * take $M without complaining.\n\nThanks to *2*, the current code gets it right and we do not end\nup with a cache that records both DF and DF/DF at the same time.\nI cannot tell if it is by design or by accident :-).\n\nThere is a safety valve at the end of write-tree to refuse to\nwrite out such a nonsensical tree but as Pasky argued back then\n(a similar problem was discussed and resolved when you were\naway, around the beginning of May, involving update-cache), when\nthat safety valuve was added, the damage has already been done\nif we allow such a cache entry to be created in the first place.\n\n"}]}