{"thread":{"id":"37289","subject":"[PATCH] Documentation/config.txt: change \"pull.rebase\" description in favor of safer 'preserve' option.","startedAt":"2014-08-05T10:58:29Z","lastAt":"2014-08-06T10:29:04Z","messageCount":3,"participants":["Sergey Organov","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"247271","messageId":"87bnrzxhrm.fsf@osv.gnss.ru","threadId":"37289","inReplyTo":null,"subject":"[PATCH] Documentation/config.txt: change \"pull.rebase\" description in favor of safer 'preserve' option.","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2014-08-05T10:58:29Z","receivedAt":"2014-08-05T10:58:29Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Previous description implicitly favored 'true' value for \"pull.rebase\"\nand \"pull..rebase\" options, when for some workflows 'preserve'\nis the right choice, and for others it hardly makes any difference.\nTherefore, 'preserve' should be preferred in general, unless the user\nknows exactly what she is doing.\n\nSigned-off-by: Sergey Organov \n---\n Documentation/config.txt | 26 ++++++++++++--------------\n 1 file changed, 12 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c55c22a..30f28d9 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -772,14 +772,13 @@ branch..mergeoptions::\n \tsupported.\n \n branch..rebase::\n-\tWhen true, rebase the branch on top of the fetched branch,\n-\tinstead of merging the default branch from the default remote when\n-\t\"git pull\" is run. See \"pull.rebase\" for doing this in a non\n-\tbranch-specific manner.\n+\tWhen `preserve` or `true`, rebase the branch on top of the\n+\tfetched branch, instead of merging the default branch from the\n+\tdefault remote when 'git pull' is run. See \"pull.rebase\" for\n+\tdoing this in a non branch-specific manner.\n +\n-\tWhen preserve, also pass `--preserve-merges` along to 'git rebase'\n-\tso that locally committed merge commits will not be flattened\n-\tby running 'git pull'.\n+\tWhen `true`, do not pass `--preserve-merges` to 'git rebase', so\n+\tthat local merge commits will be flattened by running 'git pull'.\n +\n *NOTE*: this is a possibly dangerous operation; do *not* use\n it unless you understand the implications (see linkgit:git-rebase[1]\n@@ -1948,14 +1947,13 @@ pull.ff::\n \tcommand line).\n \n pull.rebase::\n-\tWhen true, rebase branches on top of the fetched branch, instead\n-\tof merging the default branch from the default remote when \"git\n-\tpull\" is run. See \"branch..rebase\" for setting this on a\n-\tper-branch basis.\n+\tWhen `preserve` or `true`, rebase branches on top of the\n+\tfetched branch, instead of merging the default branch from the\n+\tdefault remote when 'git pull' is run. See\n+\t\"branch..rebase\" for setting this on a per-branch basis.\n +\n-\tWhen preserve, also pass `--preserve-merges` along to 'git rebase'\n-\tso that locally committed merge commits will not be flattened\n-\tby running 'git pull'.\n+\tWhen `true`, do not pass `--preserve-merges` to 'git rebase', so\n+\tthat local merge commits will be flattened by running 'git pull'.\n +\n *NOTE*: this is a possibly dangerous operation; do *not* use\n it unless you understand the implications (see linkgit:git-rebase[1]\n-- \n1.9.3\n"},{"id":"247303","messageId":"xmqqa97io9ef.fsf@gitster.dls.corp.google.com","threadId":"37289","inReplyTo":"87bnrzxhrm.fsf@osv.gnss.ru","subject":"Re: [PATCH] Documentation/config.txt: change \"pull.rebase\" description in favor of safer 'preserve' option.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-05T22:32:08Z","receivedAt":"2014-08-05T22:32:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov writes:\n\n> Previous description implicitly favored 'true' value for \"pull.rebase\"\n> and \"pull..rebase\" options,\n\nI do not share that impression. It is true that 'true' is described\nfirst before 'preserve', which can be argued that it is being\nimplicitly favoured, but we have to pick one over the other when\ndescribing two things, so I do not see it as a strong preference.\n\n> when for some workflows 'preserve'\n> is the right choice, and for others it hardly makes any difference.\n> Therefore, 'preserve' should be preferred in general, unless the user\n> knows exactly what she is doing.\n\nI doubt we saw any such conclusion in the recent thread that\ndiscussed this, other than your repeating that statement---and I've\nlearned to tell repeated voices and chorus apart over time ;-).\n\nOne approach is more suitable for some workflows while being\ninappropriate for others and that goes for both variants. A\ndownside of flattening is that it gives a wrong result when the user\nwants to keep merges. Two downsides of preserving are that it gives\na wrong result when the user wants to flatten, and it tends to be\nslower.\n\nDuring that discussion, you seemed to assume that those who want a\nflattened end result would never merge locally; I do not think that\nis true. Having your own merges on a branch that you would want to\nrebase to flatten is not unusual.\n\nYou may employ a workflow to build on separate topic branches while\ndeveloping, merge the topics among them that are ready into your\nlocal 'master' to be used personally, and after using it from your\nlocal 'master' for a while to make sure it is solid, you may decide\nto make it final by publishing it to the outside world, and at that\npoint you would want to flatten the history on top of the latest\nupstream before pushing. That's not anything advanced that takes \"\nthe user knows exactly what she is doing.\"\n"},{"id":"247316","messageId":"87bnrxzzbj.fsf@osv.gnss.ru","threadId":"37289","inReplyTo":"xmqqa97io9ef.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Documentation/config.txt: change \"pull.rebase\" description in favor of safer 'preserve' option.","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2014-08-06T10:29:04Z","receivedAt":"2014-08-06T10:29:04Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano writes:\n> Sergey Organov writes:\n>\n>> Previous description implicitly favored 'true' value for \"pull.rebase\"\n>> and \"pull..rebase\" options,\n>\n> I do not share that impression. It is true that 'true' is described\n> first before 'preserve', which can be argued that it is being\n> implicitly favoured, but we have to pick one over the other when\n> describing two things, so I do not see it as a strong preference.\n\nI didn't say it's a strong preference, and I consider my patch to be\nminor documenation improvement that may save from major trouble.\n\nI still suggest to peek 'preserve' over 'true' in the description, an\nopposite to what is there right now. I believe it'd help me to avoid the\nmistake I did setting pull.merge to true, by stopping reading when I saw\nthat \"pull.rebase\" should be set to 'true' to rebase instead of merge.\n\nCouldn't you please just read the new description and tell what's wrong\nwith it? Besides, it puts less favor on one mode over another than the\noriginal.\n\nAm I right that 'true' for \"pull.rebase\" historically precedes\n'preserve'? This is the only cause of current wording in documentation I\ncan imagine.\n\n>> when for some workflows 'preserve' is the right choice, and for\n>> others it hardly makes any difference. Therefore, 'preserve' should\n>> be preferred in general, unless the user knows exactly what she is\n>> doing.\n>\n> I doubt we saw any such conclusion in the recent thread that\n> discussed this, other than your repeating that statement---and I've\n> learned to tell repeated voices and chorus apart over time ;-).\n\nI've repeated request to tell me if somebody has any evidence 'preserve'\nwould break their workflow, and nobody provided one. I even hoped you\nwould say something, but it didn't happen till I suggested the patch ;-)\n\nOn the other hand, I did suffer from 'true' setting, and that seems to\nbe more common, as was shown by the problem of another git user soon\nafter I explained mine. Moreover, the order of \"suffer\" is worse with\n'true' than it ever could be with 'preserve', see below.\n\n> One approach is more suitable for some workflows while being\n> inappropriate for others and that goes for both variants. A downside\n> of flattening is that it gives a wrong result when the user wants to\n> keep merges. Two downsides of preserving are that it gives a wrong\n> result when the user wants to flatten, and it tends to be slower.\n\nI understand this, but it does look like most of those who care about\nentirely flat history don't do merges in the first place. At least I\ndidn't do it for my simple changes for 'git', so I was not aware how\ndangerous the 'true' setting for \"pull.rebase\" I was using is for my own\nworkflows, when I started using git internally.\n\nI'd also argue that if they do the merges themselves, they'd better do\nflattening themselves as well (by \"git rebase\"), rather than (ab)using\npull.rebase feature which primary use is avoiding *automatic* merges\ncaused by pull(s).\n\n> During that discussion, you seemed to assume that those who want a\n> flattened end result would never merge locally; I do not think that\n> is true. Having your own merges on a branch that you would want to\n> rebase to flatten is not unusual.\n\nI didn't assume, I rather tried to figure it. However, I didn't hear\nfrom anybody who do it regularly, and I waited for 2 weeks, so I decided\nthat even if there are any, they are in minority, or maybe are\nexperienced enough not to care.\n\nBut that even is not my main argument. Look, 'preserve' preserves\nthings. If one doesn't like the result, he can just rebase, or set\npull.merge to true and repeat the pull, or just \"pull --rebase\".\nSimple. You are still inside the same set of commands and concepts.\n\nOn the other hand, recovering from unneeded flattening is much more\ndifficult, especially for novice, as it requres familarity with a bunch\nof new concepts behind \"git reset\".\n\n> You may employ a workflow to build on separate topic branches while\n> developing, merge the topics among them that are ready into your\n> local 'master' to be used personally, and after using it from your\n> local 'master' for a while to make sure it is solid, you may decide\n> to make it final by publishing it to the outside world, and at that\n> point you would want to flatten the history on top of the latest\n> upstream before pushing.\n\nI'm not proposing to remove any functionality, do I? It's just that\n'preserve' is safer in general and easier to deal with when result\ndoesn't fit. Once again, recovering from unexpected flattening is more\ndifficult.\n\nBTW, even in this workflow, forgetting to pull after merge and before\npush may still lead to the same result that 'preserve' has (no changes\nupstream, so merge will be simply pushed), so setting pull.merge to true\ndoesn't eliminate the problem entirely even in this case. I think the\nright advice for such workflow would then be to flatten your\nhistory locally using \"git rebase\" explicitly, before trying to push\nsomething upstream, that in turn would make this workflow pull.rebase\ntrue/preserve - agnostic.\n\n> That's not anything advanced that takes \"the user knows exactly what\n> she is doing.\"\n\nI didn't write this. This note was there, and I just left it as is. I\nthink it meant to describe both 'true' and 'preserve' settings, even\nthough originally 'preserve' description was closer to it ;-)\n\nTo summarize, my point is that even if there are indeed workflows that\nwould somehow \"suffer\" from 'preserve' setting, it's much easier to\nrealize what happens and fix it either by changing settings and\nrepeating operation, or by rebasing manually, than it is in the case\nwhen \"pull.rebase\" being set to 'true' flattens your merge(s). When that\nhappened to me, there were 3 merges and tens of commits that were\nflattened, and I wish nobody to repeat this experience.\n\nBTW, while we are at the subject,\n\n$ git pull --rebase=true\nCurrent branch master is up to date.\n$ git pull --rebase=preserve\nSuccessfully rebased and updated refs/heads/master.\n$ git pull --rebase=preserve\nSuccessfully rebased and updated refs/heads/master.\n\n(notice no any changes anywhere) is rather surprising. Should I raise a\nseparate issue about it?\n\n-- \nSergey.\n"}]}