{"thread":{"id":"39920","subject":"[RFC/PATCH] log: add log.firstparent option","startedAt":"2015-07-23T01:23:44Z","lastAt":"2015-07-27T04:55:33Z","messageCount":32,"participants":["Jeff King","David Aguilar","Jacob Keller","Michael J Gruber","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"266639","messageId":"20150723012343.GA21000@peff.net","threadId":"39920","inReplyTo":null,"subject":"[RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-23T01:23:44Z","receivedAt":"2015-07-23T01:23:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This patch adds an option to turn on --first-parent all the\ntime, along with the corresponding --no-first-parent to\ndisable it. The \"why\" of this requires a bit of backstory.\n\nSome projects (like git.git) encourage frequent rebasing to\ngenerate a set of clean, bisectable patches for each topic.\nThe messy sequence of bug-ridden and bug-fixup commits is\nlost in the rebase, and not part of the final history.\n\nBut other projects prefer to keep the messy history intact.\nFor one thing, it makes collaboration on a topic easier, as\ndevelopers can simply pull from each other during the messy\ndevelopment. And two, that history may later be useful when\ntracking down a bug, because it gives more insight into the\nactual thought process of the developer.\n\nBut in this latter case you want _two_ views of history. You\nmay want to see the \"simple\" version in which a series of\nfully-formed topics hit the branch (and you would like to\nsee the diff of their final form). Or you may want to see\nthe messy details, because you are digging into a bug\nrelated to the topic.\n\nOne proposal we have seen in the past is to keep the messy\nhistory as a \"shadow\" parent of the real commits. That is,\nto introduce a new parent-like header into the commit\nobject, but not traverse it by default in \"git log\". So it\nremains hidden until you ask to dig into a particular topic\n(presumably with a \"log --show-messy-parents\" option or\nsimilar). So by default you get the simple view, but can dig\nfurther if you wish.\n\nBut we can observe that such shadow parents can be\nimplemented as real parents; the problem isn't one of the\nunderlying data structure, but how we present it in \"git\nlog\". In other words, a perfectly reasonable workflow is:\n\n - make your messy commits on a side branch\n\n - do a non-fast-forward merge to bring them into master.\n The commit message for this merge should be meaningful\n and describe the topic as a whole.\n\n - view the simple history with \"git log --first-parent -m\"\n\n - view the complex history with \"git log\"\n\nBut since you probably want to view the simple history most\nof the time, it would be nice to be able to default to that,\nand switch to the more complicated view with a command line\noption. Hence this patch.\n\nSuggested-by: Josh Bleecher Snyder \n---\nThis came out of a discussion I had with Josh as OSCON. I\ndon't think I would personally use it, because git.git is\nnot a messy-workflow project. But I think that GitHub pushes\npeople into this sort of workflow (the PR becomes the\ninteresting unit of change), and my understanding is that\nGerrit does, as well.\n\nThere are probably some other things we (and others) could\ndo to help support it:\n\n - currently \"--first-parent -p\" needs \"-m\" to show\n anything useful; this is being discussed elsewhere, and\n it would be nice if it Just Worked (and showed the diff\n between the merge and the first-parent)\n\n - the commit messages for merges are often not great. A\n few versions ago, I think, we started opening the editor\n for merges, which is good. GitHub's PR-merge includes\n the PR subject in the commit message, but not all of the\n rationale and discussion. And in both git-generated and\n GitHub-generated messages, the subject isn't amazing\n (it's \"merge topic jk/some-shorthand\", which is barely\n tolerable if you use good branch names; it could be\n something like the subject-line of the cover letter for\n the patch series).\n\n So I think this could easily be improved by GitHub (we\n have the PR subject and body, after all). It's harder\n for a mailing list project like git.git, because Git\n never actually sees the subject line. I think it would\n require teaching git-am the concept of a patch series.\n\n I don't know offhand what Gerrit merges look like.\n\n - we already have merge.ff to default to making extra\n merge commits. And if you use GitHub's UI to do the\n merge, it uses --no-ff. I don't think we would want\n these to become the default, so there's probably nothing\n else to be done there.\n\nSigned-off-by: Jeff King \n---\n Documentation/config.txt | 4 ++++\n builtin/log.c | 6 ++++++\n revision.c | 2 ++\n t/t4202-log.sh | 30 ++++++++++++++++++++++++++++++\n 4 files changed, 42 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3e37b93..e9c3763 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1802,6 +1802,10 @@ log.mailmap::\n \tIf true, makes linkgit:git-log[1], linkgit:git-show[1], and\n \tlinkgit:git-whatchanged[1] assume `--use-mailmap`.\n \n+log.firstparent::\n+\tIf true, linkgit:git-log[1] will default to `--first-parent`;\n+\tcan be overridden by supplying `--no-first-parent`.\n+\n mailinfo.scissors::\n \tIf true, makes linkgit:git-mailinfo[1] (and therefore\n \tlinkgit:git-am[1]) act by default as if the --scissors option\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8781049..3e9b034 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;\n \n static int default_abbrev_commit;\n static int default_show_root = 1;\n+static int default_first_parent;\n static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config;\n@@ -109,6 +110,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n+\trev->first_parent_only = default_first_parent;\n \tDIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n \n \tif (default_date_mode)\n@@ -396,6 +398,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tuse_mailmap_config = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"log.firstparent\")) {\n+\t\tdefault_first_parent = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (grep_config(var, value, cb) < 0)\n \t\treturn -1;\ndiff --git a/revision.c b/revision.c\nindex ab97ffd..a03a84b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1760,6 +1760,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\treturn argcount;\n \t} else if (!strcmp(arg, \"--first-parent\")) {\n \t\trevs->first_parent_only = 1;\n+\t} else if (!strcmp(arg, \"--no-first-parent\")) {\n+\t\trevs->first_parent_only = 0;\n \t} else if (!strcmp(arg, \"--ancestry-path\")) {\n \t\trevs->ancestry_path = 1;\n \t\trevs->simplify_history = 0;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 1b2e981..de1c35d 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -871,4 +871,34 @@ test_expect_success 'log --graph --no-walk is forbidden' '\n \ttest_must_fail git log --graph --no-walk\n '\n \n+test_expect_success 'setup simple merge for first-parent tests' '\n+\tgit tag fp-base &&\n+\ttest_commit master &&\n+\tgit checkout -b fp-side &&\n+\ttest_commit side &&\n+\tgit checkout master &&\n+\tgit merge --no-ff fp-side\n+'\n+\n+test_expect_success 'log.firstparent config turns on first-parent' '\n+\ttest_config log.firstparent true &&\n+\tcat >expect <<-\\EOF &&\n+\tMerge branch '\\''fp-side'\\''\n+\tmaster\n+\tEOF\n+\tgit log --format=%s fp-base.. >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --no-first-parent override log.firstparent' '\n+\ttest_config log.firstparent true &&\n+\tcat >expect <<-\\EOF &&\n+\tMerge branch '\\''fp-side'\\''\n+\tside\n+\tmaster\n+\tEOF\n+\tgit log --no-first-parent --format=%s fp-base.. >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.5.0.rc2.540.ge5d4f14\n"},{"id":"266640","messageId":"20150723044007.GA3651@gmail.com","threadId":"39920","inReplyTo":"20150723012343.GA21000@peff.net","subject":"Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-07-23T04:40:10Z","receivedAt":"2015-07-23T04:40:10Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Wed, Jul 22, 2015 at 06:23:44PM -0700, Jeff King wrote:\n> This patch adds an option to turn on --first-parent all the\n> time, along with the corresponding --no-first-parent to\n> disable it.\n\n[Putting on my scripter hat]\n\nI sometimes think, \"it would be really helpful if we had a way\nto tell Git that it should ignore config variables\".\n\nThis is especially helpful for script writers. It's pretty\neasy to break existing scripts by introducing new config knobs.\n\nFor example, \"user.name\" and \"user.email\" can be whitelisted by\nthe calling script and and everything else would just use the\nstock defaults.\n\nThat way, script writers don't have to do version checks to\nfiguring out when and when not to include flags like\n--no-first-parent, etc.\n\nWould something like,\n\n\tGIT_CONFIG_WHITELIST=\"user.email user.name\" \\\n\tgit ...\n\nbe a sensible interface to such a feature?\n-- \nDavid\n"},{"id":"266641","messageId":"20150723051445.GA24029@peff.net","threadId":"39920","inReplyTo":"20150723044007.GA3651@gmail.com","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-23T05:14:45Z","receivedAt":"2015-07-23T05:14:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 22, 2015 at 09:40:10PM -0700, David Aguilar wrote:\n\n> On Wed, Jul 22, 2015 at 06:23:44PM -0700, Jeff King wrote:\n> > This patch adds an option to turn on --first-parent all the\n> > time, along with the corresponding --no-first-parent to\n> > disable it.\n> \n> [Putting on my scripter hat]\n> \n> I sometimes think, \"it would be really helpful if we had a way\n> to tell Git that it should ignore config variables\".\n> \n> This is especially helpful for script writers. It's pretty\n> easy to break existing scripts by introducing new config knobs.\n\nI think the purpose of --no-first-parent here is slightly orthogonal. It\nis meant to help the user during the odd time that they need to\ncountermand their config.\n\nScript writers should not care here, because they should not be parsing\nthe output of the porcelain \"log\" command in the first place. It already\nhas many gotchas (e.g., log.date, log.abbrevCommit).\n\nI am sympathetic, though. There are some things that git-log can do that\nrev-list cannot, so people end up using it in scripts. I think you can\navoid it with a \"rev-list | diff-tree\" pipeline, though I'm not 100%\nsure if that covers all cases. But I would much rather see a solution\nalong the lines of making the plumbing cover more cases, rather than\ntrying to make the porcelain behave in a script.\n\n> That way, script writers don't have to do version checks to\n> figuring out when and when not to include flags like\n> --no-first-parent, etc.\n\nOne trick you can do is:\n\n git -c log.firstparent=false log ...\n\nOlder versions of git will ignore the unknown config option, and newer\nones will override anything the user has in their config file.\n\n> Would something like,\n> \n> \tGIT_CONFIG_WHITELIST=\"user.email user.name\" \\\n> \tgit ...\n> \n> be a sensible interface to such a feature?\n\nI dunno. That's at least easy to implement. But the existing suggested\ninterface is really \"run the plumbing\", and then it automatically has a\nsensible set of config options for each command, so that scripts don't\nhave to make their own whitelist (e.g., diff-tree still loads userdiff\nconfig, but not anything that would change the output drastically, like\ndiff.mnemonicprefix).\n\n-Peff\n"},{"id":"266642","messageId":"20150723054816.GA1917@peff.net","threadId":"39920","inReplyTo":"20150723051445.GA24029@peff.net","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-23T05:48:17Z","receivedAt":"2015-07-23T05:48:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 22, 2015 at 10:14:45PM -0700, Jeff King wrote:\n\n> Script writers should not care here, because they should not be parsing\n> the output of the porcelain \"log\" command in the first place. It already\n> has many gotchas (e.g., log.date, log.abbrevCommit).\n> \n> I am sympathetic, though. There are some things that git-log can do that\n> rev-list cannot, so people end up using it in scripts. I think you can\n> avoid it with a \"rev-list | diff-tree\" pipeline, though I'm not 100%\n> sure if that covers all cases. But I would much rather see a solution\n> along the lines of making the plumbing cover more cases, rather than\n> trying to make the porcelain behave in a script.\n\nAh, I see in a nearby thread that you just recently fixed a problem with\ngit-subtree and log.date, so I see now why you are so interested. :)\n\nAnd I was also reminded by that usage of why rev-list is annoying in\nscripts: even with \"--format\", it insists on writing the \"commit ...\"\nheader. I wonder if we could fix that...\n\n-Peff\n"},{"id":"266644","messageId":"CA+P7+xrNiK168rXX3SwwmQFPR6i4iA0nhFdfHKL7wqwtwvnEPA@mail.gmail.com","threadId":"39920","inReplyTo":"20150723054816.GA1917@peff.net","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-23T06:32:49Z","receivedAt":"2015-07-23T06:32:49Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Jul 22, 2015 at 10:48 PM, Jeff King wrote:\n> On Wed, Jul 22, 2015 at 10:14:45PM -0700, Jeff King wrote:\n>\n>> Script writers should not care here, because they should not be parsing\n>> the output of the porcelain \"log\" command in the first place. It already\n>> has many gotchas (e.g., log.date, log.abbrevCommit).\n>>\n>> I am sympathetic, though. There are some things that git-log can do that\n>> rev-list cannot, so people end up using it in scripts. I think you can\n>> avoid it with a \"rev-list | diff-tree\" pipeline, though I'm not 100%\n>> sure if that covers all cases. But I would much rather see a solution\n>> along the lines of making the plumbing cover more cases, rather than\n>> trying to make the porcelain behave in a script.\n>\n> Ah, I see in a nearby thread that you just recently fixed a problem with\n> git-subtree and log.date, so I see now why you are so interested. :)\n>\n> And I was also reminded by that usage of why rev-list is annoying in\n> scripts: even with \"--format\", it insists on writing the \"commit ...\"\n> header. I wonder if we could fix that...\n>\n> -Peff\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\nAgreed. Fix the plumbing instead and document how/why to use it\ninstead of the porcelain. We might do better to help clearly document\nwhich commands are porcelain and which are plumbing maybe by\nreferencing which plumbings to use in place of various porcelain\ncommands. I know, for example, that git status already does this.\n\nRegards,\nJake\n"},{"id":"266645","messageId":"20150723065310.GA7542@peff.net","threadId":"39920","inReplyTo":"CA+P7+xrNiK168rXX3SwwmQFPR6i4iA0nhFdfHKL7wqwtwvnEPA@mail.gmail.com","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-23T06:53:11Z","receivedAt":"2015-07-23T06:53:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 22, 2015 at 11:32:49PM -0700, Jacob Keller wrote:\n\n> Agreed. Fix the plumbing instead and document how/why to use it\n> instead of the porcelain. We might do better to help clearly document\n> which commands are porcelain and which are plumbing maybe by\n> referencing which plumbings to use in place of various porcelain\n> commands. I know, for example, that git status already does this.\n\n\"man git\" already has such a list (which is generated from the\nannotations in command-list.txt). But I agree that it would probably be\nhelpful to point people directly from \"git log\" to \"git rev-list\" and\nvice versa.\n\n-Peff\n"},{"id":"266646","messageId":"CA+P7+xrX5q_abQYZ-LkzAjvEKmZ+-ykJBS-DwOaLJ6d62-CHPw@mail.gmail.com","threadId":"39920","inReplyTo":"20150723065310.GA7542@peff.net","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-23T06:55:27Z","receivedAt":"2015-07-23T06:55:27Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Jul 22, 2015 at 11:53 PM, Jeff King wrote:\n> \"man git\" already has such a list (which is generated from the\n> annotations in command-list.txt). But I agree that it would probably be\n> helpful to point people directly from \"git log\" to \"git rev-list\" and\n> vice versa.\n>\n> -Peff\n\nThat's good. I just know that I've had many a co-worker complain\nbecause the man page felt too technical because they accidentally\nfound their way into a plumbing section. If I heard a specific case of\nconfusion again in the future I'll try to work up a patch for it.\n\nRegards,\nJake\n"},{"id":"266653","messageId":"55B0B9A1.7060609@drmicha.warpmail.net","threadId":"39920","inReplyTo":"CA+P7+xrX5q_abQYZ-LkzAjvEKmZ+-ykJBS-DwOaLJ6d62-CHPw@mail.gmail.com","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-07-23T09:53:37Z","receivedAt":"2015-07-23T09:53:37Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jacob Keller venit, vidit, dixit 23.07.2015 08:55:\n> On Wed, Jul 22, 2015 at 11:53 PM, Jeff King wrote:\n>> \"man git\" already has such a list (which is generated from the\n>> annotations in command-list.txt). But I agree that it would probably be\n>> helpful to point people directly from \"git log\" to \"git rev-list\" and\n>> vice versa.\n>>\n>> -Peff\n> \n> That's good. I just know that I've had many a co-worker complain\n> because the man page felt too technical because they accidentally\n> found their way into a plumbing section. If I heard a specific case of\n> confusion again in the future I'll try to work up a patch for it.\n> \n> Regards,\n> Jake\n> \n\nThat reminds me of my attempt to add those \"categories\" to the man pages\nof each command (rather than just to that of \"git\") so that users know\nwhere they landed. It died off, though: I preferred just specifying the\ncategory (maybe with a long form), others including the whole\nexplanation of the category (which I thought would be too much text; we\nhave the glossary for that).\n\nWould something like that help? Maybe \"category\" plus optionally pointer\nto a related command in the \"other\" category.\n\nMichael\n"},{"id":"266673","messageId":"20150723173542.GA18686@peff.net","threadId":"39920","inReplyTo":"55B0B9A1.7060609@drmicha.warpmail.net","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-23T17:35:43Z","receivedAt":"2015-07-23T17:35:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 23, 2015 at 11:53:37AM +0200, Michael J Gruber wrote:\n\n> That reminds me of my attempt to add those \"categories\" to the man pages\n> of each command (rather than just to that of \"git\") so that users know\n> where they landed. It died off, though: I preferred just specifying the\n> category (maybe with a long form), others including the whole\n> explanation of the category (which I thought would be too much text; we\n> have the glossary for that).\n> \n> Would something like that help? Maybe \"category\" plus optionally pointer\n> to a related command in the \"other\" category.\n\nMaybe a \"SCRIPTING\" section at the end of the page?\n\n-Peff\n"},{"id":"266674","messageId":"xmqqoaj2pygu.fsf@gitster.dls.corp.google.com","threadId":"39920","inReplyTo":"20150723051445.GA24029@peff.net","subject":"Re: Config variables and scripting // was Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-23T17:37:05Z","receivedAt":"2015-07-23T17:37:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I am sympathetic, though. There are some things that git-log can do that\n> rev-list cannot, so people end up using it in scripts. I think you can\n> avoid it with a \"rev-list | diff-tree\" pipeline, though I'm not 100%\n> sure if that covers all cases. But I would much rather see a solution\n> along the lines of making the plumbing cover more cases, rather than\n> trying to make the porcelain behave in a script.\n\nVery well said.\n"},{"id":"266689","messageId":"CAGZ79kbY8hAom+voAOourkDy34nwaACQEM-n=f25Qbg0+ha_8g@mail.gmail.com","threadId":"39920","inReplyTo":"20150723012343.GA21000@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-07-23T22:14:50Z","receivedAt":"2015-07-23T22:14:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jul 22, 2015 at 6:23 PM, Jeff King wrote:\n> This patch adds an option to turn on --first-parent all the\n> time, along with the corresponding --no-first-parent to\n> disable it. The \"why\" of this requires a bit of backstory.\n>\n> Some projects (like git.git) encourage frequent rebasing to\n> generate a set of clean, bisectable patches for each topic.\n> The messy sequence of bug-ridden and bug-fixup commits is\n> lost in the rebase, and not part of the final history.\n>\n> But other projects prefer to keep the messy history intact.\n> For one thing, it makes collaboration on a topic easier, as\n> developers can simply pull from each other during the messy\n> development. And two, that history may later be useful when\n> tracking down a bug, because it gives more insight into the\n> actual thought process of the developer.\n>\n> But in this latter case you want _two_ views of history. You\n> may want to see the \"simple\" version in which a series of\n> fully-formed topics hit the branch (and you would like to\n> see the diff of their final form). Or you may want to see\n> the messy details, because you are digging into a bug\n> related to the topic.\n>\n> One proposal we have seen in the past is to keep the messy\n> history as a \"shadow\" parent of the real commits. That is,\n> to introduce a new parent-like header into the commit\n> object, but not traverse it by default in \"git log\". So it\n> remains hidden until you ask to dig into a particular topic\n> (presumably with a \"log --show-messy-parents\" option or\n> similar). So by default you get the simple view, but can dig\n> further if you wish.\n>\n> But we can observe that such shadow parents can be\n> implemented as real parents; the problem isn't one of the\n> underlying data structure, but how we present it in \"git\n> log\". In other words, a perfectly reasonable workflow is:\n>\n> - make your messy commits on a side branch\n>\n> - do a non-fast-forward merge to bring them into master.\n> The commit message for this merge should be meaningful\n> and describe the topic as a whole.\n>\n> - view the simple history with \"git log --first-parent -m\"\n>\n> - view the complex history with \"git log\"\n>\n> But since you probably want to view the simple history most\n> of the time, it would be nice to be able to default to that,\n> and switch to the more complicated view with a command line\n> option. Hence this patch.\n>\n> Suggested-by: Josh Bleecher Snyder \n> ---\n> This came out of a discussion I had with Josh as OSCON. I\n> don't think I would personally use it, because git.git is\n> not a messy-workflow project. But I think that GitHub pushes\n> people into this sort of workflow (the PR becomes the\n> interesting unit of change), and my understanding is that\n> Gerrit does, as well.\n\nGithub pull request messages\nare similar to cover letters, so you could send a series with a\ngood cover letter, but crappy unfinished patches inside the series.\nAfter applying all patches it could be all nice, i.e. compiles, tests, adds the\nnew functionality. It might be just a commit in between which may not even\ncompile. That's my understanding of the messy-workflow.\n\nGerrit cannot provide such a workflow easily as it's rather commit\ncentric and not branch centered. So you need to approve each\ncommit on its own and until 2 weeks ago you even needed to submit\neach commit. (By now Gerrit has learned to submit a full branch, that\nis you submit a commit and all its ancestors will integrate as well if they\nwere approved.)\nPreviously when the ancestors were not approved the commit would be\n\"submitted, merge pending\", so it would wait for each commit to be approved\nand submitted.\nAnd because of this commit-centric workflow, the crappy commit in the series\nis put into spot light.\n\nApart from that, the first-parent option gains some traction currently, Compare\nhttps://git.eclipse.org/r/#/c/52381/ for example ;)\n\n>\n> There are probably some other things we (and others) could\n> do to help support it:\n>\n> - currently \"--first-parent -p\" needs \"-m\" to show\n> anything useful; this is being discussed elsewhere, and\n> it would be nice if it Just Worked (and showed the diff\n> between the merge and the first-parent)\n>\n> - the commit messages for merges are often not great. A\n> few versions ago, I think, we started opening the editor\n> for merges, which is good. GitHub's PR-merge includes\n> the PR subject in the commit message, but not all of the\n> rationale and discussion. And in both git-generated and\n> GitHub-generated messages, the subject isn't amazing\n> (it's \"merge topic jk/some-shorthand\", which is barely\n> tolerable if you use good branch names; it could be\n> something like the subject-line of the cover letter for\n> the patch series).\n>\n> So I think this could easily be improved by GitHub (we\n> have the PR subject and body, after all). It's harder\n> for a mailing list project like git.git, because Git\n> never actually sees the subject line. I think it would\n> require teaching git-am the concept of a patch series.\n\nThis would be cool I would imagine.\n\n>\n> I don't know offhand what Gerrit merges look like.\n\nLet's say it's complicated. Depending on configuration a few things\nmay happen. There are different integration strategies\n* merge always\n* merge if necessary (fastforward else)\n* fastforward only\n* rebase if necessary (to make it a linear history)\n* cherrypick (which may add footers like Reviewed-by: to have that\ninformation in the git history)\n\nI think the \"merge always\" strategy would comfort from this patch.\n\n>\n> - we already have merge.ff to default to making extra\n> merge commits. And if you use GitHub's UI to do the\n> merge, it uses --no-ff. I don't think we would want\n> these to become the default, so there's probably nothing\n> else to be done there.\n>\n> Signed-off-by: Jeff King \n\nThe signoff is better placed above :)\n\n> ---\n> Documentation/config.txt | 4 ++++\n> builtin/log.c | 6 ++++++\n> revision.c | 2 ++\n> t/t4202-log.sh | 30 ++++++++++++++++++++++++++++++\n> 4 files changed, 42 insertions(+)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 3e37b93..e9c3763 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1802,6 +1802,10 @@ log.mailmap::\n> If true, makes linkgit:git-log[1], linkgit:git-show[1], and\n> linkgit:git-whatchanged[1] assume `--use-mailmap`.\n>\n> +log.firstparent::\n> + If true, linkgit:git-log[1] will default to `--first-parent`;\n> + can be overridden by supplying `--no-first-parent`.\n> +\n> mailinfo.scissors::\n> If true, makes linkgit:git-mailinfo[1] (and therefore\n> linkgit:git-am[1]) act by default as if the --scissors option\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 8781049..3e9b034 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;\n>\n> static int default_abbrev_commit;\n> static int default_show_root = 1;\n> +static int default_first_parent;\n> static int decoration_style;\n> static int decoration_given;\n> static int use_mailmap_config;\n> @@ -109,6 +110,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n> rev->abbrev_commit = default_abbrev_commit;\n> rev->show_root_diff = default_show_root;\n> rev->subject_prefix = fmt_patch_subject_prefix;\n> + rev->first_parent_only = default_first_parent;\n> DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n>\n> if (default_date_mode)\n> @@ -396,6 +398,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n> use_mailmap_config = git_config_bool(var, value);\n> return 0;\n> }\n> + if (!strcmp(var, \"log.firstparent\")) {\n> + default_first_parent = git_config_bool(var, value);\n> + return 0;\n> + }\n>\n> if (grep_config(var, value, cb) < 0)\n> return -1;\n> diff --git a/revision.c b/revision.c\n> index ab97ffd..a03a84b 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1760,6 +1760,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> return argcount;\n> } else if (!strcmp(arg, \"--first-parent\")) {\n> revs->first_parent_only = 1;\n> + } else if (!strcmp(arg, \"--no-first-parent\")) {\n> + revs->first_parent_only = 0;\n> } else if (!strcmp(arg, \"--ancestry-path\")) {\n> revs->ancestry_path = 1;\n> revs->simplify_history = 0;\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 1b2e981..de1c35d 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -871,4 +871,34 @@ test_expect_success 'log --graph --no-walk is forbidden' '\n> test_must_fail git log --graph --no-walk\n> '\n>\n> +test_expect_success 'setup simple merge for first-parent tests' '\n> + git tag fp-base &&\n> + test_commit master &&\n> + git checkout -b fp-side &&\n> + test_commit side &&\n> + git checkout master &&\n> + git merge --no-ff fp-side\n> +'\n> +\n> +test_expect_success 'log.firstparent config turns on first-parent' '\n> + test_config log.firstparent true &&\n> + cat >expect <<-\\EOF &&\n> + Merge branch '\\''fp-side'\\''\n> + master\n> + EOF\n> + git log --format=%s fp-base.. >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --no-first-parent override log.firstparent' '\n> + test_config log.firstparent true &&\n> + cat >expect <<-\\EOF &&\n> + Merge branch '\\''fp-side'\\''\n> + side\n> + master\n> + EOF\n> + git log --no-first-parent --format=%s fp-base.. >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> test_done\n> --\n> 2.5.0.rc2.540.ge5d4f14\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"266690","messageId":"xmqqpp3io5km.fsf@gitster.dls.corp.google.com","threadId":"39920","inReplyTo":"20150723012343.GA21000@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-23T22:46:33Z","receivedAt":"2015-07-23T22:46:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But other projects prefer to keep the messy history intact.\n> For one thing, it makes collaboration on a topic easier, as\n> developers can simply pull from each other during the messy\n> development. And two, that history may later be useful when\n> tracking down a bug, because it gives more insight into the\n> actual thought process of the developer.\n>\n> But in this latter case you want _two_ views of history. You\n> may want to see the \"simple\" version in which a series of\n> fully-formed topics hit the branch (and you would like to\n> see the diff of their final form). Or you may want to see\n> the messy details, because you are digging into a bug\n> related to the topic.\n\nWhile I can see the reasoning behind the above [*1*], I am not sure\nif the output with \"--first-parent\" would always be a good match for\nthe \"simple\" version. Wouldn't the people who keep these messy\nhistories also those who merge project trunk into a random topic and\npush the result back as the updated project trunk? Admittedly, that\nmerely is saying that \"--first-parent\" is not a solution to such a\nproject, and does not say much about the usefulness of the new\nconfiguration, so I'd queue it as-is.\n\n[Footnote]\n\n*1* I do not necessarily agree, though. The history being messy is\n a sign that \"the actual thought process of the developer\" was\n not clearly expressed in the work, and it is not likely that you\n have more insight by looking at it---you will see more mess, for\n certain, though.\n"},{"id":"266713","messageId":"CA+P7+xoOkTfair3M7HH_TPs9qxBAUR5GodHGDuw8Ux_HaUvvyQ@mail.gmail.com","threadId":"39920","inReplyTo":"xmqqpp3io5km.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-24T06:07:58Z","receivedAt":"2015-07-24T06:07:58Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Jul 23, 2015 at 3:46 PM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> But other projects prefer to keep the messy history intact.\n>> For one thing, it makes collaboration on a topic easier, as\n>> developers can simply pull from each other during the messy\n>> development. And two, that history may later be useful when\n>> tracking down a bug, because it gives more insight into the\n>> actual thought process of the developer.\n>>\n>> But in this latter case you want _two_ views of history. You\n>> may want to see the \"simple\" version in which a series of\n>> fully-formed topics hit the branch (and you would like to\n>> see the diff of their final form). Or you may want to see\n>> the messy details, because you are digging into a bug\n>> related to the topic.\n>\n> While I can see the reasoning behind the above [*1*], I am not sure\n> if the output with \"--first-parent\" would always be a good match for\n> the \"simple\" version. Wouldn't the people who keep these messy\n> histories also those who merge project trunk into a random topic and\n> push the result back as the updated project trunk? Admittedly, that\n> merely is saying that \"--first-parent\" is not a solution to such a\n> project, and does not say much about the usefulness of the new\n> configuration, so I'd queue it as-is.\n>\n> [Footnote]\n>\n> *1* I do not necessarily agree, though. The history being messy is\n> a sign that \"the actual thought process of the developer\" was\n> not clearly expressed in the work, and it is not likely that you\n> have more insight by looking at it---you will see more mess, for\n> certain, though.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\nBased on the work I've done with people who don't mind \"messy\"\nhistory, I find that they tend to break the first-parent rule fairly\noften, and I don't personally find much value in the \"mess\" of their\nhistory. However, I think the ability for git-am to see a patch series\ncover-letter and include a merge message would be valuable, as there\nis often information lost in the cover letter when you apply a series.\nI'm not sure how much work this would take in practice though.\n\nI think some projects definitely benefit from the first-parent setup,\nand it could be valuable, but I do tend to agree with Junio here that\nthe mess is always helpful. If may be helpful if people's commit\nmessages on that mess are good, but generally those that don't take\nthe time to rebase local work and re-express the commit messages are\nnot going to leave insightful messages the first time. However, have\nthe ability to view history this way is still possibly valuable.\n\nRegards,\nJake\n"},{"id":"266715","messageId":"20150724072109.GB2111@peff.net","threadId":"39920","inReplyTo":"xmqqpp3io5km.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-24T07:21:10Z","receivedAt":"2015-07-24T07:21:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 23, 2015 at 03:46:33PM -0700, Junio C Hamano wrote:\n\n> While I can see the reasoning behind the above [*1*], I am not sure\n> if the output with \"--first-parent\" would always be a good match for\n> the \"simple\" version. Wouldn't the people who keep these messy\n> histories also those who merge project trunk into a random topic and\n> push the result back as the updated project trunk? Admittedly, that\n> merely is saying that \"--first-parent\" is not a solution to such a\n> project, and does not say much about the usefulness of the new\n> configuration, so I'd queue it as-is.\n> \n> [Footnote]\n> \n> *1* I do not necessarily agree, though. The history being messy is\n> a sign that \"the actual thought process of the developer\" was\n> not clearly expressed in the work, and it is not likely that you\n> have more insight by looking at it---you will see more mess, for\n> certain, though.\n\nI don't think people who keep \"messy\" commit histories are necessarily\nprone to pushing to the trunk. And ditto for the footnote, I do not\nthink \"messy\" is necessarily a bad thing. It is really about whether you\nchoose to rebase out follow-on fixups (e.g., those that come from\nreview), or whether you choose to leave them in.\n\nAnd there is a very good reason to leave them in: two people\ncollaborating on a topic have a much easier time if the other person is\nnot constantly rebasing.\n\nSo a GitHub-oriented workflow can be something like (and this is what we\nuse to develop GitHub internally):\n\n 1. I do the usual process of creating a branch, writing some commits,\n pushing them up, and opening a pull request. This is equivalent to\n git.git, except the final step is \"send to the list\". I may use\n \"rebase -i\" during this process to make a nice sequence of commits.\n\n 2. During review, I may get comments. In a git.git workflow, I am\n expected to re-roll the topic myself to address the comments. If I\n am lucky, the reviewer expressed their comments as a diff, and I\n can squash them in as appropriate. But in the GitHub workflow, the\n reviewer may simply check out the branch themselves locally, commit\n the fixes, and push them up. They become part of the PR.\n\n 3. I may go back and forth with the reviewer several times, and there\n may be intermingled commits from both of us. At the end, one of us\n decides to merge the PR, and GitHub's merge button creates a non-ff\n commit on master, merging in the topic.\n\nSo we're \"messy\" in the sense that we chose to record the back-and-forth\nduring the development of the topic. But at no point was anybody in\ndanger of back-merging and putting the result directly onto master. Even\nif they did back-merge from master onto the topic[1], the non-ff merge\nto master means that any first-parent traversal starting from master\nwill never stray onto a PR branch at all.\n\nOf course not everybody does this. But I think it's a perfectly sane\nworkflow. The PR merges become the most interesting unit of change to\nreview or find in the history, with the individual commits closer to\n\"what really happened\".\n\n-Peff\n\n[1] We do actually back-merge master to topics all the time, because we\n have to make sure that we whatever we test and deploy contains all\n of what is already in master, even if the topic was originally\n forked from an older point. IOW, the QA process for merging is\n basically:\n\n - merge back from master so we know the tree at the tip of our\n\ttopic is what _would_ be on master if we merged\n\n - test that tip (with automated tests, temporarily deploying to\n\tthe live site, etc)\n\n - it's OK to merge to master if it has not moved; the merge would\n be a fast-forward. Instead, we create a merge commit, but the\n\ttree content will be the same (so we know that we are merging\n\tand deploying what has just passed QA).\n\n Incidentally, doing \"git log --first-parent\" while on the topic\n branch from this workflow does the reasonable thing. Because the\n merges on the topic branch are all backwards merges from master,\n following the first parent keeps you on the topic branch (rather\n than showing you all the upstream commits you just pulled in).\n\n Where I'd be most worried about a default first-parent hurting you\n is when you \"git log $some_random_commit\", at which point you may or\n may not want first-parent behavior. I suppose we could have a config\n option that kicks in only when using the default \"HEAD\". That seems\n more likely to do what you want all the time, but it also sounds\n rather confusingly magical.\n"},{"id":"266716","messageId":"20150724072325.GC2111@peff.net","threadId":"39920","inReplyTo":"xmqqpp3io5km.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-24T07:23:27Z","receivedAt":"2015-07-24T07:23:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 23, 2015 at 03:46:33PM -0700, Junio C Hamano wrote:\n\n> Admittedly, that\n> merely is saying that \"--first-parent\" is not a solution to such a\n> project, and does not say much about the usefulness of the new\n> configuration, so I'd queue it as-is.\n\nYeah, I agree that this patch does not make a judgement on particular\nworkflows, and can stand on its own. I buried a related deep in a\nfootnote in my other email, so let me bring it up again here.\n\nI am not entirely convinced this won't bite somebody who gets a sha1\nfrom some other source, and then wants to run:\n\n git log $some_other_sha1\n\nwho might be quite confused to start a first-parent traversal from\nsomewhere other than the tip of \"master\" or the tip of a topic branch.\n\n-Peff\n"},{"id":"266717","messageId":"20150724073415.GD2111@peff.net","threadId":"39920","inReplyTo":"CA+P7+xoOkTfair3M7HH_TPs9qxBAUR5GodHGDuw8Ux_HaUvvyQ@mail.gmail.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-24T07:34:16Z","receivedAt":"2015-07-24T07:34:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 23, 2015 at 11:07:58PM -0700, Jacob Keller wrote:\n\n> I think some projects definitely benefit from the first-parent setup,\n> and it could be valuable, but I do tend to agree with Junio here that\n> the mess is always helpful. If may be helpful if people's commit\n> messages on that mess are good, but generally those that don't take\n> the time to rebase local work and re-express the commit messages are\n> not going to leave insightful messages the first time. However, have\n> the ability to view history this way is still possibly valuable.\n\nI think a really simple example is something like:\n\n 1. somebody implements as feature. It needs to handle cases a, b, and\n c, but it only handles case a. Therefore it is buggy.\n\n 2. During review, somebody notices case b, and a new commit is made to\n fix it. Nobody notices case c.\n\n 3. The topic is merged.\n\n 4. Much later, somebody notices the system is buggy and hunts in the\n history.\n\nIn a \"clean\" history, the patches from steps 1 and 2 are squashed. While\nreading the history, you see only \"implement feature X\", and no mention\nof the bug and its fix. But even if the person writes a terrible commit\nmessage for step (2), even seeing it pulled out into its own diff shows\nthe exact nature of the already-seen bug, and may make it more obvious\nto realize that case (c) is a problem.\n\nI realize that's kind of vague. Another way to think about it is: in a\nsquashing workflow like git.git, any time you have to turn to the\nmailing list to read the original sequence of re-rolls, you would have\nbeen better off if that information were in git. That's a minority case,\nbut I certainly have turned to it (in some cases, the \"fix\" from our\nstep 2 above actually introduces the new bug, and it's nice to see the\nreasoning that went into it :) ).\n\nNot that I am advocating for git.git to move to such a workflow. I think\non balance the \"clean\" history is nicer to work with. I am only arguing\nthat keeping the messy history is not without value; there are some\ncases where it is nice to have (and we keep it in the list archive,\nwhich is a minor pain to access compared to git).\n\n-Peff\n"},{"id":"266719","messageId":"20150724074028.GE2111@peff.net","threadId":"39920","inReplyTo":"CAGZ79kbY8hAom+voAOourkDy34nwaACQEM-n=f25Qbg0+ha_8g@mail.gmail.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-24T07:40:28Z","receivedAt":"2015-07-24T07:40:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 23, 2015 at 03:14:50PM -0700, Stefan Beller wrote:\n\n> Github pull request messages\n> are similar to cover letters, so you could send a series with a\n> good cover letter, but crappy unfinished patches inside the series.\n> After applying all patches it could be all nice, i.e. compiles, tests, adds the\n> new functionality. It might be just a commit in between which may not even\n> compile. That's my understanding of the messy-workflow.\n\nYeah, that's certainly one messy workflow. There are many levels of\nmessy. But I think fundamentally it comes down to: do you show the steps\nthat went into the result, or do you squash them out into a clean\nhistory?\n\n> Gerrit cannot provide such a workflow easily as it's rather commit\n> centric and not branch centered. So you need to approve each\n> commit on its own and until 2 weeks ago you even needed to submit\n> each commit. (By now Gerrit has learned to submit a full branch, that\n> is you submit a commit and all its ancestors will integrate as well if they\n> were approved.)\n> Previously when the ancestors were not approved the commit would be\n> \"submitted, merge pending\", so it would wait for each commit to be approved\n> and submitted.\n> And because of this commit-centric workflow, the crappy commit in the series\n> is put into spot light.\n\nYeah, I agree that does not sound quite the same as the GitHub flow.\n\n> > So I think this could easily be improved by GitHub (we\n> > have the PR subject and body, after all). It's harder\n> > for a mailing list project like git.git, because Git\n> > never actually sees the subject line. I think it would\n> > require teaching git-am the concept of a patch series.\n> \n> This would be cool I would imagine.\n\nI talked with some GitHub folks about this. One of the challenges is\nthat the PR body is in Markdown, but we'd probably want \"real\" text in\nthe merge commit. One option would be to simply convert\nMarkdown->HTML->Text, which should provide a fairly clean version. It\nwill take some playing with, but I'm going to see what I can do.\n\n> > I don't know offhand what Gerrit merges look like.\n> \n> Let's say it's complicated. Depending on configuration a few things\n> may happen. There are different integration strategies\n> * merge always\n> * merge if necessary (fastforward else)\n> * fastforward only\n> * rebase if necessary (to make it a linear history)\n> * cherrypick (which may add footers like Reviewed-by: to have that\n> information in the git history)\n> \n> I think the \"merge always\" strategy would comfort from this patch.\n\nYeah, I think for anything else it's not a good idea (especially if you\nfast-forward onto master).\n\n> > - we already have merge.ff to default to making extra\n> > merge commits. And if you use GitHub's UI to do the\n> > merge, it uses --no-ff. I don't think we would want\n> > these to become the default, so there's probably nothing\n> > else to be done there.\n> >\n> > Signed-off-by: Jeff King \n> \n> The signoff is better placed above :)\n\nWhoops. Usually I \"format-patch -s\" and then add any notes while\nsending. But the wifi at OSCON was so abysmal that instead I wrote the\nnotes directly into the commit message to send the whole thing later.\nAnd of course format-patch is not smart enough to know that I meant\neverything after the \"---\" as notes. :)\n\n-Peff\n"},{"id":"266720","messageId":"CA+P7+xrmu-waa6OTg4f5Q_gcDbDz3d_5f7W1HfbmMHWyBSe3=A@mail.gmail.com","threadId":"39920","inReplyTo":"20150724073415.GD2111@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-24T07:44:33Z","receivedAt":"2015-07-24T07:44:33Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Jul 24, 2015 at 12:34 AM, Jeff King wrote:\n> On Thu, Jul 23, 2015 at 11:07:58PM -0700, Jacob Keller wrote:\n>\n>> I think some projects definitely benefit from the first-parent setup,\n>> and it could be valuable, but I do tend to agree with Junio here that\n>> the mess is always helpful. If may be helpful if people's commit\n>> messages on that mess are good, but generally those that don't take\n>> the time to rebase local work and re-express the commit messages are\n>> not going to leave insightful messages the first time. However, have\n>> the ability to view history this way is still possibly valuable.\n>\n> I think a really simple example is something like:\n>\n> 1. somebody implements as feature. It needs to handle cases a, b, and\n> c, but it only handles case a. Therefore it is buggy.\n>\n> 2. During review, somebody notices case b, and a new commit is made to\n> fix it. Nobody notices case c.\n>\n> 3. The topic is merged.\n>\n> 4. Much later, somebody notices the system is buggy and hunts in the\n> history.\n>\n> In a \"clean\" history, the patches from steps 1 and 2 are squashed. While\n> reading the history, you see only \"implement feature X\", and no mention\n> of the bug and its fix. But even if the person writes a terrible commit\n> message for step (2), even seeing it pulled out into its own diff shows\n> the exact nature of the already-seen bug, and may make it more obvious\n> to realize that case (c) is a problem.\n>\n> I realize that's kind of vague. Another way to think about it is: in a\n> squashing workflow like git.git, any time you have to turn to the\n> mailing list to read the original sequence of re-rolls, you would have\n> been better off if that information were in git. That's a minority case,\n> but I certainly have turned to it (in some cases, the \"fix\" from our\n> step 2 above actually introduces the new bug, and it's nice to see the\n> reasoning that went into it :) ).\n>\n\nActually this and the GitHub workflow make sense to me. I am concerned\nlike you are about what to do in the case for passing commits.. maybe\njust make the option turn it on only if no arguments are passed (ie:\n\"git log\") and clearly document that it does not enable the behavior\nfor arbitrary commits? That would make it useful without being too\nconfusing nor too magical. Though, it does mean if you do pass a\ncommit and want it on you'd have to specify.. I certainly would find\nit more confusing to have it enabled if I pass an arbitrary commit..\nMaybe not though, I am not 100% sure on that.\n\n> Not that I am advocating for git.git to move to such a workflow. I think\n> on balance the \"clean\" history is nicer to work with. I am only arguing\n> that keeping the messy history is not without value; there are some\n> cases where it is nice to have (and we keep it in the list archive,\n> which is a minor pain to access compared to git).\n>\n> -Peff\n\nFor projects which make this easier (pull request style) I think this\nis making more sense. I'm not sure exactly whether it would be best to\nonly enable via \"git log\" with no parameters, but it does seem weird\nto turn this on and then accidentally get the behavior on a random\ncommit log sequence.... Maybe it's not a huge deal either way though..\n\nRegards,\nJake\n"},{"id":"266721","messageId":"CA+P7+xo11PwE+L3zCy8Nw=xt+ZWKadd_-ra7C-QvNV80B6dCGw@mail.gmail.com","threadId":"39920","inReplyTo":"20150724074028.GE2111@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-24T07:46:57Z","receivedAt":"2015-07-24T07:46:57Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Jul 24, 2015 at 12:40 AM, Jeff King wrote:\n> Whoops. Usually I \"format-patch -s\" and then add any notes while\n> sending. But the wifi at OSCON was so abysmal that instead I wrote the\n> notes directly into the commit message to send the whole thing later.\n> And of course format-patch is not smart enough to know that I meant\n> everything after the \"---\" as notes. :)\n>\n> -Peff\n\nKind of a side track but...\n\nI think it's up to the caller of git-am to use \"--scissors\" to cut the\nlog? But maybe we could add an option to git-format patch which\nformats and cuts via scissors as it generates the message? Not sure\nthe best way to interpret this, but I know I've had trouble where I\nwrote some notes into an email and lost it because I killed the email\nfor some other edit. Keeping them inside my local commits before\nsending out email would be handy.. hmmmm\n\nRegards,\nJake\n"},{"id":"266722","messageId":"20150724081753.GA29451@peff.net","threadId":"39920","inReplyTo":"CA+P7+xo11PwE+L3zCy8Nw=xt+ZWKadd_-ra7C-QvNV80B6dCGw@mail.gmail.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-24T08:17:54Z","receivedAt":"2015-07-24T08:17:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 24, 2015 at 12:46:57AM -0700, Jacob Keller wrote:\n\n> On Fri, Jul 24, 2015 at 12:40 AM, Jeff King wrote:\n> > Whoops. Usually I \"format-patch -s\" and then add any notes while\n> > sending. But the wifi at OSCON was so abysmal that instead I wrote the\n> > notes directly into the commit message to send the whole thing later.\n> > And of course format-patch is not smart enough to know that I meant\n> > everything after the \"---\" as notes. :)\n> \n> Kind of a side track but...\n> \n> I think it's up to the caller of git-am to use \"--scissors\" to cut the\n> log? But maybe we could add an option to git-format patch which\n> formats and cuts via scissors as it generates the message? Not sure\n> the best way to interpret this, but I know I've had trouble where I\n> wrote some notes into an email and lost it because I killed the email\n> for some other edit. Keeping them inside my local commits before\n> sending out email would be handy.. hmmmm\n\nThe \"---\" is orthogonal to \"--scissors\". With \"--scissors\", the full\nformat of the body is:\n\n some notes or cover letter\n\n -- >8 --\n the actual commit message\n\n ---\n more notes\n\n diff --git ...etc...\n\nSo here I was trying to use the \"---\" to add notes at the end (not\nbecause --scissors is not used consistently, but because I wanted the\nreader to see them after reading the commit message). So you could keep\nnotes in the commit message by writing:\n\n my notes here\n\n -- >8 --\n the real commit message\n\nand then \"format-patch -s\" just works, because it is munging the end.\nBut if you want to be able to add commit notes at the end, you need\nformat-patch to realize that any trailers should go before the \"---\"\n(i.e., to realize that the \"---\" is syntactically significant, and not\njust part of your message).\n\nAnother option would be to teach git-commit to split the \"---\" from the\ncommit message itself, and put the bits after it into git-notes (and\nthen format-patch already knows how to handle that). I had a patch\nseries to do that long ago, but I found that I never used it (I usually\n_do_ type my notes in the mailer as I'm sending), so I never seriously\npushed for inclusion. I might be able to dig it out of the archive if\nyou're interested.\n\n-Peff\n"},{"id":"266729","messageId":"xmqqd1zhoave.fsf@gitster.dls.corp.google.com","threadId":"39920","inReplyTo":"20150724073415.GD2111@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T15:04:21Z","receivedAt":"2015-07-24T15:04:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think a really simple example is something like:\n>\n> 1. somebody implements as feature. It needs to handle cases a, b, and\n> c, but it only handles case a. Therefore it is buggy.\n>\n> 2. During review, somebody notices case b, and a new commit is made to\n> fix it. Nobody notices case c.\n>\n> 3. The topic is merged.\n>\n> 4. Much later, somebody notices the system is buggy and hunts in the\n> history.\n>\n> In a \"clean\" history, the patches from steps 1 and 2 are squashed. While\n> reading the history, you see only \"implement feature X\", and no mention\n> of the bug and its fix. But even if the person writes a terrible commit\n> message for step (2), even seeing it pulled out into its own diff shows\n> the exact nature of the already-seen bug, and may make it more obvious\n> to realize that case (c) is a problem.\n>\n> I realize that's kind of vague. Another way to think about it is: in a\n> squashing workflow like git.git, any time you have to turn to the\n> mailing list to read the original sequence of re-rolls, you would have\n> been better off if that information were in git. That's a minority case,\n> but I certainly have turned to it (in some cases, the \"fix\" from our\n> step 2 above actually introduces the new bug, and it's nice to see the\n> reasoning that went into it :) ).\n>\n> Not that I am advocating for git.git to move to such a workflow. \n\nI actually do not think the above is quite true. In our kind of\n\"clean history, we do not squash 1 & 2. See Paul's \"rewrite am in\nC\" series, for example, that starts from a \"buggy\" (in the sense\nthat it does almost nothing in the beginning and then gradually\nbuilds on).\n\nInstead, even somebody did not have foresight to realize 'b' when\nshe adds code to handle 'a', we would make sure the solution for 'a'\nis sufficiently clearly described in commit #1.\n\nIn other words, without #1 and #2 squashed together, the history you\npresented in your example _could_ be already \"clean\". It just boils\ndown to the quality of individual commit.\n"},{"id":"266730","messageId":"xmqq8ua5oapm.fsf@gitster.dls.corp.google.com","threadId":"39920","inReplyTo":"20150724072325.GC2111@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T15:07:49Z","receivedAt":"2015-07-24T15:07:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Jul 23, 2015 at 03:46:33PM -0700, Junio C Hamano wrote:\n>\n>> Admittedly, that\n>> merely is saying that \"--first-parent\" is not a solution to such a\n>> project, and does not say much about the usefulness of the new\n>> configuration, so I'd queue it as-is.\n>\n> Yeah, I agree that this patch does not make a judgement on particular\n> workflows, and can stand on its own. I buried a related deep in a\n> footnote in my other email, so let me bring it up again here.\n>\n> I am not entirely convinced this won't bite somebody who gets a sha1\n> from some other source, and then wants to run:\n>\n> git log $some_other_sha1\n>\n> who might be quite confused to start a first-parent traversal from\n> somewhere other than the tip of \"master\" or the tip of a topic branch.\n\nYeah, you actually convinced me reasonably well that it would\nhappen. I'd never use it myself. If people want to shoot\nthemselves in the foot, be my guest ;-)\n\nPerhaps we should drop this, and give a shorter synonym to the\noption?\n.\n"},{"id":"266731","messageId":"xmqq4mkto9lg.fsf@gitster.dls.corp.google.com","threadId":"39920","inReplyTo":"20150724074028.GE2111@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T15:31:55Z","receivedAt":"2015-07-24T15:31:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Whoops. Usually I \"format-patch -s\" and then add any notes while\n> sending. But the wifi at OSCON was so abysmal that instead I wrote the\n> notes directly into the commit message to send the whole thing later.\n> And of course format-patch is not smart enough to know that I meant\n> everything after the \"---\" as notes. :)\n\nI think in the cycle we merged Couder's trailer stuff we updated the\nhelper functions to locate where the S-o-b should go in an existing\nmessage and consolidated (or, at least \"talked about consolidating\")\nthem into a single helper. I do not think we wrote any special case\nfor \"a line with three-dashes and nothing else on it\" when we did\nso, but that function would be the logical place to do so.\n"},{"id":"266736","messageId":"20150724181322.GB17730@peff.net","threadId":"39920","inReplyTo":"xmqqd1zhoave.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-24T18:13:22Z","receivedAt":"2015-07-24T18:13:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 24, 2015 at 08:04:21AM -0700, Junio C Hamano wrote:\n\n> > I think a really simple example is something like:\n> >\n> > 1. somebody implements as feature. It needs to handle cases a, b, and\n> > c, but it only handles case a. Therefore it is buggy.\n> >\n> > 2. During review, somebody notices case b, and a new commit is made to\n> > fix it. Nobody notices case c.\n> >\n> > 3. The topic is merged.\n> >\n> > 4. Much later, somebody notices the system is buggy and hunts in the\n> > history.\n> [...]\n> \n> I actually do not think the above is quite true. In our kind of\n> \"clean history, we do not squash 1 & 2. See Paul's \"rewrite am in\n> C\" series, for example, that starts from a \"buggy\" (in the sense\n> that it does almost nothing in the beginning and then gradually\n> builds on).\n> \n> Instead, even somebody did not have foresight to realize 'b' when\n> she adds code to handle 'a', we would make sure the solution for 'a'\n> is sufficiently clearly described in commit #1.\n\nSometimes. There's definitely human wisdom going into the decision to\nsquash or make a new commit, which is a good thing. Here's a\ncounter-example to the am series. When I wrote the object-freshening\ncode, I accidentally inverted the test for checking whether packs were\nfresh. This was noticed in review, and I corrected it by inverting the\nis_fresh function. But in doing so, I introduced a new bug, where the\ntest for loose objects was inverted.\n\nThe original fix was squashed in the re-roll, but much later we noticed\nand diagnosed that new bug. It was very valuable to me to read the\nmailing list archive to see what happened, because the fact that there\n_was_ a bug fix was lost in the clean history.\n\n-Peff\n"},{"id":"266765","messageId":"20150725013634.GA7269@peff.net","threadId":"39920","inReplyTo":"xmqq4mkto9lg.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-25T01:36:36Z","receivedAt":"2015-07-25T01:36:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 24, 2015 at 08:31:55AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Whoops. Usually I \"format-patch -s\" and then add any notes while\n> > sending. But the wifi at OSCON was so abysmal that instead I wrote the\n> > notes directly into the commit message to send the whole thing later.\n> > And of course format-patch is not smart enough to know that I meant\n> > everything after the \"---\" as notes. :)\n> \n> I think in the cycle we merged Couder's trailer stuff we updated the\n> helper functions to locate where the S-o-b should go in an existing\n> message and consolidated (or, at least \"talked about consolidating\")\n> them into a single helper. I do not think we wrote any special case\n> for \"a line with three-dashes and nothing else on it\" when we did\n> so, but that function would be the logical place to do so.\n\nYeah, it nicely has the concept of \"ignore this footer\". But we would\nwant it only to kick in when doing emails (where the \"---\" is\nsyntactically significant), I would think. So something like the patch\nbelow (no commit message because I'm in an airport right now; I'll add\ntests and repost in the next day or two).\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 7b1b57a..8a9c35b 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -688,8 +688,16 @@ void show_log(struct rev_info *opt)\n \t\tctx.from_ident = &opt->from_ident;\n \tpretty_print_commit(&ctx, commit, &msgbuf);\n \n-\tif (opt->add_signoff)\n-\t\tappend_signoff(&msgbuf, 0, APPEND_SIGNOFF_DEDUP);\n+\tif (opt->add_signoff) {\n+\t\tint ignore = 0;\n+\t\tif (ctx.fmt == CMIT_FMT_EMAIL) {\n+\t\t\tconst char *dashes = strstr(msgbuf.buf, \"---\\n\");\n+\t\t\tif (dashes &&\n+\t\t\t (dashes == msgbuf.buf || dashes[-1] == '\\n'))\n+\t\t\t\tignore = msgbuf.len - (dashes - msgbuf.buf);\n+\t\t}\n+\t\tappend_signoff(&msgbuf, ignore, APPEND_SIGNOFF_DEDUP);\n+\t}\n \n \tif ((ctx.fmt != CMIT_FMT_USERFORMAT) &&\n \t ctx.notes_message && *ctx.notes_message) {\n"},{"id":"266766","messageId":"20150725014722.GA17830@peff.net","threadId":"39920","inReplyTo":"20150725013634.GA7269@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-25T01:47:23Z","receivedAt":"2015-07-25T01:47:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 24, 2015 at 06:36:34PM -0700, Jeff King wrote:\n\n> > I think in the cycle we merged Couder's trailer stuff we updated the\n> > helper functions to locate where the S-o-b should go in an existing\n> > message and consolidated (or, at least \"talked about consolidating\")\n> > them into a single helper. I do not think we wrote any special case\n> > for \"a line with three-dashes and nothing else on it\" when we did\n> > so, but that function would be the logical place to do so.\n> \n> Yeah, it nicely has the concept of \"ignore this footer\". But we would\n> want it only to kick in when doing emails (where the \"---\" is\n> syntactically significant), I would think. So something like the patch\n> below (no commit message because I'm in an airport right now; I'll add\n> tests and repost in the next day or two).\n\nThis works for \"format-patch -s\". But I guess that leaves open the\nquestion of \"commit --signoff\". It should not matter when making a\ncommit new (after all, you have not yet had a chance to put the \"---\"\nin). But something like \"git commit --amend --signoff\" might want to\nhandle it. Of course we have no idea if any \"---\" we find there is meant\nto be an email notes-separator by the user, or if they happened to use\n\"---\" for something else[1] (which is a bad idea if you have an emailed\npatches workflow, but many people do not). So it's a bit riskier.\n\n-Peff\n\n[1] While reading the old \"git commit --notes\" thread recently, Johan\n Herland gave a plausible confusing example:\n\n What\n ----\n\n A commit message using markdown-like formatting conventions.\n\n Why\n ---\n\n To show that \"---\" can be part of a commit message. :)\n"},{"id":"266767","messageId":"20150725020526.GA8948@peff.net","threadId":"39920","inReplyTo":"xmqq8ua5oapm.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-25T02:05:27Z","receivedAt":"2015-07-25T02:05:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 24, 2015 at 08:07:49AM -0700, Junio C Hamano wrote:\n\n> > I am not entirely convinced this won't bite somebody who gets a sha1\n> > from some other source, and then wants to run:\n> >\n> > git log $some_other_sha1\n> >\n> > who might be quite confused to start a first-parent traversal from\n> > somewhere other than the tip of \"master\" or the tip of a topic branch.\n> \n> Yeah, you actually convinced me reasonably well that it would\n> happen. I'd never use it myself. If people want to shoot\n> themselves in the foot, be my guest ;-)\n> \n> Perhaps we should drop this, and give a shorter synonym to the\n> option?\n\nI'm still on the fence to have the config kick in only for HEAD.\nSomething like (on top of my other patch, and the tests would still need\nadjusted):\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e9c3763..f2b6a21 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1802,9 +1802,11 @@ log.mailmap::\n \tIf true, makes linkgit:git-log[1], linkgit:git-show[1], and\n \tlinkgit:git-whatchanged[1] assume `--use-mailmap`.\n \n-log.firstparent::\n-\tIf true, linkgit:git-log[1] will default to `--first-parent`;\n-\tcan be overridden by supplying `--no-first-parent`.\n+log.defaultImpliesFirstParent::\n+\tIf true, linkgit:git-log[1] will default to `--first-parent`\n+\twhen showing the default ref (i.e., if you run only `git\n+\tlog` to show `HEAD`, but not `git log $sha1`). Can be overridden\n+\tby supplying `--no-first-parent`.\n \n mailinfo.scissors::\n \tIf true, makes linkgit:git-mailinfo[1] (and therefore\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 3e9b034..2bdb3fc 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -31,7 +31,7 @@ static const char *default_date_mode = NULL;\n \n static int default_abbrev_commit;\n static int default_show_root = 1;\n-static int default_first_parent;\n+static int default_implies_first_parent;\n static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config;\n@@ -110,7 +110,6 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n-\trev->first_parent_only = default_first_parent;\n \tDIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n \n \tif (default_date_mode)\n@@ -398,8 +397,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tuse_mailmap_config = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(var, \"log.firstparent\")) {\n-\t\tdefault_first_parent = git_config_bool(var, value);\n+\tif (!strcmp(var, \"log.defaultimpliesfirstparent\")) {\n+\t\tdefault_implies_first_parent = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n \n@@ -504,6 +503,8 @@ static int show_tree_object(const unsigned char *sha1,\n \n static void show_rev_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)\n {\n+\tif (default_implies_first_parent && !rev->pending.nr)\n+\t\trev->first_parent_only = 1;\n \tif (rev->ignore_merges) {\n \t\t/* There was no \"-m\" on the command line */\n \t\trev->ignore_merges = 0;\n\n\nIt feels somewhat magical, but at least the config option name makes it\npainfully clear exactly when it would kick in. I dunno. I am happy\nenough for myself to just run \"--first-parent\" when that is what I want\nto see. Giving it a shorter name would not help much, I think. It is not\nthe number of characters, but the fact that most people do not _know_\nthat --first-parent exists in the first place, or that it would be\nuseful in this case. I hoped with a config option it might become\nsomething projects could recommend to their users[1] if the project has a\nmatching workflow. But maybe we could also rely on those same projects\nto educate their users.\n\n-Peff\n\n[1] And if not an official recommendation from a project, this is the\n sort of \"tips and tricks\" information that may spread informally. But in\n theory so could knowledge of --first-parent.\n"},{"id":"266770","messageId":"xmqqlhe4jgui.fsf@gitster.dls.corp.google.com","threadId":"39920","inReplyTo":"20150725014722.GA17830@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-25T17:18:45Z","receivedAt":"2015-07-25T17:18:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This works for \"format-patch -s\". But I guess that leaves open the\n> question of \"commit --signoff\". It should not matter when making a\n> commit new (after all, you have not yet had a chance to put the \"---\"\n> in). But something like \"git commit --amend --signoff\" might want to\n> handle it. Of course we have no idea if any \"---\" we find there is meant\n> to be an email notes-separator by the user, or if they happened to use\n> \"---\" for something else[1] (which is a bad idea if you have an emailed\n> patches workflow, but many people do not). So it's a bit riskier.\n>\n> -Peff\n>\n> [1] While reading the old \"git commit --notes\" thread recently, Johan\n> Herland gave a plausible confusing example:\n>\n> ...\n> Why\n> ---\n>\n> To show that \"---\" can be part of a commit message. :)\n\nThat is all true, but such a commit already is problematic when used\nas an input to \"am\", regardless of where the sign-off goes. With or\nwithout our change, the intended explanation of \"why\" part will be\nmissing from the resulting commit at the receiving end. The only\ndifference your change makes is that the resulting truncated log\nmessage at least will have a sign-off, albeit at a place that the\nuser did not intend to put it.\n\nWe could invent a new and more prominent delimiter, teach\n\"format-patch\" to add that between the log and patch if and only if\nthe log has a three-dashes line in it (with an option to override\nthat \"if and only if\" default), and teach \"mailsplit\" to pay\nattention to it. People who are relying on the fact that a\nthree-dashes line in the local log message will be stripped off at\nthe receiving end have to pass that \"The commit has three-dash in it\nas a cut-mark on purpose; don't add that prominent delimiter\" option\nwhen formatting their patches out for submission.\n\nBut I somehow think it is not worth the effort. It is fairly well\nestablished that three-dash lines are cut marks and Johan's example\nlog message above deliberately violates only to spite itself. My\nknee-jerk advice is that people can just rephrase s/Why/Reason/ and\nbe done with it.\n\nI dunno.\n"},{"id":"266771","messageId":"xmqqh9osjfsu.fsf@gitster.dls.corp.google.com","threadId":"39920","inReplyTo":"20150725020526.GA8948@peff.net","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-25T17:41:21Z","receivedAt":"2015-07-25T17:41:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Jul 24, 2015 at 08:07:49AM -0700, Junio C Hamano wrote:\n>\n>> Yeah, you actually convinced me reasonably well that it would\n>> happen. I'd never use it myself. If people want to shoot\n>> themselves in the foot, be my guest ;-)\n>> \n>> Perhaps we should drop this, and give a shorter synonym to the\n>> option?\n>\n> I'm still on the fence to have the config kick in only for HEAD.\n\nHmm, I cannot tell offhand if the confusion factor is worth it (I\ndidn't say \"I don't think it is worth it\"). I'd imagine that one\ncommon thing to want is to get an overview of what has happened\nupstream since the topic one is currently working on forked from it,\ni.e. \"log --first-parent ..master\", for an individual contributor,\nand nother is to see what has happened since the last stable point,\ni.e. \"log --first-parent origin..\" or \"log --first-parent v1.0..\",\nfor an integrator. Neither is covered by the \"fp when implied HEAD\".\n\nWhen I am playing an individual contributor, I often want to see my\nprogress with \"log -9\" or something, only because \"log origin..\" is\nlonger to type and I know my topic is not that long as nine commits.\nI guess implied first-parent would not hurt that much in that case,\nsimply because I do not expect too many merges on a topic, but it\nfeels wrong to default to first-parent traversal there.\n\nSo...\n\n> It feels somewhat magical, but at least the config option name makes it\n> painfully clear exactly when it would kick in. I dunno. I am happy\n> enough for myself to just run \"--first-parent\" when that is what I want\n> to see. Giving it a shorter name would not help much, I think.\n\nI admit I may be minority, but two common things I do everyday are\n\"log --first-parent v2.5.0-rc0..\" and \"log --first-parent master..pu\";\nI could certainly use a short-hand there.\n\nI already have alias for it, so this is not to help me personally,\nbut \"log -FO\" to trigger first-parent one-line would make the alias\nunnecessary.\n\n> It is not\n> the number of characters, but the fact that most people do not _know_\n> that --first-parent exists in the first place, or that it would be\n> useful in this case.\n\nThat is a separate \"education\" problem. My suggestion was more\nabout \"I know there is a way already, but it is cumbersome to type\".\n\n> I hoped with a config option it might become\n> something projects could recommend to their users[1] if the project has a\n> matching workflow. But maybe we could also rely on those same projects\n> to educate their users.\n\nThey could educate their users to use \"log -F\" just like they could\ntell them to say \"config log.firstparent true\", I would think. The\nrisk of the latter is that those who blindly follow the config path\nwithout understanding what is going on will not even realize that\nthe problem is that they told Git to only follow the first-parent\npath, when they do want to see commits on the side branch, let alone\ndiscovering how to countermand it from the command line one shot.\n\nAn instruction to use an extra option, on the other hand, makes it\nclear that there is a non-default thing going on, which is more\ndiscoverable: \"perhaps I can run it without -F?\"\n"},{"id":"266776","messageId":"CA+P7+xq7cL2M+EUQ6gafbn_qTLzz1jgcAHmfWccsrKUvpXK52g@mail.gmail.com","threadId":"39920","inReplyTo":"xmqqh9osjfsu.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-25T22:41:13Z","receivedAt":"2015-07-25T22:41:13Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sat, Jul 25, 2015 at 10:41 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> On Fri, Jul 24, 2015 at 08:07:49AM -0700, Junio C Hamano wrote:\n>>\n>>> Yeah, you actually convinced me reasonably well that it would\n>>> happen. I'd never use it myself. If people want to shoot\n>>> themselves in the foot, be my guest ;-)\n>>>\n>>> Perhaps we should drop this, and give a shorter synonym to the\n>>> option?\n>>\n>> I'm still on the fence to have the config kick in only for HEAD.\n>\n> Hmm, I cannot tell offhand if the confusion factor is worth it (I\n> didn't say \"I don't think it is worth it\"). I'd imagine that one\n> common thing to want is to get an overview of what has happened\n> upstream since the topic one is currently working on forked from it,\n> i.e. \"log --first-parent ..master\", for an individual contributor,\n> and nother is to see what has happened since the last stable point,\n> i.e. \"log --first-parent origin..\" or \"log --first-parent v1.0..\",\n> for an integrator. Neither is covered by the \"fp when implied HEAD\".\n>\n> When I am playing an individual contributor, I often want to see my\n> progress with \"log -9\" or something, only because \"log origin..\" is\n> longer to type and I know my topic is not that long as nine commits.\n> I guess implied first-parent would not hurt that much in that case,\n> simply because I do not expect too many merges on a topic, but it\n> feels wrong to default to first-parent traversal there.\n>\n> So...\n>\n>> It feels somewhat magical, but at least the config option name makes it\n>> painfully clear exactly when it would kick in. I dunno. I am happy\n>> enough for myself to just run \"--first-parent\" when that is what I want\n>> to see. Giving it a shorter name would not help much, I think.\n>\n> I admit I may be minority, but two common things I do everyday are\n> \"log --first-parent v2.5.0-rc0..\" and \"log --first-parent master..pu\";\n> I could certainly use a short-hand there.\n>\n> I already have alias for it, so this is not to help me personally,\n> but \"log -FO\" to trigger first-parent one-line would make the alias\n> unnecessary.\n>\n>> It is not\n>> the number of characters, but the fact that most people do not _know_\n>> that --first-parent exists in the first place, or that it would be\n>> useful in this case.\n>\n> That is a separate \"education\" problem. My suggestion was more\n> about \"I know there is a way already, but it is cumbersome to type\".\n>\n>> I hoped with a config option it might become\n>> something projects could recommend to their users[1] if the project has a\n>> matching workflow. But maybe we could also rely on those same projects\n>> to educate their users.\n>\n> They could educate their users to use \"log -F\" just like they could\n> tell them to say \"config log.firstparent true\", I would think. The\n> risk of the latter is that those who blindly follow the config path\n> without understanding what is going on will not even realize that\n> the problem is that they told Git to only follow the first-parent\n> path, when they do want to see commits on the side branch, let alone\n> discovering how to countermand it from the command line one shot.\n>\n> An instruction to use an extra option, on the other hand, makes it\n> clear that there is a non-default thing going on, which is more\n> discoverable: \"perhaps I can run it without -F?\"\n\nI like the idea of a shorthand more than the configuration simply\nbecause it doesn't make sense (to me) to always be enabled, and it is\nmuch easier to do so without realizing that you've done it.\n\nRegards,\nJake\n"},{"id":"266801","messageId":"20150727044352.GA27465@peff.net","threadId":"39920","inReplyTo":"xmqqlhe4jgui.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-27T04:43:52Z","receivedAt":"2015-07-27T04:43:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 25, 2015 at 10:18:45AM -0700, Junio C Hamano wrote:\n\n> > [1] While reading the old \"git commit --notes\" thread recently, Johan\n> > Herland gave a plausible confusing example:\n> >\n> > ...\n> > Why\n> > ---\n> >\n> > To show that \"---\" can be part of a commit message. :)\n> \n> That is all true, but such a commit already is problematic when used\n> as an input to \"am\", regardless of where the sign-off goes.\n\nRight, and that is why I think there is no problem at all with treating\n\"---\" specially as part of format-patch. What I was trying to say here\nis that doing it for \"git commit\" is less obviously OK. Many people do\nnot use \"am\" at all, and are otherwise fine with a message like the one\nabove (tools like rebase used to eat their message, but I think that was\nfixed long ago).\n\n> We could invent a new and more prominent delimiter, teach\n> \"format-patch\" to add that between the log and patch if and only if\n> the log has a three-dashes line in it (with an option to override\n> that \"if and only if\" default), and teach \"mailsplit\" to pay\n> attention to it. People who are relying on the fact that a\n> three-dashes line in the local log message will be stripped off at\n> the receiving end have to pass that \"The commit has three-dash in it\n> as a cut-mark on purpose; don't add that prominent delimiter\" option\n> when formatting their patches out for submission.\n> \n> But I somehow think it is not worth the effort. It is fairly well\n> established that three-dash lines are cut marks and Johan's example\n> log message above deliberately violates only to spite itself. My\n> knee-jerk advice is that people can just rephrase s/Why/Reason/ and\n> be done with it.\n\nYeah, I agree it is not worth the effort. Three-dash is a totally fine\nmicro-format for email messages, and I do not see anybody complaining\nabout it. I just think that people who do not use \"am\" should not have\nto care about it.\n\n-Peff\n"},{"id":"266802","messageId":"20150727045533.GB27465@peff.net","threadId":"39920","inReplyTo":"xmqqh9osjfsu.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] log: add log.firstparent option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-27T04:55:33Z","receivedAt":"2015-07-27T04:55:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 25, 2015 at 10:41:21AM -0700, Junio C Hamano wrote:\n\n> > I'm still on the fence to have the config kick in only for HEAD.\n> \n> Hmm, I cannot tell offhand if the confusion factor is worth it (I\n> didn't say \"I don't think it is worth it\").\n> [...]\n\nI've snipped most of your response because it all seemed pretty\nreasonable to me.\n\nAt this point I think I am of the opinion that the decision to use\n--first-parent is sufficiently nuanced that it the config option is not\nreally a \"drop-in\" solution for people, even if their projects follow\nthe matching workflow. Like you, I am not really _against_ it, but since\nnobody in this thread is saying \"yes, I would turn that on\", that may be\na sign. The patch is out there on the list, and I'd encourage people who\nthink it might be useful to apply the patch and report back in a few\nweeks or months if they find it useful.\n\nWe _could_ merge the patch to make that experimentation easier for\nusers. The downside is we will be stuck supporting the log.firstParent\noption forever, but I don't think it is actively _wrong_ to have. Just\npossibly useless.\n\nAnd poor Josh, who so nicely came to the Git table at OSCON and talked\nto me about his project's workflow, has now had to put up with a slew of\nemails and no applied patch. :)\n\nBut maybe this discussion is of some use; it has not been fruitless, as\nI think the best answer so far is \"encourage awareness and appropriate\nuse of --first-parent\".\n\n> I admit I may be minority, but two common things I do everyday are\n> \"log --first-parent v2.5.0-rc0..\" and \"log --first-parent master..pu\";\n> I could certainly use a short-hand there.\n> \n> I already have alias for it, so this is not to help me personally,\n> but \"log -FO\" to trigger first-parent one-line would make the alias\n> unnecessary.\n\nI do not have an alias, but I spell it \"--fir\". :)\n\n-Peff\n"}]}