{"thread":{"id":"12133","subject":"Rewriting boundary parents when path-limiting","startedAt":"2008-02-16T12:10:21Z","lastAt":"2008-02-24T01:55:48Z","messageCount":3,"participants":["Paul Mackerras","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"68897","messageId":"18358.53933.747911.449078@cargo.ozlabs.ibm.com","threadId":"12133","inReplyTo":null,"subject":"Rewriting boundary parents when path-limiting","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-02-16T12:10:21Z","receivedAt":"2008-02-16T12:10:21Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"When using both path and commit limiting, git log --parents --boundary\nrewrites the parents that are actually in the graph but doesn't\nrewrite the parents on the boundary, that is, the boundary parents may\nbe commits that don't modify any of the specified paths.  I'm\nwondering if there is a way to get the boundary parents rewritten too.\n\nThe reason is this.  One of the nice things about the dev branch of\ngitk is that updating the graph is really quick, because the git log\nthat gitk does when updating excludes all the commits that gitk has\nalready seen, using commit limiting.\n\nWithout path limiting, that works nicely, and the boundary of the new\nset consists of commits that are already in the graph, so gitk can\njoin the new graph into the old.  But if path limiting is being used,\nthe boundary commits of the new set aren't necessarily commits that\nare already in the graph, so gitk can't (at present) join the new\ngraph to the old properly.\n\nHere's an example: suppose the repository looks like this:\n\nHEAD ->\tc\tmodifies file x\n\t|\n\tb\tmodifies file y\n\t|\n\ta\tmodifies file x\n\nSuppose you run \"gitk -- x\", so gitk will display:\n\n\tc\n\t|\n\ta\n\nThen suppose commit c gets amended, and still modifies file x, so the\nHEAD is now commit c', whose parent is b.  Then if you do \"Update\" in\ngitk (or press F5), gitk will do\n\n\tgit log HEAD ^c -- x\n\nand it will get one entry, which is c', with b listed as its parent.\nSo then gitk ends up displaying:\n\n\tc'\n\t|\n\tb\n\n\tc\n\t|\n\ta\n\nbecause it doesn't realize that the parent of c' in the path-limited\ngraph should be a.\n\nSo can anyone suggest a good way for gitk to know that the parent of\nc' should be a in this situation?\n\nPaul.\n"},{"id":"69297","messageId":"alpine.LFD.1.00.0802190921460.7833@woody.linux-foundation.org","threadId":"12133","inReplyTo":"18358.53933.747911.449078@cargo.ozlabs.ibm.com","subject":"Re: Rewriting boundary parents when path-limiting","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-19T17:41:05Z","receivedAt":"2008-02-19T17:41:05Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 16 Feb 2008, Paul Mackerras wrote:\n>\n> When using both path and commit limiting, git log --parents --boundary\n> rewrites the parents that are actually in the graph but doesn't\n> rewrite the parents on the boundary, that is, the boundary parents may\n> be commits that don't modify any of the specified paths.  I'm\n> wondering if there is a way to get the boundary parents rewritten too.\n\nHmm. Right now we never rewrite uninteresting parents. The reason is \nrather simple: we simply haven't followed the chain down uninteresting \ncommits!\n\nChanging that is pretty hard. The thing is, we do the commit range \nlimiting mostly separately from the commit parent simplification: one \nhappens while actually walking the commits, the other one happens \nafter-the-fact when you do the \"simplify_commit()\" thing.\n\nIn addition to that problem (which is likely solvable: we do actually do \na lot of the groundwork for the later simplification stage during the \nfirst stage, so we might be able to just move more of that logic up), we \nactually explicitly do *not* simplify uninteresting commits, ie we have:\n\n        if (!revs->simplify_history || (p->object.flags & UNINTERESTING)) {\n                /* Even if a merge with an uninteresting\n                 * side branch brought the entire change\n                 * we are interested in, we do not want\n                 * to lose the other branches of this\n                 * merge, so we just keep going.\n                 */\n                pp = &parent->next;\n                continue;\n        }\n\nin there, where the comment says it all: we literally *avoid* simplifying \nuninteresting parents, and if the commit itself is uninteresting, we go \neven further, and don't even compare the trees at all! (See the big \ncomment in revision.c: add_parents_to_list, and note how it checks for the \nUNINTERESTING bit and returns early without even calling \ntry_to_simplify_commit() for such a commit).\n\nSo right now, not only is the code not really organized for what you ask, \nit actually explicitly tries to avoid doing what you ask. \n\nThat said, I'm not sure either of these problems are really impossible (or \neven hard) issues to avoid.\n\nWe could simply move the \"try_to_simplify_commit()\" call up, and remove \nsome of the code that explicitly avoids simplifying negative commits. But \nthe logic on when to stop even traversing the list is fundamentally pretty \nhard, and that might require splitting up UNINTERESTING into two separate \nbits: a \"fairly uninteresting\" bit (which means that it is negative) and a \n\"_really_ uninteresting bit\" (which means that not only was it negtive, \nbut it also changed the tree, so the parents of this commit are *really* \nnot interesting any more, even after simplification).\n\nThen the \"can we stop traversing the tree\" code would have to look at the \n\"really uninteresting\" bit rather than just the regular uninteresting one.\n\nQuite frankly, I suspect it's not worth it, and maybe you just shouldn't \ndo that optimization and limit the commits in other ways instead (ie you \nmight try to limit them *numerically* instead of by using negative \ncommits, and do one first run with the number of commits limited to <n>, \nand then if that wasn't enough to re-connect the trees, you do the whole \nthing)\n\n\n\t\tLinus\n"},{"id":"69755","messageId":"18368.52900.408974.782802@cargo.ozlabs.ibm.com","threadId":"12133","inReplyTo":"alpine.LFD.1.00.0802190921460.7833@woody.linux-foundation.org","subject":"Re: Rewriting boundary parents when path-limiting","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-02-24T01:55:48Z","receivedAt":"2008-02-24T01:55:48Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> Quite frankly, I suspect it's not worth it, and maybe you just shouldn't \n> do that optimization and limit the commits in other ways instead (ie you \n> might try to limit them *numerically* instead of by using negative \n> commits, and do one first run with the number of commits limited to <n>, \n> and then if that wasn't enough to re-connect the trees, you do the whole \n> thing)\n\nOK.  I have fixed the problem in gitk by making it do:\n\n    git rev-list --first-parent --max-count=1 $id -- paths...\n\nfor each id that it gets while updating that is a boundary commit, if\nwe are doing path limiting and the id is one that isn't in the graph.\nIf what we get back is a commit that is already in the graph, then we\nuse it instead.  That seems to do what I want.\n\nCurrently gitk executes that git rev-list synchronously, but if that\ncauses noticeable pauses, it could be done asynchronously instead.\n\nPaul.\n"}]}