{"thread":{"id":"29882","subject":"[PATCH 1/2] rebase -i: optimize the creation of the todo file","startedAt":"2012-03-08T10:42:37Z","lastAt":"2012-03-09T09:45:25Z","messageCount":18,"participants":["Dominique Quatravaux","Thomas Rast","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"186410","messageId":"1331203358-28277-1-git-send-email-domq@google.com","threadId":"29882","inReplyTo":null,"subject":"[PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-08T10:42:37Z","receivedAt":"2012-03-08T10:42:37Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"Instead of obtaining short SHA1's from \"git rev-list\" and hitting the repository\nonce more with \"git rev-parse\" for the full-size SHA1's, obtain long SHA1's from\n\"git rev-list\" and truncate them with \"cut\".\n---\n git-rebase--interactive.sh |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 5812222..8dcb8b0 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -774,17 +774,17 @@ else\n \trevisions=$onto...$orig_head\n \tshortrevisions=$shorthead\n fi\n-git rev-list $merges_option --pretty=oneline --abbrev-commit \\\n-\t--abbrev=7 --reverse --left-right --topo-order \\\n+git rev-list $merges_option --pretty=oneline --no-abbrev-commit \\\n+\t--reverse --left-right --topo-order \\\n \t$revisions | \\\n \tsed -n \"s/^>//p\" |\n-while read -r shortsha1 rest\n+while read -r sha1 rest\n do\n+\tshortsha1=$(echo $sha1 | cut -c1-7)\n \tif test t != \"$preserve_merges\"\n \tthen\n \t\tprintf '%s\\n' \"pick $shortsha1 $rest\" >> \"$todo\"\n \telse\n-\t\tsha1=$(git rev-parse $shortsha1)\n \t\tif test -z \"$rebase_root\"\n \t\tthen\n \t\t\tpreserve=t\n-- \n1.7.7.3\n"},{"id":"186411","messageId":"1331203358-28277-2-git-send-email-domq@google.com","threadId":"29882","inReplyTo":"1331203358-28277-1-git-send-email-domq@google.com","subject":"[PATCH 2/2] rebase -i: new option --name-rev","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-08T10:42:38Z","receivedAt":"2012-03-08T10:42:38Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"If set, the second column of the rebase todo contains named revisions (obtained\nwith git name-rev) instead of short SHA1s.\n---\n Documentation/git-rebase.txt  |   11 +++++++++++\n git-rebase--interactive.sh    |   11 ++++++++---\n git-rebase.sh                 |   10 ++++++++++\n t/t3404-rebase-interactive.sh |   11 +++++++++++\n 4 files changed, 40 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 504945c..e7ecd2c 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -365,6 +365,17 @@ If the '--autosquash' option is enabled by default using the\n configuration variable `rebase.autosquash`, this option can be\n used to override and disable this setting.\n \n+--name-rev::\n+--no-name-rev::\n+\tInstead of showing short SHA1 hashes in the todo list, show\n+\thuman-readable revisions obtained with linkgit:git-name-rev[1].\n++\n+This option is only valid when the '--interactive' option is used.\n++\n+If the '--name-rev' option is enabled by default using the\n+configuration variable `rebase.interactivenamerev`, this option can be\n+used to override and disable this setting.\n+\n --no-ff::\n \tWith --interactive, cherry-pick all rebased commits instead of\n \tfast-forwarding over the unchanged ones.  This ensures that the\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 8dcb8b0..5583dcb 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -780,10 +780,15 @@ git rev-list $merges_option --pretty=oneline --no-abbrev-commit \\\n \tsed -n \"s/^>//p\" |\n while read -r sha1 rest\n do\n-\tshortsha1=$(echo $sha1 | cut -c1-7)\n+\tif test t = \"$name_rev\"\n+\tthen\n+\t\trev=\"$(git name-rev $sha1 | cut -d\\  -f2)\"\n+\telse\n+\t\trev=$(echo $sha1 | cut -c1-7)\n+\tfi\n \tif test t != \"$preserve_merges\"\n \tthen\n-\t\tprintf '%s\\n' \"pick $shortsha1 $rest\" >> \"$todo\"\n+\t\tprintf '%s\\n' \"pick $rev $rest\" >> \"$todo\"\n \telse\n \t\tif test -z \"$rebase_root\"\n \t\tthen\n@@ -801,7 +806,7 @@ do\n \t\tif test f = \"$preserve\"\n \t\tthen\n \t\t\ttouch \"$rewritten\"/$sha1\n-\t\t\tprintf '%s\\n' \"pick $shortsha1 $rest\" >> \"$todo\"\n+\t\t\tprintf '%s\\n' \"pick $rev $rest\" >> \"$todo\"\n \t\tfi\n \tfi\n done\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 69c1374..9330be3 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -43,6 +43,8 @@ s,strategy=!       use the given merge strategy\n no-ff!             cherry-pick all commits, even if unchanged\n m,merge!           use merging strategies to rebase\n i,interactive!     let the user edit the list of commits to rebase\n+name-rev           show revisions by name in the list of commits\n+no-name-rev        show revisions by short SHA1 in the list (default)\n f,force-rebase!    force rebase even if branch is up to date\n X,strategy-option=! pass the argument through to the merge strategy\n stat!              display a diffstat of what changed upstream\n@@ -98,6 +100,8 @@ action=\n preserve_merges=\n autosquash=\n test \"$(git config --bool rebase.autosquash)\" = \"true\" && autosquash=t\n+name_rev=\n+test \"$(git config --bool rebase.interactivenamerev)\" = \"true\" && name_rev=t\n \n read_basic_state () {\n \thead_name=$(cat \"$state_dir\"/head-name) &&\n@@ -287,6 +291,12 @@ do\n \t-f|--no-ff)\n \t\tforce_rebase=t\n \t\t;;\n+\t--name-rev)\n+\t\tname_rev=t\n+\t\t;;\n+\t--no-name-rev)\n+\t\tname_rev=\n+\t\t;;\n \t--rerere-autoupdate|--no-rerere-autoupdate)\n \t\tallow_rerere_autoupdate=\"$1\"\n \t\t;;\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex b981572..299ce40 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -163,6 +163,17 @@ test_expect_success 'exchange two commits' '\n \ttest G = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n+cat > expect-rebase-todo <<EOF\n+pick branch1~1 H\n+pick branch1 G\n+EOF\n+\n+test_expect_success 'Symbolic revisions in --name-rev' '\n+\texec > debug.log 2>&1 &&\n+\tFAKE_LINES=\"exec_cp_.git/rebase-merge/git-rebase-todo_rebase-todo 1 2\" git rebase -i --name-rev HEAD~2 &&\n+\ttest_cmp expect-rebase-todo rebase-todo\n+'\n+\n cat > expect << EOF\n diff --git a/file1 b/file1\n index f70f10e..fd79235 100644\n-- \n1.7.7.3\n"},{"id":"186413","messageId":"87boo7pdvc.fsf@thomas.inf.ethz.ch","threadId":"29882","inReplyTo":"1331203358-28277-1-git-send-email-domq@google.com","subject":"Re: [PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-03-08T10:48:39Z","receivedAt":"2012-03-08T10:48:39Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Dominique Quatravaux <domq@google.com> writes:\n\n> Instead of obtaining short SHA1's from \"git rev-list\" and hitting the repository\n> once more with \"git rev-parse\" for the full-size SHA1's, obtain long SHA1's from\n> \"git rev-list\" and truncate them with \"cut\".\n\nThat doesn't work if there are 7-digit SHA1 collisions in the repo.\nEven my git.git has a bunch of them, as checked with\n\n  git rev-list --objects --all | cut -c1-7 | sort | uniq -d\n\nand I expect a bigger project would have collisions beyond the 9th\ndigit.\n\nWhat you can, however, do:\n\n> -git rev-list $merges_option --pretty=oneline --abbrev-commit \\\n> -\t--abbrev=7 --reverse --left-right --topo-order \\\n> +git rev-list $merges_option --pretty=oneline --no-abbrev-commit \\\n> +\t--reverse --left-right --topo-order \\\n>  \t$revisions | \\\n\nrev-list can give you *both* the abbreviated and full SHA1s if you say\n\n  git rev-list $merges_option --format=\"%>%h %H %s\" <...etc...>\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"186414","messageId":"87399jnyxh.fsf@thomas.inf.ethz.ch","threadId":"29882","inReplyTo":"1331203358-28277-2-git-send-email-domq@google.com","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-03-08T10:56:42Z","receivedAt":"2012-03-08T10:56:42Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"On a general note: you are submitting a completely new feature touching\na heavily-used tool (and code path) during -rc0 time.  As a rule of\nthumb: Don't do that.  If you do it, don't Cc Junio unless it's his area\nof code.\n\nDominique Quatravaux <domq@google.com> writes:\n\n> If set, the second column of the rebase todo contains named revisions (obtained\n> with git name-rev) instead of short SHA1s.\n\nHum.  I'm not sure yet if I find that very useful, since frequently the\nnames will just be 'topic', 'topic~1', ...., 'topic~N' if you are\nrebasing a topic with N+1 commits not in master.  But you might, so who\nam I to judge.\n\n> +--name-rev::\n> +--no-name-rev::\n\nThe --no- version is implicitly always supported, see gitcli(1).\n\n> +configuration variable `rebase.interactivenamerev`, this option can be\n\nYou should spell it in a more readable way such as\nrebase.interactiveNameRev.  The config machinery internally downcases\neverything so the cosmetics won't prevent it from working.\n\n> -\tshortsha1=$(echo $sha1 | cut -c1-7)\n> +\tif test t = \"$name_rev\"\n> +\tthen\n> +\t\trev=\"$(git name-rev $sha1 | cut -d\\  -f2)\"\n> +\telse\n> +\t\trev=$(echo $sha1 | cut -c1-7)\n> +\tfi\n\nIn the spirit of your previous patch, wouldn't it be faster to run 'git\nname-rev --stdin' within the pipeline?\n\nHow does this interact with --autosquash?\n\n> +test_expect_success 'Symbolic revisions in --name-rev' '\n> +\texec > debug.log 2>&1 &&\n> +\tFAKE_LINES=\"exec_cp_.git/rebase-merge/git-rebase-todo_rebase-todo 1 2\" git rebase -i --name-rev HEAD~2 &&\n> +\ttest_cmp expect-rebase-todo rebase-todo\n> +'\n\nIn line with the --autosquash concern, please write a test that uses\nboth option (and verifies that *both* work!).\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"186415","messageId":"4F589611.6060106@viscovery.net","threadId":"29882","inReplyTo":"1331203358-28277-1-git-send-email-domq@google.com","subject":"Re: [PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-08T11:20:49Z","receivedAt":"2012-03-08T11:20:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/8/2012 11:42, schrieb Dominique Quatravaux:\n> +\tshortsha1=$(echo $sha1 | cut -c1-7)\n\n> -\t\tsha1=$(git rev-parse $shortsha1)\n\nWhy do you call it \"optimization\" when you spend two or three subprocesses\ninstead of one?\n\n-- Hannes\n"},{"id":"186418","messageId":"CAJh6GrGch9JHYnGW+K-wxdX9wPRry4Lxi2ysTtt0g9FTF4ThTQ@mail.gmail.com","threadId":"29882","inReplyTo":"4F589611.6060106@viscovery.net","subject":"Re: [PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-08T11:36:43Z","receivedAt":"2012-03-08T11:36:43Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"On Thu, Mar 8, 2012 at 12:20 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n> Am 3/8/2012 11:42, schrieb Dominique Quatravaux:\n>> +     shortsha1=$(echo $sha1 | cut -c1-7)\n>\n>> -             sha1=$(git rev-parse $shortsha1)\n>\n> Why do you call it \"optimization\" when you spend two or three subprocesses\n> instead of one?\n\necho is a shell internal. \"git rev-parse\" is two processes just as\n\"cut\" and a pipe. The difference is that cut doesn't hit the git\nrepository.\n\nBut the real purpose of this first change is to lay the ground for the\nnext one, so I'd be happy to change the patch description...\n\n\n-- \n  Dominique Quatravaux\n  +41 79 609 40 72\n"},{"id":"186419","messageId":"CAJh6GrGcr3nOMR7Yrq7g8c-RgQ4FS_FrhcMb+w-nXQO2nic_rw@mail.gmail.com","threadId":"29882","inReplyTo":"CAJh6GrGch9JHYnGW+K-wxdX9wPRry4Lxi2ysTtt0g9FTF4ThTQ@mail.gmail.com","subject":"Re: [PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-08T11:41:57Z","receivedAt":"2012-03-08T11:41:57Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"On Thu, Mar 8, 2012 at 12:36 PM, Dominique Quatravaux <domq@google.com> wrote:\n> On Thu, Mar 8, 2012 at 12:20 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n>> Am 3/8/2012 11:42, schrieb Dominique Quatravaux:\n>>> +     shortsha1=$(echo $sha1 | cut -c1-7)\n>>\n>>> -             sha1=$(git rev-parse $shortsha1)\n>>\n>> Why do you call it \"optimization\" when you spend two or three subprocesses\n>> instead of one?\n>\n> echo is a shell internal. \"git rev-parse\" is two processes just as\n> \"cut\" and a pipe.\n\nMy mistake, strace git rev-parse revals that this is only one process.\nStill, I think that saving a bunch of filesystem access beats saving\none fork() (one of the two processes in my patched version is a shell,\nso no execve() there) but I admit I haven't benchmarked this.\n\n\n-- \n  Dominique Quatravaux\n  +41 79 609 40 72\n"},{"id":"186420","messageId":"CAJh6GrGWfyKguhgf9dJR4yG3qYjh2jJz7OzzN8wao0rWLcohoA@mail.gmail.com","threadId":"29882","inReplyTo":"87boo7pdvc.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-08T11:48:24Z","receivedAt":"2012-03-08T11:48:24Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"On Thu, Mar 8, 2012 at 11:48 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n> Dominique Quatravaux <domq@google.com> writes:\n>\n>> Instead of obtaining short SHA1's from \"git rev-list\" and hitting the repository\n>> once more with \"git rev-parse\" for the full-size SHA1's, obtain long SHA1's from\n>> \"git rev-list\" and truncate them with \"cut\".\n>\n> That doesn't work if there are 7-digit SHA1 collisions in the repo.\n\nI don't see how my patch changes the picture wrt SHA1 collisions. In\nthe current state, the rebase todo already uses short SHA1s.\n\n> Even my git.git has a bunch of them, as checked with\n>\n>  git rev-list --objects --all | cut -c1-7 | sort | uniq -d\n>\n> and I expect a bigger project would have collisions beyond the 9th\n> digit.\n>\n> What you can, however, do:\n>\n>> -git rev-list $merges_option --pretty=oneline --abbrev-commit \\\n>> -     --abbrev=7 --reverse --left-right --topo-order \\\n>> +git rev-list $merges_option --pretty=oneline --no-abbrev-commit \\\n>> +     --reverse --left-right --topo-order \\\n>>       $revisions | \\\n>\n> rev-list can give you *both* the abbreviated and full SHA1s if you say\n>\n>  git rev-list $merges_option --format=\"%>%h %H %s\" <...etc...>\n\nInteresting, thanks! It seems that %h works with find_unique_abbrev\nwhich according to the name, should Do The Right Thing. I'll update my\npatch.\n\n\n-- \n  Dominique Quatravaux\n  +41 79 609 40 72\n"},{"id":"186422","messageId":"4F589D40.5060804@viscovery.net","threadId":"29882","inReplyTo":"CAJh6GrGcr3nOMR7Yrq7g8c-RgQ4FS_FrhcMb+w-nXQO2nic_rw@mail.gmail.com","subject":"Re: [PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-08T11:51:28Z","receivedAt":"2012-03-08T11:51:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/8/2012 12:41, schrieb Dominique Quatravaux:\n> On Thu, Mar 8, 2012 at 12:36 PM, Dominique Quatravaux <domq@google.com> wrote:\n>> On Thu, Mar 8, 2012 at 12:20 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n>>> Am 3/8/2012 11:42, schrieb Dominique Quatravaux:\n>>>> +     shortsha1=$(echo $sha1 | cut -c1-7)\n>>>\n>>>> -             sha1=$(git rev-parse $shortsha1)\n>>>\n>>> Why do you call it \"optimization\" when you spend two or three subprocesses\n>>> instead of one?\n>>\n>> echo is a shell internal. \"git rev-parse\" is two processes just as\n>> \"cut\" and a pipe.\n> \n> My mistake, strace git rev-parse revals that this is only one process.\n> Still, I think that saving a bunch of filesystem access beats saving\n> one fork()... \n\nNot so on Windows.\n\nBut you must look at the repository in any case to avoid truncating the\nSHA1 too much, as Thomas pointed out.\n\n-- Hannes\n"},{"id":"186424","messageId":"87haxzl337.fsf@thomas.inf.ethz.ch","threadId":"29882","inReplyTo":"CAJh6GrGWfyKguhgf9dJR4yG3qYjh2jJz7OzzN8wao0rWLcohoA@mail.gmail.com","subject":"Re: [PATCH 1/2] rebase -i: optimize the creation of the todo file","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-03-08T11:55:08Z","receivedAt":"2012-03-08T11:55:08Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Dominique Quatravaux <domq@google.com> writes:\n\n> On Thu, Mar 8, 2012 at 11:48 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>> Dominique Quatravaux <domq@google.com> writes:\n>>\n>>> Instead of obtaining short SHA1's from \"git rev-list\" and hitting the repository\n>>> once more with \"git rev-parse\" for the full-size SHA1's, obtain long SHA1's from\n>>> \"git rev-list\" and truncate them with \"cut\".\n>>\n>> That doesn't work if there are 7-digit SHA1 collisions in the repo.\n>\n> I don't see how my patch changes the picture wrt SHA1 collisions. In\n> the current state, the rebase todo already uses short SHA1s.\n\nBut it gets them from --abbrev-commit, which makes sure that the\nabbreviation is unique.  The 7 is just a request for the minimal number.\n\nYou can easily verify this in git.git with e.g.\n\n  git show --abbrev-commit --abbrev=7 \\\n      2539a7b64dbc7e214704f6b879749a94685fb99e\n\nNote how the output uses an 8-digit SHA1 in the first line.  The 7-digit\ncolliding commit is 2539a7bdb81dcc920c97837a9bfb9d4c4e1ac280.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"186425","messageId":"CAJh6GrFKJw1506c6nOzh+EcNe7E_UM=NxV_yBPWx9BUCdpAvAg@mail.gmail.com","threadId":"29882","inReplyTo":"87399jnyxh.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-08T11:57:02Z","receivedAt":"2012-03-08T11:57:02Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"On Thu, Mar 8, 2012 at 11:56 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n> On a general note: you are submitting a completely new feature touching\n> a heavily-used tool (and code path) during -rc0 time.  As a rule of\n> thumb: Don't do that.  If you do it, don't Cc Junio unless it's his area\n> of code.\n\n[- gitster]\nSorry about that, I skimmed Junio's \"What's cooking in git.git (Mar\n2012, #03; Mon, 5)\" and I thought I was in the \"high value/damage\nratio\" category.\n\n> Hum.  I'm not sure yet if I find that very useful, since frequently the\n> names will just be 'topic', 'topic~1', ...., 'topic~N'\n\nExactly, and when rebasing a branch with merges, the \"foreign\" commits\nwill stick out which is what I want.\n\n> The --no- version is implicitly always supported, see gitcli(1).\n\nDo you mean I should omit it from the doc?\n\n>> +configuration variable `rebase.interactivenamerev`, this option can be\n>\n> You should spell it in a more readable way such as\n> rebase.interactiveNameRev.\n\nWill do.\n\n>> -     shortsha1=$(echo $sha1 | cut -c1-7)\n>> +     if test t = \"$name_rev\"\n>> +     then\n>> +             rev=\"$(git name-rev $sha1 | cut -d\\  -f2)\"\n>> +     else\n>> +             rev=$(echo $sha1 | cut -c1-7)\n>> +     fi\n>\n> In the spirit of your previous patch, wouldn't it be faster to run 'git\n> name-rev --stdin' within the pipeline?\n\nI fear that it would also substitute hashes on the right-hand side (in\nthe commit messages), and also I would be piping data from and to git\nname-rev from the same process which is a recipe for deadlocks.\n\n> In line with the --autosquash concern, please write a test that uses\n> both option (and verifies that *both* work!).\n\nWill do.\n\n-- \n  Dominique Quatravaux\n  +41 79 609 40 72\n"},{"id":"186476","messageId":"7veht2ex7v.fsf@alter.siamese.dyndns.org","threadId":"29882","inReplyTo":"CAJh6GrFKJw1506c6nOzh+EcNe7E_UM=NxV_yBPWx9BUCdpAvAg@mail.gmail.com","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-08T18:58:28Z","receivedAt":"2012-03-08T18:58:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dominique Quatravaux <domq@google.com> writes:\n\n> On Thu, Mar 8, 2012 at 11:56 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>> On a general note: you are submitting a completely new feature touching\n>> a heavily-used tool (and code path) during -rc0 time. As a rule of\n>> thumb: Don't do that. If you do it, don't Cc Junio unless it's his area\n>> of code.\n>\n> [- gitster]\n> Sorry about that, I skimmed Junio's \"What's cooking in git.git (Mar\n> 2012, #03; Mon, 5)\" and I thought I was in the \"high value/damage\n> ratio\" category.\n\nWell, these days, nothing that comes without prior discussion on\npain points before the feature freeze, be it from seasoned list\nregulars or from people new to this list, can be of so high-value\nthat it cannot wait until the next cycle.\n\nThe only handful of exceptions I can think of are:\n\n (1) an obvious fix to a high-risk bug (new or old) that was\n     recently discovered as a potential attack vector;\n\n (2) an obvious fix to a bug in a new feature added in the upcoming\n     release; or\n\n (3) an obviously necessary tweak and adjustment to parts of the\n     system related to a new feature added in the upcoming release.\n\n(1) is called \"security fix\" and (2) is \"brown paper bag fix\".\n\nI do not think of a good word to explain (3), but for example, if a\nrelease candidate added a feature to \"git merge\" so that a signed\ntag can be merged while recording the signature in the resulting\ncommit, but the feature did not trigger when the merge is made via\n\"git pull\" because it unwrapped the fetched tag to a commit before\ncalling \"git merge\", an update to \"git pull\" and probably \"git\nfmt-merge-msg\" that is used by \"git pull\" may have to be made after\nwe enter the soft feature freeze (-rc0) for the new feature in \"git\nmerge\" to be useful.\n"},{"id":"186477","messageId":"7vaa3qewqw.fsf@alter.siamese.dyndns.org","threadId":"29882","inReplyTo":"87399jnyxh.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-08T19:08:39Z","receivedAt":"2012-03-08T19:08:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@inf.ethz.ch> writes:\n\n> Dominique Quatravaux <domq@google.com> writes:\n>\n>> If set, the second column of the rebase todo contains named revisions (obtained\n>> with git name-rev) instead of short SHA1s.\n>\n> Hum.  I'm not sure yet if I find that very useful, since frequently the\n> names will just be 'topic', 'topic~1', ...., 'topic~N' if you are\n> rebasing a topic with N+1 commits not in master.  But you might, so who\n> am I to judge.\n\nI think the only use case where this might be useful is when you\nhave totally undescriptive one-line description to your commits that\nthey alone do not help distinguishing the commits being picked, e.g.\n\n  pick 47a02ff add frotz\n  pick d41489a fix frotz\n  pick 00c8fd4 fix frotz more\n  pick 090ea12 again fix frotz\n  pick 74775a0 fix frotz one more time\n  pick 6f7f3be at least now frotz compiles\n\nAs we treat the one-line subject merely as a human aid, I would\nprobably do not mind if a workaround for such an issue were done to\nproduce something like this instead:\n\n  pick 47a02ff (1) add frotz\n  pick d41489a (2) fix frotz\n  pick 00c8fd4 (3) fix frotz more\n  pick 090ea12 (4) again fix frotz\n  pick 74775a0 (5) fix frotz one more time\n  pick 6f7f3be (6) at least now frotz compiles\n\nIt is not a problem with the current rebase-i that detaches HEAD\nwhile working, but using object names like \"topic~7\" that is\nrelative to something inherently fluid (i.e. a branch) makes me feel\nnervous as an end user from aesthetics point of view.\n"},{"id":"186505","messageId":"7vipiebv1r.fsf@alter.siamese.dyndns.org","threadId":"29882","inReplyTo":"7vaa3qewqw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-08T22:13:36Z","receivedAt":"2012-03-08T22:13:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Thomas Rast <trast@inf.ethz.ch> writes:\n>\n>> Dominique Quatravaux <domq@google.com> writes:\n>>\n>>> If set, the second column of the rebase todo contains named revisions (obtained\n>>> with git name-rev) instead of short SHA1s.\n>>\n>> Hum.  I'm not sure yet if I find that very useful, since frequently the\n>> names will just be 'topic', 'topic~1', ...., 'topic~N' if you are\n>> rebasing a topic with N+1 commits not in master.  But you might, so who\n>> am I to judge.\n>\n> I think the only use case where this might be useful is when you\n> have totally undescriptive one-line description to your commits that\n> they alone do not help distinguishing the commits being picked, e.g.\n> ...\n\nThis may need a bit of clarification for readers from the future.\nIf you _were_ somehow interactively rebasing changes made on two or\nmore branches into a single branch, knowing which branch each commit\ncame from may have value, even if your commit titles are descriptive\nenough.\n\nToday's \"git rebase -i\" wouldn't do something like that, and we will\nnot know how the user would interact with such a yet-to-be-written\ntool, so it is too early to judge if using \"topic~1\" is the desired\nimprovement or not at this point.\n"},{"id":"186530","messageId":"4F59AFA9.5030205@viscovery.net","threadId":"29882","inReplyTo":"7vipiebv1r.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-03-09T07:22:17Z","receivedAt":"2012-03-09T07:22:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/8/2012 23:13, schrieb Junio C Hamano:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> Thomas Rast <trast@inf.ethz.ch> writes:\n>>\n>>> Dominique Quatravaux <domq@google.com> writes:\n>>>\n>>>> If set, the second column of the rebase todo contains named revisions (obtained\n>>>> with git name-rev) instead of short SHA1s.\n>>>\n>>> Hum.  I'm not sure yet if I find that very useful, since frequently the\n>>> names will just be 'topic', 'topic~1', ...., 'topic~N' if you are\n>>> rebasing a topic with N+1 commits not in master.  But you might, so who\n>>> am I to judge.\n>>\n>> I think the only use case where this might be useful is when you\n>> have totally undescriptive one-line description to your commits that\n>> they alone do not help distinguishing the commits being picked, e.g.\n>> ...\n> \n> This may need a bit of clarification for readers from the future.\n> If you _were_ somehow interactively rebasing changes made on two or\n> more branches into a single branch, knowing which branch each commit\n> came from may have value, even if your commit titles are descriptive\n> enough.\n> \n> Today's \"git rebase -i\" wouldn't do something like that, and we will\n> not know how the user would interact with such a yet-to-be-written\n> tool, so it is too early to judge if using \"topic~1\" is the desired\n> improvement or not at this point.\n\nYet-to-be-written? Rebase -i happily linearizes mergy history, so this\ndoes have some merits even today.\n\nI do share your concerns that naming to-be-rebased commits with a relative\nspecifier such as \"topic~1\" could be dangerous. However, this is a problem\nonly when the rebase -i is not completed timely, so that you have\nsufficient time to mess with the ref \"topic\" from a different terminal.\nYou would have to run \"git branch -f\", \"git fetch\", or \"git push\" (the\nlatter could even happen from remote) that involve the ref name. Can't we\njust declare this as \"don't do that then\"? (We do say this more often than\nnot since recently :-)\n\n-- Hannes\n"},{"id":"186533","messageId":"CAJh6GrFRaW7HdRjdxWvt=8cC6qfDz5BJESJGKqR3Bymg7SDb2A@mail.gmail.com","threadId":"29882","inReplyTo":"7veht2ex7v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-09T07:58:05Z","receivedAt":"2012-03-09T07:58:05Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"On Thu, Mar 8, 2012 at 11:56 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>>> On a general note: you are submitting a completely new feature touching\n>>> a heavily-used tool (and code path) during -rc0 time. As a rule of\n>>> thumb: Don't do that. [..]\n\nDominique Quatravaux <domq@google.com> writes:\n>> Sorry about that, I skimmed Junio's \"What's cooking in git.git (Mar\n>> 2012, #03; Mon, 5)\" and I thought I was in the \"high value/damage\n>> ratio\" category.\n\nOn Thu, Mar 8, 2012 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Well, these days, nothing that comes without prior discussion on\n> pain points before the feature freeze, be it from seasoned list\n> regulars or from people new to this list, can be of so high-value\n> that it cannot wait until the next cycle.\n\nACK. I'm cool with skipping a cycle, they are so fast anyways!\n\n-- \n  Dominique Quatravaux\n  +41 79 609 40 72\n"},{"id":"186541","messageId":"CAJh6GrGEix8qfAuJamWzcr69=LjkZ-O9B=Vorvy_KTcJe3TcPA@mail.gmail.com","threadId":"29882","inReplyTo":"4F59AFA9.5030205@viscovery.net","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Dominique Quatravaux","fromEmail":"domq@google.com","sentAt":"2012-03-09T09:04:12Z","receivedAt":"2012-03-09T09:04:12Z","isPatch":true,"sender":{"key":"domq@google.com","avatar":"https://gravatar.com/avatar/121b20f85f75e3366ecd70be7f2777189ed53d0cc5472531cde13e4ed9e4f61e?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n>> Today's \"git rebase -i\" wouldn't do something like that, and we will\n>> not know how the user would interact with such a yet-to-be-written\n>> tool, so it is too early to judge if using \"topic~1\" is the desired\n>> improvement or not at this point.\n>\nOn Fri, Mar 9, 2012 at 8:22 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n> Yet-to-be-written? Rebase -i happily linearizes mergy history, so this\n> does have some merits even today.\n\nRight, my personal itch is to \"transplant\" topic branches without their merge\nhistory getting in the way, eg go from\n\n\n   M1 ------ M2 ----- M3 ---- M4                master\n     \\                  \\\n      \\           B1 --- B2 --- B3              topicB\n       \\         /\n        A1 --- A2 --- A3                        topicA\n\nto\n\n   M1 ---- M2 ---- M3 ---- M4                   master\n     \\                       \\\n      \\                       B1 ---- B3        topicB\n       \\\n        A1 ---- A2 ---- A3                      topicA\n\n\nIf I \"git rebase -i --onto master topicA topicB\", the rebase todo might go like\n\n  pick 1234abc Cool shiny new stuff\n  pick 234abc1 Something something master\n  pick 34abc12 Fix something something\n  pick 4abc123 Fix shiny new stuff\n\nWith my patch (combined with Junio's suggestion, and some whitespace padding\nfor extra niceness) we would get instead\n\n  pick 1234abc (topicB~2) Cool shiny new stuff\n  pick 234abc1 (master~2) Something something master\n  pick 34abc12 (master~1) Fix something something\n  pick 4abc123 (topicB)   Fix shiny new stuff\n\nSnip the lines that don't m/topicB/ with your text editor, save file, done.\n\n> I do share your concerns that naming to-be-rebased commits with a relative\n> specifier such as \"topic~1\" could be dangerous. However, this is a problem\n> only when the rebase -i is not completed timely, so that you have\n> sufficient time to mess with the ref \"topic\" from a different terminal.\n\nI think that Junio's suggestion fixes that (at the expense of 8 of the precious\n80 columns).\n\n--\n  Dominique Quatravaux\n  +41 79 609 40 72\n"},{"id":"186545","messageId":"7vty1y85vu.fsf@alter.siamese.dyndns.org","threadId":"29882","inReplyTo":"CAJh6GrGEix8qfAuJamWzcr69=LjkZ-O9B=Vorvy_KTcJe3TcPA@mail.gmail.com","subject":"Re: [PATCH 2/2] rebase -i: new option --name-rev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-09T09:45:25Z","receivedAt":"2012-03-09T09:45:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dominique Quatravaux <domq@google.com> writes:\n\n> On Fri, Mar 9, 2012 at 8:22 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n>> Rebase -i happily linearizes mergy history, so this\n>> does have some merits even today.\n>\n> Right, my personal itch is to \"transplant\" topic branches without their merge\n> history getting in the way,...\n> ...\n>   pick 1234abc (topicB~2) Cool shiny new stuff\n>   pick 234abc1 (master~2) Something something master\n>   pick 34abc12 (master~1) Fix something something\n>   pick 4abc123 (topicB)   Fix shiny new stuff\n\nOk. I didn't consider the \"flattening\" aspect of the rebase, and the\nabove makes sense to me.\n"}]}