{"thread":{"id":"29334","subject":"Re: Regulator updates for 3.3","startedAt":"2012-01-10T22:54:27Z","lastAt":"2012-01-17T08:03:03Z","messageCount":20,"participants":["Linus Torvalds","Mark Brown","Junio C Hamano","Phil Hord","Paul Gortmaker","Nguyen Thai Ngoc Duy","Pete Harlan","Martin Fick","Miles Bader"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"182288","messageId":"CA+55aFxvQF=Bm4ae6euB_UO8otMCuN9Lv37Zn3TpE-L7JH3Kzw@mail.gmail.com","threadId":"29334","inReplyTo":"20120110222711.GK7164@opensource.wolfsonmicro.com","subject":"Re: Regulator updates for 3.3","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-01-10T22:54:27Z","receivedAt":"2012-01-10T22:54:27Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Jan 10, 2012 at 2:27 PM, Mark Brown\n wrote:\n>\n> Especially in the cases where the lack of the bug fix breaks the new\n> code it sems sensible enough to want to do the merges so that the\n> history includes things that actually work.\n\nSo I don't mind merges if they have a lear reason for existing.\n\nThis is actually one of my major gripes with the git UI, and one of\nthe few areas where I really think I screwed up: I made merging *too*\neasy by default. I should have made it always start up an editor for a\nmerge message, the way it does for a commit - rather than just do a\ntrivial pointless merge without even asking the user for a reason for\nthe merge.\n\nSo looking at that almost two months of regulator history in\n\n gitk d52739c62e00..269d430131b6\n\nI would not have reacted badly at all if there were one or two of\nthose merges, and they actually had a reason associated with them.\nSadly, due to that git UI mess-up, that's harder to do than it should\nbe. Oh, it's easy enough with \"git merge --no-commit\" followed by just\n\"git commit\", and then you get the normal git editor window.\n\nSo right now \"git merge\" (and \"git pull\") make it too easy to make\nthose meaningless merge commits. If instead of seven pointless merges\nyou had (say) had two merges that had messages about *why* they\nweren't pointless, I'd be perfectly happy.\n\nAddid junio and git to the cc just to bring up this issue of bad UI\nonce again. I realize it could break old scripts to start up an editor\nwindow, but still..\n\n Linus\n"},{"id":"182290","messageId":"20120110231700.GA14242@opensource.wolfsonmicro.com","threadId":"29334","inReplyTo":"CA+55aFxvQF=Bm4ae6euB_UO8otMCuN9Lv37Zn3TpE-L7JH3Kzw@mail.gmail.com","subject":"Re: Regulator updates for 3.3","fromName":"Mark Brown","fromEmail":"broonie@opensource.wolfsonmicro.com","sentAt":"2012-01-10T23:17:03Z","receivedAt":"2012-01-10T23:17:03Z","isPatch":false,"sender":{"key":"broonie@opensource.wolfsonmicro.com","avatar":"https://gravatar.com/avatar/5fb25e4e0de3255caa21123e2b518c314d26245069221ff55910d5c6ba3343c4?d=mp&s=160"},"body":"On Tue, Jan 10, 2012 at 02:54:27PM -0800, Linus Torvalds wrote:\n> On Tue, Jan 10, 2012 at 2:27 PM, Mark Brown\n\n> > Especially in the cases where the lack of the bug fix breaks the new\n> > code it sems sensible enough to want to do the merges so that the\n> > history includes things that actually work.\n\n> So I don't mind merges if they have a lear reason for existing.\n\nOK, good - I figured that was the case but wanted to make sure as you\nwere stating things rather more strongly than that.\n\nJust to warn you there's also a whole stack of similar merges going to\ncome in via the sound tree too due to the same workflow, I *could* try\nto rebuild the history and ask Takashi to redo his tree using that but\nthere's a lot of history there and it'd be hard to figure out which of\nthe merges was actually important. Is it OK to leave things as they are\nfor this release?\n\n> So right now \"git merge\" (and \"git pull\") make it too easy to make\n> those meaningless merge commits. If instead of seven pointless merges\n> you had (say) had two merges that had messages about *why* they\n> weren't pointless, I'd be perfectly happy.\n\n> Addid junio and git to the cc just to bring up this issue of bad UI\n> once again. I realize it could break old scripts to start up an editor\n> window, but still..\n\nI'd use a configuration option that popped up an editor by default, even\nif I did have to manually enable it.\n"},{"id":"182297","messageId":"7vmx9v7z1r.fsf@alter.siamese.dyndns.org","threadId":"29334","inReplyTo":"CA+55aFxvQF=Bm4ae6euB_UO8otMCuN9Lv37Zn3TpE-L7JH3Kzw@mail.gmail.com","subject":"Re: Regulator updates for 3.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-11T02:28:32Z","receivedAt":"2012-01-11T02:28:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Addid junio and git to the cc just to bring up this issue of bad UI\n> once again. I realize it could break old scripts to start up an editor\n> window, but still..\n\nIt is a non-starter to unconditionally start an editor. We would need a\ngood way for users to conveniently say \"I am doing this unusual merge that\nneeds to be justified, and I want an editor to write my justification\".\n\nObviously, \"git merge -e regulator/for-linus\" would work and is just three\nkeystrokes, which can be said \"convenient enough\" once the user gets used\nto, but I think this is still inadequate as a solution, as the real\nproblem is it is _too_ easy to forget to give the option. Until the user\nbecomes _aware_ of the issues, it will not even occur to the user that\ns/he _has_ to justify a merge (or not create a merge at all) in certain\ncircumstances and directions. After all, you have been repeating the \"do\nnot make meaningless merges\" for the past five years on the list. UI tweak\nalone will not fix that.\n\nIf we are to rely on user's conscious action, I think it may be something\nlike a set of configurations that say things like:\n\n - This branch is for advancing a specific topic, and not for merging\n random development that happen elsewhere;\n\n - This branch is for merging works by people downstream from me;\n\n - This remote tracking branch (and by extension that branch at that\n remote that uses this as its remote tracking branch) is my upstream and\n I should not be merging back from it; and\n\n - This remote tracking branch is my downstream, and I should freely merge\n it when I heard it is ready.\n\nand depending on the combination of what is being merged into what, toggle\nthe --edit option by default for \"git merge\" when neither \"--edit\" nor\n\"--no-edit\" is given, just like \"git merge\" defaults to \"--edit\" when\nmerging an annotated tag.\n"},{"id":"182300","messageId":"CA+55aFx5NATrpLnkMiV2vAxSAJPK7wkY2vyHbyeZGgT9+jP06w@mail.gmail.com","threadId":"29334","inReplyTo":"7vmx9v7z1r.fsf@alter.siamese.dyndns.org","subject":"Re: Regulator updates for 3.3","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-01-11T02:47:55Z","receivedAt":"2012-01-11T02:47:55Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Jan 10, 2012 at 6:28 PM, Junio C Hamano wrote:\n>\n> It is a non-starter to unconditionally start an editor.\n\nI really wonder. Because not being default will always lead to really\nodd ways of saying \"it should have been default, so we'll make up\nthese complex and arbitrary special rules\" (like the ones you were\nstarting to outline).\n\nSo I really suspect it would be easier and more straightforward to\ninstead just bite the bullet, and say:\n\n (a) start an editor by default if both stdin/stdout matched in fstat\nand were istty().\n\n (b) have some trivial way to disable that default behavior for people\nwho really want the legacy behavior. And by \"trivial\" I mean \"set the\nGIT_LEGACY_MERGE environment variable\" or something.\n\n (c) have a \"--no-editor\" command line switch so that scripts and/or\nusers that want to make it explicit (rather than rely on the hacky\nlegacy workaround) can do so (and a explicit \"--editor\" switch to\nenable people to use a GUI editor even if they aren't on a terminal -\nthink something IDE environment, whatever).\n\nWhere (a) is so that people will always get the editor if they aren't\naware of it, and (b) is so that existing scripting environments can\nthen *trivially* work around the fact that we changed semantics,\nincluding on a site-wide basis. With (c) being for future users. Of\ncourse, just a \"git merge < /dev/null\" would also do it, but sounds\nridiculously hacky (and doesn't allow the \"--editor\" version), so that\n\"--no-editor\" flag sounds saner and much more powerful.\n\nOf course, if you use \"-m\", no editor would fire up anyway, exactly\nlike with \"git commit\", so that's one way to avoid the issue forever\n(and be backwards compatible). But if you actually *want* to get the\nauto-generated message and no editor, that would need that new switch.\n\nYes, git has been very good about not breaking semantics. But it's\nhappened before too when it needed to happen. We've had much bigger\nbreaks (like the whole \"git-xyz\" to \"git xyz\" transition, for example,\nwhich broke a lot of scripts).\n\n Linus\n"},{"id":"182302","messageId":"7vehv77xeq.fsf@alter.siamese.dyndns.org","threadId":"29334","inReplyTo":"CA+55aFx5NATrpLnkMiV2vAxSAJPK7wkY2vyHbyeZGgT9+jP06w@mail.gmail.com","subject":"Re: Regulator updates for 3.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-11T03:03:57Z","receivedAt":"2012-01-11T03:03:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Tue, Jan 10, 2012 at 6:28 PM, Junio C Hamano wrote:\n>>\n>> It is a non-starter to unconditionally start an editor.\n>\n> I really wonder. Because not being default will always lead to really\n> odd ways of saying \"it should have been default, so we'll make up\n> these complex and arbitrary special rules\" (like the ones you were\n> starting to outline).\n>\n> So I really suspect it would be easier and more straightforward to\n> instead just bite the bullet, and say:\n>\n> (a) start an editor by default if both stdin/stdout matched in fstat\n> and were istty().\n>\n> (b) have some trivial way to disable that default behavior for people\n> who really want the legacy behavior. And by \"trivial\" I mean \"set the\n> GIT_LEGACY_MERGE environment variable\" or something.\n>\n> (c) have a \"--no-editor\" command line switch so that scripts and/or\n> users that want to make it explicit (rather than rely on the hacky\n> legacy workaround) can do so (and a explicit \"--editor\" switch to\n> enable people to use a GUI editor even if they aren't on a terminal -\n> think something IDE environment, whatever).\n\nHrm. Lack of any quoted line other than the first line from my message,\ntogether with (c) above, makes me suspect that you did not read beyond the\nfirst line before composing this message you are responding to.\n\n> Yes, git has been very good about not breaking semantics. But it's\n> happened before too when it needed to happen. We've had much bigger\n> breaks (like the whole \"git-xyz\" to \"git xyz\" transition, for example,\n> which broke a lot of scripts).\n\nYes, I am learning from the experience to be cautious ;-)\n\nI dunno. You just scrapped the plan for 1.7.10; it may have to be called 2.0\ninstead.\n"},{"id":"182304","messageId":"CA+55aFzuGtJkQFDooSGWQ2_NiJVHN2E7S5dmOnWTYn8_s8Gg3g@mail.gmail.com","threadId":"29334","inReplyTo":"7vehv77xeq.fsf@alter.siamese.dyndns.org","subject":"Re: Regulator updates for 3.3","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-01-11T03:14:55Z","receivedAt":"2012-01-11T03:14:55Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Jan 10, 2012 at 7:03 PM, Junio C Hamano wrote:\n>>\n>> I really wonder. Because not being default will always lead to really\n>> odd ways of saying \"it should have been default, so we'll make up\n>> these complex and arbitrary special rules\" (like the ones you were\n>> starting to outline).\n>\n> Hrm. Lack of any quoted line other than the first line from my message,\n> together with (c) above, makes me suspect that you did not read beyond the\n> first line before composing this message you are responding to.\n\nNo. See again. I did read your suggestion, and that's where the \"we'll\nmake up these complex and arbitrary special rules\" comment comes from.\nDid I mis-understand it?\n\nI think it's a *horrible* idea to go down the road or some\nbranch-specific configurations and then, and I quote:\n\n \"depending on the combination of what is being merged into what,\ntoggle the --edit option by default\"\n\nTHAT is the kind of design that sounds crazy.\n\nInstead, just make editing the default. No ifs, buts, or maybe. No\nconfiguration, no complexities - just make it act the same way our\npager logic acts (ie redirecting stdin/stdout obviously shuts down the\npager, and equally obviously needs to shut down the editor).\n\nThen, the --edit/--no-edit flags are for future users that want to\nmake it explicit. But they aren't about rules, they are about just\nmaking very explicit statements of \"I don't want the editor\".\n\nThe (b) thing I suggested was for \"work around for people who have\nlegacy cases that they don't want to make explicit\". I guess you could\ncount that as some rule, but I really think it's more of a \"ok, we had\nbad legacy behavior, and now we have scripts that depended on that bad\nlegacy\".\n\nBut the notion of complex rules? That sounds really really bad. I'd\nmuch rather get *rid* of the one complex rule we have (the \"merging a\ntag implies --edit\"). That rule is already a hack.\n\n Linus\n"},{"id":"182305","messageId":"CA+55aFwuyjDsRogEugTRnzSGHpO231MkZ9YYNpTTxNSgsfBVrg@mail.gmail.com","threadId":"29334","inReplyTo":"7vehv77xeq.fsf@alter.siamese.dyndns.org","subject":"Re: Regulator updates for 3.3","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-01-11T03:21:19Z","receivedAt":"2012-01-11T03:21:19Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Jan 10, 2012 at 7:03 PM, Junio C Hamano wrote:\n>\n> I dunno. You just scrapped the plan for 1.7.10; it may have to be called 2.0\n> instead.\n\nBtw, version numbers are cheap. I already argued for updating to 2.0\njust because of the new signed tag pulling, which I think is a much\nbigger issue.\n\nI don't think a small change like \"start the editor by default for\nmerge messages\" is nearly as worthy of a version number. But I\nwouldn't argue against it either, exactly because those major numbers\nare cheap.\n\nIt took the kernel until 2.6.39 to learn that, I think git could learn\nto use its major number more freely much earlier.\n\n Linus\n"},{"id":"182323","messageId":"7vzkdu7miv.fsf@alter.siamese.dyndns.org","threadId":"29334","inReplyTo":"CA+55aFzuGtJkQFDooSGWQ2_NiJVHN2E7S5dmOnWTYn8_s8Gg3g@mail.gmail.com","subject":"Re* Regulator updates for 3.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-11T06:59:04Z","receivedAt":"2012-01-11T06:59:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> The (b) thing I suggested was for \"work around for people who have\n> legacy cases that they don't want to make explicit\". I guess you could\n> count that as some rule, but I really think it's more of a \"ok, we had\n> bad legacy behavior, and now we have scripts that depended on that bad\n> legacy\".\n\nI would think that would solve the issue for scripts like the one used to\nrebuild the linux-next tree. I also have such a script to rebuild 'pu' I\nuse three or four times a day, but admittedly the standard input of \"git\nmerge\" in that script is connected to a here document that feeds the list\nof branches to be merged to the loop that drives the \"git merge\", so your\nheuristic (a) will kick in and I wouldn't need the GIT_MERGE_NO_EDIT\nenvironment myself.\n\nWhat makes me uneasy about the idea of running the editor by default is\nthat many people still use Git as a better CVS/SVN. Their workflow is to\nbuild randomly on their 'master', attempt to push and get rejected, pull\nonly so that they can push out, and then push the merge result out. Such\nmerges are done without any consideration on the cohesiveness of the\nbranch (the segment of the history that records their work since they last\npulled from the central repository). Having to justify the backmerge is\nnothing but a nuisance for them, as they do not have any justification\nbetter than \"I am done for the day, and my commuter shuttle will leave in\n15 minutes, so I tried to push what I've done so far, but it was rejected\ndue to non fast-forward, and I am merging random things others did so that\nI can push back\". They won't be saying \"This merges the great work I\ncompleted and have been testing privately for a few days to the trunk\", as\nthe direction of their merge is backwards.\n\nWith that caveat, the patch should look like this.\n\n-- >8 --\nSubject: [PATCH] merge: use editor by default in interactive sessions\n\nTraditionally, a cleanly resolved merge was committed by \"git merge\" using\nthe auto-generated merge commit log message with invoking the editor.\n\nAfter 5 years of use in the field, it turns out that many people perform\ntoo many unjustified backmerges of the upstream history into their topic\nbranches. These merges are not just useless, but they are more often than\nnot explained and making the end result unreadable when it gets time for\nmerging their history back to their upstream.\n\nEarlier we added the \"--edit\" option to the command, so that people can\nedit the log message to explain and justify their merge commits. Let's\ntake it one step further and spawn the editor by default when we are in an\ninteractive session (i.e. the standard input and the standard output are\npointing at the same tty device).\n\nThere may be existing scripts that leave the standard input and the\nstandard output of the \"git merge\" connected to whatever environment the\nscripts were started, and such invocation might trigger the above\n\"interactive session\" heuristics. Such scripts can export GIT_MERGE_LEGACY\nenvironment variable set to \"yes\" to force the traditional behaviour.\n\nSuggested-by: Linus Torvalds\nSigned-off-by: Junio C Hamano \n---\n builtin/merge.c | 34 ++++++++++++++++++++++++++++++----\n t/test-lib.sh | 3 ++-\n 2 files changed, 32 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 99f1429..6a80e1e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -46,7 +46,7 @@ static const char * const builtin_merge_usage[] = {\n \n static int show_diffstat = 1, shortlog_len, squash;\n static int option_commit = 1, allow_fast_forward = 1;\n-static int fast_forward_only, option_edit;\n+static int fast_forward_only, option_edit = -1;\n static int allow_trivial = 1, have_message;\n static struct strbuf merge_msg;\n static struct commit_list *remoteheads;\n@@ -189,7 +189,7 @@ static struct option builtin_merge_options[] = {\n \t\t\"create a single commit instead of doing a merge\"),\n \tOPT_BOOLEAN(0, \"commit\", &option_commit,\n \t\t\"perform a commit if the merge succeeds (default)\"),\n-\tOPT_BOOLEAN('e', \"edit\", &option_edit,\n+\tOPT_BOOL('e', \"edit\", &option_edit,\n \t\t\"edit message before committing\"),\n \tOPT_BOOLEAN(0, \"ff\", &allow_fast_forward,\n \t\t\"allow fast-forward (default)\"),\n@@ -877,12 +877,12 @@ static void prepare_to_commit(void)\n \twrite_merge_msg(&msg);\n \trun_hook(get_index_file(), \"prepare-commit-msg\",\n \t\t git_path(\"MERGE_MSG\"), \"merge\", NULL, NULL);\n-\tif (option_edit) {\n+\tif (0 < option_edit) {\n \t\tif (launch_editor(git_path(\"MERGE_MSG\"), NULL, NULL))\n \t\t\tabort_commit(NULL);\n \t}\n \tread_merge_msg(&msg);\n-\tstripspace(&msg, option_edit);\n+\tstripspace(&msg, 0 < option_edit);\n \tif (!msg.len)\n \t\tabort_commit(_(\"Empty commit message.\"));\n \tstrbuf_release(&merge_msg);\n@@ -1076,6 +1076,29 @@ static void write_merge_state(void)\n \tclose(fd);\n }\n \n+static int default_edit_option(void)\n+{\n+\tstatic const char name[] = \"GIT_MERGE_LEGACY\";\n+\tconst char *e = getenv(name);\n+\tstruct stat st_stdin, st_stdout;\n+\n+\tif (e) {\n+\t\tint v = git_config_maybe_bool(name, e);\n+\t\tif (v < 0)\n+\t\t\tdie(\"Bad value '%s' in environment '%s'\", e, name);\n+\t\treturn !v;\n+\t}\n+\n+\t/* Use editor if stdin and stdout are the same and is a tty */\n+\treturn (!fstat(0, &st_stdin) &&\n+\t\t!fstat(1, &st_stdout) &&\n+\t\tisatty(0) &&\n+\t\tst_stdin.st_dev == st_stdout.st_dev &&\n+\t\tst_stdin.st_ino == st_stdout.st_ino &&\n+\t\tst_stdin.st_rdev == st_stdout.st_rdev);\n+}\n+\n+\n int cmd_merge(int argc, const char **argv, const char *prefix)\n {\n \tunsigned char result_tree[20];\n@@ -1261,6 +1284,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n+\tif (option_edit < 0)\n+\t\toption_edit = default_edit_option();\n+\n \tif (!use_strategies) {\n \t\tif (!remoteheads->next)\n \t\t\tadd_strategies(pull_twohead, DEFAULT_TWOHEAD);\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex bdd9513..439f192 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -63,7 +63,8 @@ GIT_AUTHOR_NAME='A U Thor'\n GIT_COMMITTER_EMAIL=committer@example.com\n GIT_COMMITTER_NAME='C O Mitter'\n GIT_MERGE_VERBOSITY=5\n-export GIT_MERGE_VERBOSITY\n+GIT_MERGE_LEGACY=yes\n+export GIT_MERGE_VERBOSITY GIT_MERGE_LEGACY\n export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME\n export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME\n export EDITOR\n-- \n1.7.9.rc0.39.ged86b\n"},{"id":"182355","messageId":"CABURp0qwjNmHtgkCqdsOk7+_isbpGB6S43WudRgbvE4xnvGUOg@mail.gmail.com","threadId":"29334","inReplyTo":"7vzkdu7miv.fsf@alter.siamese.dyndns.org","subject":"Re: Re* Regulator updates for 3.3","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-01-11T16:14:17Z","receivedAt":"2012-01-11T16:14:17Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Wed, Jan 11, 2012 at 1:59 AM, Junio C Hamano wrote:\n[...]\n>\n> With that caveat, the patch should look like this.\n>\n> -- >8 --\n> Subject: [PATCH] merge: use editor by default in interactive sessions\n>\n> Traditionally, a cleanly resolved merge was committed by \"git merge\" using\n> the auto-generated merge commit log message with invoking the editor.\n>\n> After 5 years of use in the field, it turns out that many people perform\n> too many unjustified backmerges of the upstream history into their topic\n> branches. These merges are not just useless, but they are more often than\n> not explained and making the end result unreadable when it gets time for\n> merging their history back to their upstream.\n\nTypo, I think. I believe you meant \"they are more often than not not\nexplained\", but as this is unclear, maybe you can use \"they are\nusually not explained\" or \"they more often than not go in without\nexplanation\".\n\nP\n"},{"id":"182356","messageId":"CA+55aFy679Skqi_D3x8=M=mwZiViMX9EbZrqP11riiLb_Hzb9g@mail.gmail.com","threadId":"29334","inReplyTo":"7vzkdu7miv.fsf@alter.siamese.dyndns.org","subject":"Re: Re* Regulator updates for 3.3","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-01-11T16:23:01Z","receivedAt":"2012-01-11T16:23:01Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Jan 10, 2012 at 10:59 PM, Junio C Hamano wrote:\n>\n> What makes me uneasy about the idea of running the editor by default is\n> that many people still use Git as a better CVS/SVN. Their workflow is to\n> build randomly on their 'master', attempt to push and get rejected, pull\n> only so that they can push out, and then push the merge result out.\n\nSure. And I don't think we can do much about it. They'll either set\nthe legacy flag, or they'll just exit the editor without adding\nanything useful (if you come from a CVS background in particular, you\nprobably never learnt to do good commit logs anyway).\n\nSo it will be a bit more work for the bad workflow, I agree - although\nif it really irritates people, they can just set that GIT_MERGE_LEGACY\nin their .bashrc files or something. But we can *hope* that even those\npeople might sometimes actually talk about what/why they are doing\nthings, or maybe even learn about that whole \"distributed\" thing.\n\nI agree that is unlikely to ever happen, though. It's more likely that\nthey will change their aliases so that their \"update\" command just\nadds the --no-edit flag. Regardless, it doesn't sound *too* onerous to\nwork around.\n\nPatch looks good to me. I would personally have compared \"st_mode\"\ninstead of (or in addition to) \"st_rdev\", but I don't think it matters\nall that much.\n\n Linus\n"},{"id":"182369","messageId":"20120111184026.GA23952@windriver.com","threadId":"29334","inReplyTo":"CA+55aFxvQF=Bm4ae6euB_UO8otMCuN9Lv37Zn3TpE-L7JH3Kzw@mail.gmail.com","subject":"Re: Regulator updates for 3.3","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2012-01-11T18:40:27Z","receivedAt":"2012-01-11T18:40:27Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"[Re: Regulator updates for 3.3] On 10/01/2012 (Tue 14:54) Linus Torvalds wrote:\n\n[...]\n\n> So right now \"git merge\" (and \"git pull\") make it too easy to make\n> those meaningless merge commits. If instead of seven pointless merges\n\nIt looks like the editor-by-default solution is a go, but there still\nmight be value in increasing the visibility of the pointless merges\nvia. the patch below.\n\nPaul.\n\n> you had (say) had two merges that had messages about *why* they\n> weren't pointless, I'd be perfectly happy.\n> \n> Addid junio and git to the cc just to bring up this issue of bad UI\n> once again. I realize it could break old scripts to start up an editor\n> window, but still..\n> \n> Linus\n\n\n>From 1a548fa97b78cebcded15d2b00ee3d826f731abd Mon Sep 17 00:00:00 2001\nFrom: Paul Gortmaker \nDate: Wed, 11 Jan 2012 10:33:45 -0500\nSubject: [PATCH] merge: Make merge strategy message follow the diffstat\n\nOne of the common problems I've seen with people who are\nsomewhat new to git is that they don't realize that a pull\nis a fetch+merge. They simply decide they want all the\nlatest stuff and issue a git pull without really thinking\nif they are on a branch with local commits or on master,\nwhere a fast forward can take place.\n\nBut the one line message that tells you whether you got a fast\nforward or a real merge commit is usually pushed off the\nscreen by all the diffstat information. So these users won't\neven know that their pull has created a merge, and chances\nare they will never change their workflow.\n\nBy moving the message after the diffstat, there is a better\nchance that people will be aware they've done a pointless\nmerge commit.\n\nSigned-off-by: Paul Gortmaker \n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 3a45172..9471588 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -370,12 +370,12 @@ static void finish(struct commit *head_commit,\n {\n \tstruct strbuf reflog_message = STRBUF_INIT;\n \tconst unsigned char *head = head_commit->object.sha1;\n+\tint automsg = 0;\n \n-\tif (!msg)\n+\tif (!msg) {\n+\t\tautomsg = 1;\n \t\tstrbuf_addstr(&reflog_message, getenv(\"GIT_REFLOG_ACTION\"));\n-\telse {\n-\t\tif (verbosity >= 0)\n-\t\t\tprintf(\"%s\\n\", msg);\n+\t} else {\n \t\tstrbuf_addf(&reflog_message, \"%s: %s\",\n \t\t\tgetenv(\"GIT_REFLOG_ACTION\"), msg);\n \t}\n@@ -409,6 +409,9 @@ static void finish(struct commit *head_commit,\n \t\tdiff_flush(&opts);\n \t}\n \n+\tif (!automsg && verbosity >= 0)\n+\t\tprintf(\"%s\\n\", msg);\n+\n \t/* Run a post-merge hook */\n \trun_hook(NULL, \"post-merge\", squash ? \"1\" : \"0\", NULL);\n \n-- \n1.7.4.4\n"},{"id":"182513","messageId":"7vaa5rzaax.fsf_-_@alter.siamese.dyndns.org","threadId":"29334","inReplyTo":"20120111184026.GA23952@windriver.com","subject":"Re: [PATCH] merge: Make merge strategy message follow the diffstat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-13T19:12:22Z","receivedAt":"2012-01-13T19:12:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Gortmaker writes:\n\n> By moving the message after the diffstat, there is a better chance that\n> people will be aware they've done a pointless merge commit.\n>\n> Signed-off-by: Paul Gortmaker \n\nI think the goal of the change may be worthy, but a few points:\n\n - What does \"automsg\" mean? Is \"auto\" in contrast to \"manual\"? Even\n better, wouldn't it be far simpler to just use\n\n\tif (msg && verbosity >= 0)\n\t\tprintf(\"%s\\n\", msg);\n\n and get rid of this mysteriously named variable altogether?\n\n - Wouldn't it make more sense to move \"No merge message -- not updating\n HEAD\" also to the end?\n\n - After applying this patch, does the tests still pass?\n\nThanks.\n"},{"id":"182515","messageId":"CACsJy8BmFgssTAh=1U7JgBsGG-tSaWXQzZeODND3icXY3QUxug@mail.gmail.com","threadId":"29334","inReplyTo":"7vaa5rzaax.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] merge: Make merge strategy message follow the diffstat","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-13T19:27:01Z","receivedAt":"2012-01-13T19:27:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jan 14, 2012 at 2:12 AM, Junio C Hamano wrote:\n> Paul Gortmaker writes:\n>\n>> By moving the message after the diffstat, there is a better chance that\n>> people will be aware they've done a pointless merge commit.\n>>\n>> Signed-off-by: Paul Gortmaker \n>\n> I think the goal of the change may be worthy\n\nStill, diffstat from a fetch/pull is sometimes too verbose. It'd be\nbetter if we have something that fit in one screen (dirstat or maybe\njust a first few lines from diffstat then ellipsis) then refer users\nto \"git diff --stat HEAD@{1}\" for more detail stat.\n-- \nDuy\n"},{"id":"182518","messageId":"CA+55aFxw_-0h1FDmPRVif3LM03Qh3-6haA7=KYbae8pSFbpW2w@mail.gmail.com","threadId":"29334","inReplyTo":"CACsJy8BmFgssTAh=1U7JgBsGG-tSaWXQzZeODND3icXY3QUxug@mail.gmail.com","subject":"Re: [PATCH] merge: Make merge strategy message follow the diffstat","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-01-13T19:49:34Z","receivedAt":"2012-01-13T19:49:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Fri, Jan 13, 2012 at 11:27 AM, Nguyen Thai Ngoc Duy\n wrote:\n>\n> Still, diffstat from a fetch/pull is sometimes too verbose. It'd be\n> better if we have something that fit in one screen (dirstat or maybe\n> just a first few lines from diffstat then ellipsis) then refer users\n> to \"git diff --stat HEAD@{1}\" for more detail stat.\n\nYeah, I've wanted that. Show the beginning, the end, and the summary\nline of the diffstat would be lovely.\n\nIt would be lovely in \"git commit\" too. Not just\n\n Modified: filename\n\nbut a diffstat that shows now many lines.\n\nAnd what I've *really* wanted is to actually see the diff itself if it\nis small. So some kind of \"dynamic summary\": for one-liners (or\nten-liners), show the whole diff. For medium-sized changes, show the\nwhole diffstat. And for really big changes, show an outline and the\n\"768 files changed, 179851 lines added, 7630 lines removed\" stats.\n\nIOW, whatever fits in, say, 50 lines or less.\n\nThat would be absolutely lovely if somebody were to do it.\n\n Linus\n"},{"id":"182584","messageId":"4F136BE4.4040502@pcharlan.com","threadId":"29334","inReplyTo":"7vzkdu7miv.fsf@alter.siamese.dyndns.org","subject":"Re: Re* Regulator updates for 3.3","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2012-01-16T00:14:28Z","receivedAt":"2012-01-16T00:14:28Z","isPatch":false,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 01/10/2012 10:59 PM, Junio C Hamano wrote:\n> There may be existing scripts that leave the standard input and the\n> standard output of the \"git merge\" connected to whatever environment the\n> scripts were started, and such invocation might trigger the above\n> \"interactive session\" heuristics. Such scripts can export GIT_MERGE_LEGACY\n> environment variable set to \"yes\" to force the traditional behaviour.\n\nThe name GIT_MERGE_LEGACY gives no clue about what flavor of legacy\nmerge behavior is being enabled. Something like GIT_MERGE_LEGACY_EDIT\nmight be clearer, or perhaps just have GIT_MERGE_EDIT=0 to get the old\nbehavior without reference to whether or not that behavior is\nconsidered legacy.\n\n--Pete\n"},{"id":"182653","messageId":"7v62gbussz.fsf@alter.siamese.dyndns.org","threadId":"29334","inReplyTo":"4F136BE4.4040502@pcharlan.com","subject":"Re: Re* Regulator updates for 3.3","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2012-01-16T23:33:00Z","receivedAt":"2012-01-16T23:33:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Harlan writes:\n\n> On 01/10/2012 10:59 PM, Junio C Hamano wrote:\n>> There may be existing scripts that leave the standard input and the\n>> standard output of the \"git merge\" connected to whatever environment the\n>> scripts were started, and such invocation might trigger the above\n>> \"interactive session\" heuristics. Such scripts can export GIT_MERGE_LEGACY\n>> environment variable set to \"yes\" to force the traditional behaviour.\n>\n> The name GIT_MERGE_LEGACY gives no clue about what flavor of legacy\n> merge behavior is being enabled. Something like GIT_MERGE_LEGACY_EDIT\n> might be clearer, or perhaps just have GIT_MERGE_EDIT=0 to get the old\n> behavior without reference to whether or not that behavior is\n> considered legacy.\n\nHrm.\n\nThe only case your suggestion may make a difference would be when we find\nanother earlier UI mistake we would want to correct in a backward\nincompatible way that affects _existing_ scripts.\n\nWith your suggestion, they need to export \"GIT_MERGE_EDIT=0\" today, and\nthey will need to update again to export \"GIT_MERGE_SOMETHINGELSE=0\" when\nsuch an incompatible change comes.\n\nWith a single \"GIT_MERGE_LEGACY=YesPlease\", they can be future-proofed today\nand will not be affected when we make another incompatible change.\n\nSo I am not sure why separating the big-red-switch into smaller pieces\nwould be an improvement, especially wnen the scripts that want to specify\nfiner-grained control of features can use \"--[no-]edit\" options to\nexplicitly ask for it.\n"},{"id":"182655","messageId":"201201161643.23211.mfick@codeaurora.org","threadId":"29334","inReplyTo":"7v62gbussz.fsf@alter.siamese.dyndns.org","subject":"Re: Re* Regulator updates for 3.3","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2012-01-16T23:43:22Z","receivedAt":"2012-01-16T23:43:22Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Monday, January 16, 2012 04:33:00 pm Junio C Hamano \nwrote:\n> With your suggestion, they need to export\n> \"GIT_MERGE_EDIT=0\" today, and they will need to update\n> again to export \"GIT_MERGE_SOMETHINGELSE=0\" when such an\n> incompatible change comes.\n> \n> With a single \"GIT_MERGE_LEGACY=YesPlease\", they can be\n> future-proofed today and will not be affected when we\n> make another incompatible change.\n> \n> So I am not sure why separating the big-red-switch into\n> smaller pieces would be an improvement, especially wnen\n> the scripts that want to specify finer-grained control\n> of features can use \"--[no-]edit\" options to explicitly\n> ask for it.\n\n\nThen, what would I do if I write a script which uses the new \nedit functionality (without even being aware that there was \nan old way) and you introduce a new incompatibility? I \ncan't turn on GIT_MERGE_LEGACY then since it would revert to \nbehavior which my script would not expect (since it was \nwritten after the current incompatibility, but before the \nnew one)!\n\n-Martin\n"},{"id":"182663","messageId":"4F15080C.6060004@pcharlan.com","threadId":"29334","inReplyTo":"7v62gbussz.fsf@alter.siamese.dyndns.org","subject":"Re: Re* Regulator updates for 3.3","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2012-01-17T05:33:00Z","receivedAt":"2012-01-17T05:33:00Z","isPatch":false,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 01/16/2012 03:33 PM, Junio C Hamano wrote:\n> Pete Harlan writes:\n> \n>> On 01/10/2012 10:59 PM, Junio C Hamano wrote:\n>>> There may be existing scripts that leave the standard input and the\n>>> standard output of the \"git merge\" connected to whatever environment the\n>>> scripts were started, and such invocation might trigger the above\n>>> \"interactive session\" heuristics. Such scripts can export GIT_MERGE_LEGACY\n>>> environment variable set to \"yes\" to force the traditional behaviour.\n>>\n>> The name GIT_MERGE_LEGACY gives no clue about what flavor of legacy\n>> merge behavior is being enabled. Something like GIT_MERGE_LEGACY_EDIT\n>> might be clearer, or perhaps just have GIT_MERGE_EDIT=0 to get the old\n>> behavior without reference to whether or not that behavior is\n>> considered legacy.\n> \n> Hrm.\n> \n> The only case your suggestion may make a difference would be when we find\n> another earlier UI mistake we would want to correct in a backward\n> incompatible way that affects _existing_ scripts.\n> \n> With your suggestion, they need to export \"GIT_MERGE_EDIT=0\" today, and\n> they will need to update again to export \"GIT_MERGE_SOMETHINGELSE=0\" when\n> such an incompatible change comes.\n\nWhich is a good thing, because maybe they started using Git after the\ncurrent proposed change (which they like), and what you see as new\nbecomes their \"legacy\" behavior. If you change something after that,\nyou can't use GIT_MERGE_LEGACY=yes for that one also because which\nlegacy is it preserving?\n\nIn general, naming configuration variables \"DO_IT_\" instead\nof \"DO_IT_THE_OLD_WAY\" is better because it's self-documenting. The\nonly time I think I'd prefer \"LEGACY\" is if you're planning on\ndeprecating and removing it eventually and you want to indicate\nsomething to that effect in the name.\n\n--\nPete Harlan\npgit@pcharlan.com\n"},{"id":"182670","messageId":"7vboq2uaa9.fsf@alter.siamese.dyndns.org","threadId":"29334","inReplyTo":"4F15080C.6060004@pcharlan.com","subject":"Re: Re* Regulator updates for 3.3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-17T06:13:02Z","receivedAt":"2012-01-17T06:13:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Harlan writes:\n\n> ... The\n> only time I think I'd prefer \"LEGACY\" is if you're planning on\n> deprecating and removing it eventually and you want to indicate\n> something to that effect in the name.\n\nThe discussion that led to the naming of that LEGACY token needs to be\nre-read, then. The kind of \"LEGACY\" you prefer is exactly why the\nenvironment variable is called LEGACY in the patch you are commenting on,\nwritten in response to Linus's suggestion to switch the default, even\nthough I am not 100% buying it.\n\nHaving said that, I think I am wasting my time responding to this thread\nduring the feature-freeze period for v1.7.9, as I am not a big fan of\nswitching the default without adequate warning and transition plans, after\ngetting burned by the \"'git-foo' vs 'git foo'\" flames back in the v1.6.0\nrelease. We would likely to take a gradual and smoother migration route to\ntransition, e.g. v1.7.9 to introduce \"merge --edit\", v1.7.10 to introduce\na configuration variable merge.edit (lack of which gives a warning and an\nadvice message while defaulting to 'no' to preserve the traditional\nbehaviour), and finally v1.8.0 (or v2.0) to flip the default to 'yes'\n(while the configuration still giving a warning and an advice message)\nthat \"merge --no-edit\" can still countermand.\n\nSo you have until v1.7.10 to decide a good name for the overriding\nenvironment variable.\n"},{"id":"182677","messageId":"buomx9mhi2w.fsf@dhlpc061.dev.necel.com","threadId":"29334","inReplyTo":"CA+55aFxw_-0h1FDmPRVif3LM03Qh3-6haA7=KYbae8pSFbpW2w@mail.gmail.com","subject":"Re: [PATCH] merge: Make merge strategy message follow the diffstat","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2012-01-17T08:03:03Z","receivedAt":"2012-01-17T08:03:03Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"So ... \"--shortish-diffthingy\"\n\n-miles\n\n-- \nZeal, n. A certain nervous disorder afflicting the young and inexperienced.\n"}]}