{"thread":{"id":"23795","subject":"[PATCH] git-rebase--interactive: Handle commit messages that end in backslash","startedAt":"2010-05-13T12:39:55Z","lastAt":"2010-05-14T10:15:55Z","messageCount":4,"participants":["Chris Lamb","Jeff King","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"141583","messageId":"20100513133955.05493175@bobcat.office","threadId":"23795","inReplyTo":null,"subject":"[PATCH] git-rebase--interactive: Handle commit messages that end in backslash","fromName":"Chris Lamb","fromEmail":"lamby@debian.org","sentAt":"2010-05-13T12:39:55Z","receivedAt":"2010-05-13T12:39:55Z","isPatch":true,"sender":{"key":"lamby@debian.org","avatar":null},"body":"\nThis patch fixes three issues with handling commits that end in a\nbackslash character (\"\\\"), two of which had potential for silent data\n\"loss\".\n\nFirst, if you had two commits\n\n abcde... Remove trailing \\\n 01234... Another commit\n\nthen git rebase -i formatted this in your editor like so:\n\n pick abcde Remove trailing pick 01234: Another commit\n\nAssuming you don't spot this visually, git-rebase will then happily skip\nover 01234 upon processing, removing that commit disappear from your HEAD\n(!).\n\nSecondly, when skipping over commits whose parents were unchanged, if any\nof these commits ended in a backslash, it would try and ignore it.\n\nLastly, this patch fixes \"edit\"ing and \"squash\"ing of commits that end\nin a backslash; git-rebase would previously blow up.\n\nSigned-off-by: Chris Lamb \n---\n git-rebase--interactive.sh | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\n\nRegards,\n\n-- \n ,''`.\n : :' : Chris Lamb\n `. `'` lamby@debian.org\n `-\n\n\nFrom 63a17f07dc8eba199f59699a055eebd6e696f27f Mon Sep 17 00:00:00 2001\nFrom: Chris Lamb \nDate: Thu, 13 May 2010 13:36:45 +0100\nSubject: [PATCH] git-rebase--interactive: Handle commit messages that end in backslash\n\nThis patch fixes three issues with handling commits that end in a backslash\ncharacter (\"\\\"), two of which had potential for silent data \"loss\".\n\nFirst, if you had two commits\n\n abcde... Remove trailing \\\n 01234... Another commit\n\nthen git rebase -i formatted this in your editor like so:\n\n pick abcde Remove trailing pick 01234: Another commit\n\nAssuming you don't spot this visually, git-rebase will then happily skip\nover 01234 upon processing, removing that commit disappear from your HEAD\n(!).\n\nSecondly, when skipping over commits whose parents were unchanged, if any\nof these commits ended in a backslash, it would try and ignore it.\n\nLastly, this patch fixes \"edit\"ing and \"squash\"ing of commits that end in a\nbackslash; git-rebase would previously blow up.\n\nSigned-off-by: Chris Lamb \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 436b7f5..31c5fd2 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -450,7 +450,7 @@ record_in_rewritten() {\n \n do_next () {\n \trm -f \"$MSG\" \"$AUTHOR_SCRIPT\" \"$AMEND\" || exit\n-\tread command sha1 rest < \"$TODO\"\n+\tsed -e 's/\\\\$/\\\\\\\\/g' \"$TODO\" | read command sha1 rest\n \tcase \"$command\" in\n \t'#'*|''|noop)\n \t\tmark_action_done\n@@ -591,7 +591,7 @@ do_rest () {\n # skip picking commits whose parents are unchanged\n skip_unnecessary_picks () {\n \tfd=3\n-\twhile read command sha1 rest\n+\tsed -e 's/\\\\$/\\\\\\\\/g' \"$TODO\" | while read command sha1 rest\n \tdo\n \t\t# fd=3 means we skip the command\n \t\tcase \"$fd,$command,$(git rev-parse --verify --quiet $sha1^)\" in\n@@ -607,7 +607,7 @@ skip_unnecessary_picks () {\n \t\t\t;;\n \t\tesac\n \t\techo \"$command${sha1:+ }$sha1${rest:+ }$rest\" >&$fd\n-\tdone <\"$TODO\" >\"$TODO.new\" 3>>\"$DONE\" &&\n+\tdone >\"$TODO.new\" 3>>\"$DONE\" &&\n \tmv -f \"$TODO\".new \"$TODO\" &&\n \tcase \"$(peek_next_command)\" in\n \tsquash|s|fixup|f)\n@@ -890,7 +890,7 @@ first and then run 'git rebase --continue' again.\"\n \t\tgit rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \\\n \t\t\t--abbrev=7 --reverse --left-right --topo-order \\\n \t\t\t$REVISIONS | \\\n-\t\t\tsed -n \"s/^>//p\" | while read shortsha1 rest\n+\t\t\tsed -n \"s/^>//p\" | sed -e 's/\\\\$/\\\\\\\\/g' | while read shortsha1 rest\n \t\tdo\n \t\t\tif test t != \"$PRESERVE_MERGES\"\n \t\t\tthen\n-- \n1.7.1\n\n"},{"id":"141585","messageId":"20100513135807.548594f9@bobcat.office","threadId":"23795","inReplyTo":"20100513133955.05493175@bobcat.office","subject":"Re: [PATCH] git-rebase--interactive: Handle commit messages that end in backslash","fromName":"Chris Lamb","fromEmail":"lamby@debian.org","sentAt":"2010-05-13T12:58:07Z","receivedAt":"2010-05-13T12:58:07Z","isPatch":true,"sender":{"key":"lamby@debian.org","avatar":null},"body":"Chris Lamb wrote:\n \n> This patch fixes three issues with handling commits that end in a\n> backslash character (\"\\\"), two of which had potential for silent data\n> \"loss\".\n\nDisregard the patch; it has some funky side-effect which breaks ONTO\nparsing and I don't know why.\n\n(The problem still exists though.)\n\n\nRegards,\n\n-- \n ,''`.\n : :' : Chris Lamb\n `. `'` lamby@debian.org\n `-\n"},{"id":"141598","messageId":"20100513143350.GA2742@sigill.intra.peff.net","threadId":"23795","inReplyTo":"20100513135807.548594f9@bobcat.office","subject":"Re: [PATCH] git-rebase--interactive: Handle commit messages that end in backslash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-05-13T14:33:50Z","receivedAt":"2010-05-13T14:33:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 13, 2010 at 01:58:07PM +0100, Chris Lamb wrote:\n\n> Disregard the patch; it has some funky side-effect which breaks ONTO\n> parsing and I don't know why.\n>\n> (The problem still exists though.)\n\nHmm. It looks like there is more weirdness, too. Try\n\n git commit -m 'magic \\\\n newline'\n\nI get a stray newline in the git-rebase-todo file. IIRC, we had this\nproblem before because some shells' \"echo\" will interpret, and some will\nnot (I think bash will not, but dash will). So there are a few \"echo\"\ncalls that need to be \"printf %s\" in git-rebase--interactive.\n\nBut I wonder if we would do better to simply sanitize the subject line\nof metacharacters. As I understand it[1], the subject line shown is for\nhuman consumption only, and the actual operations happen on the included\nsha1. So we could munge it in a non-reversible way without breaking\nanything.\n\n-Peff\n\n[1] I may be wrong. This is not code I'm too familiar with, and I do\nremember a proposal at some point for automagically handling subjects\nwith \"wip\" or \"squash\" in them. Still, a light munging may work.\n"},{"id":"141660","messageId":"201005141215.55746.trast@student.ethz.ch","threadId":"23795","inReplyTo":"20100513143350.GA2742@sigill.intra.peff.net","subject":"Re: [PATCH] git-rebase--interactive: Handle commit messages that end in backslash","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-05-14T10:15:55Z","receivedAt":"2010-05-14T10:15:55Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> As I understand it[1], the subject line shown is for\n> human consumption only, and the actual operations happen on the included\n> sha1. So we could munge it in a non-reversible way without breaking\n> anything.\n> \n> -Peff\n> \n> [1] I may be wrong. This is not code I'm too familiar with, and I do\n> remember a proposal at some point for automagically handling subjects\n> with \"wip\" or \"squash\" in them. Still, a light munging may work.\n\nDuring the actual rebasing, only the 'cmd sha' portion of the line\nmatters, as evidenced by the code doing\n\n do_next () {\n rm -f \"$MSG\" \"$AUTHOR_SCRIPT\" \"$AMEND\" || exit\n read command sha1 rest < \"$TODO\"\n\nand then not using $rest except in status messages.\n\nThe automatic handling you are referring to is --autosquash, which\nhappens in rearrange_squash() which is a preprocessing stage. AFAICS\nthat function is still okay iff the munging maps prefixes to prefixes.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"}]}