{"thread":{"id":"12338","subject":"Suggestion: make --left-right work with --merge","startedAt":"2008-02-27T02:49:07Z","lastAt":"2008-03-01T10:59:01Z","messageCount":13,"participants":["Paul Mackerras","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"70056","messageId":"18372.53155.854763.12637@cargo.ozlabs.ibm.com","threadId":"12338","inReplyTo":null,"subject":"Suggestion: make --left-right work with --merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-02-27T02:49:07Z","receivedAt":"2008-02-27T02:49:07Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"It would be nice if git-log --merge --left-right did what git log\n--merge does but additionally, if the merge is a 2-way merge, marked\ncommits with '<' or '>' according to which side of the merge they came\nfrom. Would that be possible? I think the visual indication that\n--left-right gives in gitk would be useful with --merge.\n\nPaul.\n"},{"id":"70061","messageId":"7v7igqyii9.fsf@gitster.siamese.dyndns.org","threadId":"12338","inReplyTo":"18372.53155.854763.12637@cargo.ozlabs.ibm.com","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-27T07:18:38Z","receivedAt":"2008-02-27T07:18:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> It would be nice if git-log --merge --left-right did what git log\n> --merge does but additionally, if the merge is a 2-way merge, marked\n> commits with '<' or '>' according to which side of the merge they came\n> from. Would that be possible? I think the visual indication that\n> --left-right gives in gitk would be useful with --merge.\n\nLike this?\n\nA trivial feature that can be implemented in -4 lines ;-)\n\n revision.c | 12 ++++--------\n 1 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 4e36b95..38d7d94 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -771,14 +771,9 @@ static void prepare_show_merge(struct rev_info *revs)\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n \tbases = get_merge_bases(head, other, 1);\n-\twhile (bases) {\n-\t\tstruct commit *it = bases->item;\n-\t\tstruct commit_list *n = bases->next;\n-\t\tfree(bases);\n-\t\tbases = n;\n-\t\tit->object.flags |= UNINTERESTING;\n-\t\tadd_pending_object(revs, &it->object, \"(merge-base)\");\n-\t}\n+\tadd_pending_commit_list(revs, bases, UNINTERESTING);\n+\tfree_commit_list(bases);\n+\thead->object.flags |= SYMMETRIC_LEFT;\n \n \tif (!active_nr)\n \t\tread_cache();\n@@ -797,6 +792,7 @@ static void prepare_show_merge(struct rev_info *revs)\n \t\t\ti++;\n \t}\n \trevs->prune_data = prune;\n+\trevs->limited = 1;\n }\n \n int handle_revision_arg(const char *arg, struct rev_info *revs,\n"},{"id":"70169","messageId":"18373.58839.636432.448970@cargo.ozlabs.ibm.com","threadId":"12338","inReplyTo":"7v7igqyii9.fsf@gitster.siamese.dyndns.org","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-02-27T22:36:07Z","receivedAt":"2008-02-27T22:36:07Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> Like this?\n> \n> A trivial feature that can be implemented in -4 lines ;-)\n\nCool! Yes, that works nicely.\n\nAnother thing: I have noticed that git rev-parse --revs-only doesn't\npass the --merge and --left-right flags through, even though the help\nfor git rev-parse says that --revs-only means \"Do not output flags and\nparameters not meant for git-rev-list\" (which I assume means the same\nas \"Only output flags and parameters meant for git rev-list\"), and git\nrev-list accepts and uses the --merge and --left-right flags. I\nassume this is just an oversight in git rev-parse.\n\nRegards,\nPaul.\n"},{"id":"70171","messageId":"7v1w6yqaim.fsf@gitster.siamese.dyndns.org","threadId":"12338","inReplyTo":"18373.58839.636432.448970@cargo.ozlabs.ibm.com","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-27T22:50:41Z","receivedAt":"2008-02-27T22:50:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> Another thing: I have noticed that git rev-parse --revs-only doesn't\n> pass the --merge and --left-right flags through, even though the help\n> for git rev-parse says that --revs-only means \"Do not output flags and\n> parameters not meant for git-rev-list\" (which I assume means the same\n> as \"Only output flags and parameters meant for git rev-list\"), and git\n> rev-list accepts and uses the --merge and --left-right flags. I\n> assume this is just an oversight in git rev-parse.\n\nYeah, the option list is not maintained well for the last few\nyears and needs to be updated.\n"},{"id":"70241","messageId":"18374.39253.408961.634788@cargo.ozlabs.ibm.com","threadId":"12338","inReplyTo":"7v1w6yqaim.fsf@gitster.siamese.dyndns.org","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-02-28T11:21:57Z","receivedAt":"2008-02-28T11:21:57Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> Yeah, the option list is not maintained well for the last few\n> years and needs to be updated.\n\nAh. OK. I am currently using git rev-parse in the gitk dev branch to\ndetermine what starting commits git log will use, so that when the\nuser does an update, I can construct a git log command that will stop\nwhen it gets to any of those previous starting points. That way the\nsecond git log will only give me new stuff that has been added since\nthe first git log.\n\nCan you suggest an efficient way to do that that doesn't use git\nrev-parse?\n\nThanks,\nPaul.\n"},{"id":"70364","messageId":"7vprugdxpj.fsf@gitster.siamese.dyndns.org","threadId":"12338","inReplyTo":"18374.39253.408961.634788@cargo.ozlabs.ibm.com","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-29T07:32:40Z","receivedAt":"2008-02-29T07:32:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> Junio C Hamano writes:\n>\n>> Yeah, the option list is not maintained well for the last few\n>> years and needs to be updated.\n>\n> Ah. OK. I am currently using git rev-parse in the gitk dev branch to\n> determine what starting commits git log will use, so that when the\n> user does an update, I can construct a git log command that will stop\n> when it gets to any of those previous starting points. That way the\n> second git log will only give me new stuff that has been added since\n> the first git log.\n\nDoesn't\n\n\tgit rev-parse --revs-only --no-flags \"$@\" | grep '^[0-9a-f]'\n\ngive you what you want?\n\nInteresting.\n\n * gitk has already done what the user asked once. E.g. \"git\n log next..master\" to find out new trivially correct fixes\n that are already applied to 'master' and will be brought to\n 'next' when I merge 'master' to 'next' next time;\n\n * The user does \"git merge master\" while on 'next', and tell\n gitk to \"Update\".\n\n * gitk is expected to re-run \"git log next..master\" (textually\n the same command line), but most part of the graph it already\n knows about. You need a way to omit what you already know,\n so when you run the query for the first time you would want\n to remember the actual object names, so that you use them to\n tweak the query to \"git log next..master --not \" for the second query.\n\nIn the above example, the set of commits actually will shrink\n('next' moves and you will exclude more than before upon\n\"Update\").\n\nIf you are only interested in cases that the only positive end\ngrows (in the above example, the user adds commit to 'master'\ninstead of merging it into 'next'), then grabbing only the\npositive ends (e.g. 'master' in 'next..master'), negate them and\nappend them to the original query would speed things up, but\nobviously that would not work in the above example.\n\n"},{"id":"70366","messageId":"7vbq60dwx6.fsf@gitster.siamese.dyndns.org","threadId":"12338","inReplyTo":"18373.58839.636432.448970@cargo.ozlabs.ibm.com","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-29T07:49:41Z","receivedAt":"2008-02-29T07:49:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> Junio C Hamano writes:\n>\n>> Like this?\n>> \n>> A trivial feature that can be implemented in -4 lines ;-)\n>\n> Cool! Yes, that works nicely.\n\nI did not check this myself, but do arrows point in the right\ndirection? I just randomly painted HEAD side left (and other\nside right) in the patch.\n"},{"id":"70377","messageId":"18375.58359.687664.855599@cargo.ozlabs.ibm.com","threadId":"12338","inReplyTo":"7vprugdxpj.fsf@gitster.siamese.dyndns.org","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-02-29T10:52:39Z","receivedAt":"2008-02-29T10:52:39Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> Doesn't\n> \n> \tgit rev-parse --revs-only --no-flags \"$@\" | grep '^[0-9a-f]'\n> \n> give you what you want?\n\nWell, it does, except for --merge, which is perhaps a special case.\n\n(Actually, what would git rev-parse --revs-only --no-flags output that\nisn't a SHA1 ID? Why do you have the grep command there?)\n\nAlso, --left-right with symmetric difference is an interesting case,\nbecause git rev-parse will turn a symmetric difference into three SHA1\nIDs, with the 3rd one negated. If I pass what I get from git\nrev-parse to git log --left-right, then git log doesn't recognize that\nas a symmetric difference and shows all commits with the \">\" marker.\n\n> Interesting.\n> \n> * gitk has already done what the user asked once. E.g. \"git\n> log next..master\" to find out new trivially correct fixes\n> that are already applied to 'master' and will be brought to\n> 'next' when I merge 'master' to 'next' next time;\n> \n> * The user does \"git merge master\" while on 'next', and tell\n> gitk to \"Update\".\n> \n> * gitk is expected to re-run \"git log next..master\" (textually\n> the same command line), but most part of the graph it already\n> knows about. You need a way to omit what you already know,\n> so when you run the query for the first time you would want\n> to remember the actual object names, so that you use them to\n> tweak the query to \"git log next..master --not in the first round>\" for the second query.\n> \n> In the above example, the set of commits actually will shrink\n> ('next' moves and you will exclude more than before upon\n> \"Update\").\n\n\"Update\" adds new commits and moves the head/tag markers as necessary,\nbut doesn't remove commits from the graph, because that's a bit hard.\nThere is also a \"Reload\" function that restarts from scratch and so\nwill not show the newly-removed commits (i.e., the same as \"Update\"\ndoes now in current gitk master), but it is a lot slower than\n\"Update\". In the common case (a few commits added), \"Update\" takes\nless than a second.\n\n> If you are only interested in cases that the only positive end\n> grows (in the above example, the user adds commit to 'master'\n> instead of merging it into 'next'), then grabbing only the\n> positive ends (e.g. 'master' in 'next..master'), negate them and\n> append them to the original query would speed things up, but\n> obviously that would not work in the above example.\n\nThat's essentially what I do. I think I'll just have to do special\ncases for --merge and --left-right.\n\nPaul.\n"},{"id":"70408","messageId":"7vfxvbd0nu.fsf@gitster.siamese.dyndns.org","threadId":"12338","inReplyTo":"18375.58359.687664.855599@cargo.ozlabs.ibm.com","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-29T19:26:29Z","receivedAt":"2008-02-29T19:26:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> Junio C Hamano writes:\n>\n>> Doesn't\n>> \n>> \tgit rev-parse --revs-only --no-flags \"$@\" | grep '^[0-9a-f]'\n>> \n>> give you what you want?\n>\n> Well, it does, except for --merge, which is perhaps a special case.\n>\n> (Actually, what would git rev-parse --revs-only --no-flags output that\n> isn't a SHA1 ID? Why do you have the grep command there?)\n\nTry it on user inputs like \"master..next\", \"next...master\". You\nwanted to grab only the positive ones, no?\n\n> That's essentially what I do. I think I'll just have to do special\n> cases for --merge and --left-right.\n\nAre you sure? I can imagine that --merge may be problematic,\nbecause it implicitly is about MERGE_HEAD...HEAD, but rev-parse\nmay not know about it. I think --left-right and symmetric\nshould be fine.\n\n\t$ set x --left-right master...next; shift\n\t$ git rev-parse --revs-only --no-flags \"$@\" |\n git name-rev --stdin\n 88113cfea67557067f1363b210b803637d186632 (next)\n 97b97c58e609a1cd23b3c2514f41cdb0405870ee (master)\n ^97b97c58e609a1cd23b3c2514f41cdb0405870ee (master)\n\nIn the above case, master is a strict ancestor of next, but the\nthird line that is negative (and my grep would exclude) is\ngenerally the merge-base between the left and right positives.\nThe two positives are what you wanted to exclude from the second\nround, aren't they?\n\nAnd in order to handle them sanely without worrying about\nrevision machinery and rev-parse going out-of-sync, I would\nalmost suggest doing something like the attached patch and say:\n\n\t$ git rev-list --no-walk \"$@\"\n\nThe user options can contain --left-right and --boundary, so the\noutput from this could begin with \">\", \"<\" and \"-\". If you\ndiscard the ones that are prefixed with \"-\" (i.e. the user had\nthe --boundary option in \"$@\"), grab the ones that are not\nprefixed (i.e. the user did not have the --left-right option in\n\"$@\"), and the ones prefixed with \">\" and \"<\" (i.e. the user did\nhave --left-right, and you would strip the prefix from such a\nline), you would find all positive ones the user specified, I\nthink.\n\n-- >8 --\nrevision --no-walk: do not refuse to show the list with negatives\n\nScripts may want to find out what revs the user gave from the\ncommand line.\n\n\tgit rev-list --no-walk --boundary \"$@\"\n\nwould be a natural way to find out what revs (with flags) they\nare, but --no-walk refused to work if there are negative ones.\nLift that restriction.\n \nSigned-off-by: Junio C Hamano \n---\n\n revision.c | 2 --\n 1 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0eb6faa..c0931eb 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -129,8 +129,6 @@ void mark_parents_uninteresting(struct commit *commit)\n \n static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)\n {\n-\tif (revs->no_walk && (obj->flags & UNINTERESTING))\n-\t\tdie(\"object ranges do not make sense when not walking revisions\");\n \tif (revs->reflog_info && obj->type == OBJ_COMMIT &&\n \t\t\tadd_reflog_for_walk(revs->reflog_info,\n \t\t\t\t(struct commit *)obj, name))\n\n"},{"id":"70419","messageId":"alpine.LFD.1.00.0802291228560.17889@woody.linux-foundation.org","threadId":"12338","inReplyTo":"18375.58359.687664.855599@cargo.ozlabs.ibm.com","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-29T21:05:11Z","receivedAt":"2008-02-29T21:05:11Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 29 Feb 2008, Paul Mackerras wrote:\n> \n> Well, it does, except for --merge, which is perhaps a special case.\n\nWell, clearly --merge is a special case, but it probably really shouldn't \nbe.\n\nIf you want to parse revisions, then --merge should do that. But --merge \nis really special in that it expands into *both* a revision range *and* a \nset of files, so it doesn't really work wonderfully well for the trivial \nkind of parsing that git-rev-parse does.\n\nSo we certainly *could* just add \"--merge\" to the list of magic flags that \nis known about by is_rev_argument(), since that's a trivial one-liner. But \nthat's really only ok for the case where we are supposed to output both \nrevisions *and* flags, and even then it should also suppress the default \nHEAD generation (although it won't actually hurt, since the revision \noutput from --merge will include HEAD in teh \"HEAD...MERGE_HEAD\" logic \nanyway, so with --default always being HEAD in practice..)\n\nSo yeah, I think git-rev-list has trouble handling --merge sanely the way \nit is currently written.\n\n\t\tLinus\n"},{"id":"70489","messageId":"18377.2084.30531.202087@cargo.ozlabs.ibm.com","threadId":"12338","inReplyTo":"7vfxvbd0nu.fsf@gitster.siamese.dyndns.org","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-03-01T07:39:16Z","receivedAt":"2008-03-01T07:39:16Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> Try it on user inputs like \"master..next\", \"next...master\". You\n> wanted to grab only the positive ones, no?\n\nNo... gitk passes the IDs it gets from git rev-parse (both positive\nand negative) to git log, rather than the original arguments. This is\nto avoid gitk having an incorrect idea about what commits git log has\nstarted from, in the situation where somebody changes some head or tag\nbetween when gitk does the git rev-parse and when it does the git log.\n\nThat unfortunately means --left-right doesn't work however, because\ngit log doesn't recognize \"a b ^merge_base(a,b)\" as being equivalent\nto \"a...b\".\n\n> And in order to handle them sanely without worrying about\n> revision machinery and rev-parse going out-of-sync, I would\n> almost suggest doing something like the attached patch and say:\n> \n> \t$ git rev-list --no-walk \"$@\"\n> \n> The user options can contain --left-right and --boundary, so the\n> output from this could begin with \">\", \"<\" and \"-\". If you\n> discard the ones that are prefixed with \"-\" (i.e. the user had\n> the --boundary option in \"$@\"), grab the ones that are not\n> prefixed (i.e. the user did not have the --left-right option in\n> \"$@\"), and the ones prefixed with \">\" and \"<\" (i.e. the user did\n> have --left-right, and you would strip the prefix from such a\n> line), you would find all positive ones the user specified, I\n> think.\n\nYes, that would be useful. Now, if I can just think of a way to avoid\nthe race, or work around it, the problem will be solved. :) I can\nprobably do that by checking whether I see all the (positive) commits\nfrom git rev-list --no-walk in the git log output.\n\nThanks,\nPaul.\n"},{"id":"70493","messageId":"7vejau6fvf.fsf@gitster.siamese.dyndns.org","threadId":"12338","inReplyTo":"18377.2084.30531.202087@cargo.ozlabs.ibm.com","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-01T07:52:04Z","receivedAt":"2008-03-01T07:52:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> Junio C Hamano writes:\n>\n>> Try it on user inputs like \"master..next\", \"next...master\". You\n>> wanted to grab only the positive ones, no?\n>\n> No... gitk passes the IDs it gets from git rev-parse (both positive\n> and negative) to git log, rather than the original arguments.\n\nI am not sure if I was answering the right question, then.\n\nI thought the issue you were addressing was this.\n\n (1) the user says \"gitk master...next\" to start you.\n\n (2) you run \"git log master...next\" (or whatever the equivalent of what\n the user gave you) and draw.\n\n (3) the user does things outside your control to modify refs, and says\n \"Update\".\n\n (4) you could re-run \"git log master...next\" again, but that would show\n mostly what you already know.\n\n If you saved all the positive ones you used in (2), you could instead\n run \"git log master...next --not \" to ask only the\n incremental changes (as long as the branches do not rewind, but you\n do not discard nodes from the already drawn graph anyway).\n\n in this example sequence would be the object names\n for master and next when (2) was run.\n\nI think your \"gitk passes the IDs\" part is about the master...next part in\nthe above example. I do not think it really matters if they are converted\nto object names or kept symbolic when given to \"git log\". I was talking\nabout the part you add after --not in the second round, and that was why\nmy answer was only about positive ones.\n\nCome to think of it, when you are told to \"Update\", you already know the\npositive tips you can use to optimize (4), don't you? They are the\ncommits you drew in (3) that do not have children.\n"},{"id":"70509","messageId":"18377.14069.25974.10203@cargo.ozlabs.ibm.com","threadId":"12338","inReplyTo":"7vejau6fvf.fsf@gitster.siamese.dyndns.org","subject":"Re: Suggestion: make --left-right work with --merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-03-01T10:59:01Z","receivedAt":"2008-03-01T10:59:01Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> I think your \"gitk passes the IDs\" part is about the master...next part in\n> the above example. I do not think it really matters if they are converted\n> to object names or kept symbolic when given to \"git log\".\n\nThe only time it would matter is if they were kept symbolic AND\nsomehow the user changed them between the git rev-parse (or git\nrev-list) invocation and the git log invocation.\n\n> Come to think of it, when you are told to \"Update\", you already know the\n> positive tips you can use to optimize (4), don't you? They are the\n> commits you drew in (3) that do not have children.\n\nIt's possible the user could do \"Update\" before we had finished\nreading stuff from the first git log, so I don't necessarily know all\nthe commits that have no children. And in fact that's a reasonable\nthing to do if gitk is being used on a really really large\nrepository.\n\nI'm a kernel guy - I worry about race conditions. :)\n\nThanks for the tip about git rev-list --no-walk.\n\nPaul.\n"}]}