{"thread":{"id":"24731","subject":"git rebase -i exec merger broke t3404-rebase-interactive.sh","startedAt":"2010-08-13T16:37:09Z","lastAt":"2010-08-22T07:22:58Z","messageCount":7,"participants":["Ævar Arnfjörð Bjarmason","Brian Gernhardt","Brandon Casey","Matthieu Moy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"148003","messageId":"AANLkTinzBuR+9=+PwejJVwSkUiGODaP-RC7=agyLOgMt@mail.gmail.com","threadId":"24731","inReplyTo":null,"subject":"git rebase -i exec merger broke t3404-rebase-interactive.sh","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-13T16:37:09Z","receivedAt":"2010-08-13T16:37:09Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"39e388728 (merging git rebase -i exec support) broke the funny names\ntest in t3404-rebase-interactive.sh:\n\n    expecting success:\n            git rev-list A..funny >expect &&\n            test_tick &&\n            FAKE_LINES=\"1 2 3 4\" git rebase -i A &&\n            git rev-list A.. >actual &&\n            test_cmp expect actual\n\n    rebase -i script before editing:\n    pick f6fec15 end with slash\\\n    pick 4f2ade4 something (\\000) that looks like octal\n    pick f95dfbf something (\\n) that looks like a newline\n    pick 0e82af6 another commit\n\n    rebase -i script after editing:\n    pick f6fec15 end with slash\\\n    pick 4f2ade4 something (\\000) that looks like octal\n    pick f95dfbf something (\\n) that looks like a newline\n    pick 0e82af6 another commit\n    Unknown command: ) that looks like a newline\n    Please fix this in the file /home/avar/g/git/t/trash\ndirectory.t3404-rebase-interactive/.git/rebase-merge/git-rebase-todo.\n    not ok - 50 rebase-i history with funny messages\n    #\n    #               git rev-list A..funny >expect &&\n    #               test_tick &&\n    #               FAKE_LINES=\"1 2 3 4\" git rebase -i A &&\n    #               git rev-list A.. >actual &&\n    #               test_cmp expect actual\n    #\n\nThis one breaks under bash too, does it work for you Matthieu? If so\nwhat sort of environment are you executing it in?\n"},{"id":"148007","messageId":"D1A252AE-5D4C-4E51-9359-F4A443BB8A2E@silverinsanity.com","threadId":"24731","inReplyTo":"AANLkTinzBuR+9=+PwejJVwSkUiGODaP-RC7=agyLOgMt@mail.gmail.com","subject":"Re: git rebase -i exec merger broke t3404-rebase-interactive.sh","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2010-08-13T17:20:02Z","receivedAt":"2010-08-13T17:20:02Z","isPatch":false,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"On Aug 13, 2010, at 12:37 PM, Ævar Arnfjörð Bjarmason wrote:\n\n> 39e388728 (merging git rebase -i exec support) broke the funny names\n> test in t3404-rebase-interactive.sh:\n\nJust noticed this myself when your e-mail hit the list.\n\n> This one breaks under bash too, does it work for you Matthieu? If so\n> what sort of environment are you executing it in?\n\nAlso broken here: OS X 10.6.4 using bash 3.2.48 and dash 0.5.6-20-ga92255d\n\n~~ Brian"},{"id":"148032","messageId":"vFgTzGXLhalxcMpLoOFhqltkPrQzeonuuQVYAuW79a2bfz1SRlvXGh7w8kLO_mBu9DM-e8Omq9k@cipher.nrlssc.navy.mil","threadId":"24731","inReplyTo":"D1A252AE-5D4C-4E51-9359-F4A443BB8A2E@silverinsanity.com","subject":"[PATCH 1/2] git-rebase--interactive.sh: rework skip_unnecessary_picks","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-08-13T20:47:34Z","receivedAt":"2010-08-13T20:47:34Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nCommit cd035b1c introduced the exec command to interactive rebase.  In\ndoing so, it modified the way that skip_unnecessary_picks iterates through\nthe list of rebase commands so that it avoided collapsing multiple spaces\ninto a single space.  This is necessary for example if the argument to the\nexec command contains a path with multiple spaces in it.\n\nThe way it did this was by reading each line of rebase commands into a\nsingle variable, and then breaking the individual components out using\necho, sed, and cut.  It used the individual broken-out components for\ndecision making, and was still able to write the original line to the\noutput file from the variable it had saved it in.  But, since we only\nreally need to look at anything other than the first element of the line\nwhen a 'pick' command is encountered, and even that is only necessary when\nwe are still searching for \"unnecessary\" picks, and since newer rebase\ncommands like 'exec' may not even require a sha1 field, let's make our read\nstatement parse its input into a \"command\" variable, and a \"rest\" variable,\nand then only break out the sha1 from $rest, and call git-rev-parse, when\nabsolutely necessary.\n\nI think this future proofs this subroutine, avoids calling git-rev-parse\nunnecessarily, and possibly with bogus arguments, and still accomplishes\nthe goal of not mangling the $rest of the rebase command.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nBrian Gernhardt wrote:\n> On Aug 13, 2010, at 12:37 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n>> 39e388728 (merging git rebase -i exec support) broke the funny names\n>> test in t3404-rebase-interactive.sh:\n> \n> Just noticed this myself when your e-mail hit the list.\n> \n>> This one breaks under bash too, does it work for you Matthieu? If so\n>> what sort of environment are you executing it in?\n> \n> Also broken here: OS X 10.6.4 using bash 3.2.48 and dash 0.5.6-20-ga92255d\n\nThe conversion to use printf instead of echo, introduced by 938791cd, was\nlost in the merge.  This first patch is just a cleanup that I think\nsimplifies and improves the code.  The second patch should fix the breakage.\n\n-Brandon\n\n\n git-rebase--interactive.sh |   23 ++++++++++++++---------\n 1 files changed, 14 insertions(+), 9 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex bf49b5b..2e5bed0 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -619,25 +619,30 @@ do_rest () {\n # skip picking commits whose parents are unchanged\n skip_unnecessary_picks () {\n \tfd=3\n-\twhile read -r line\n+\twhile read -r command rest\n \tdo\n-\t\tcommand=$(echo \"$line\" | sed 's/  */ /' | cut -d ' ' -f 1)\n-\t\tsha1=$(echo \"$line\"    | sed 's/  */ /' | cut -d ' ' -f 2)\n-\t\trest=$(echo \"$line\"    | sed 's/  */ /' | cut -d ' ' -f 3-)\n \t\t# fd=3 means we skip the command\n-\t\tcase \"$fd,$command,$(git rev-parse --verify --quiet \"$sha1\"^)\" in\n-\t\t3,pick,\"$ONTO\"*|3,p,\"$ONTO\"*)\n+\t\tcase \"$fd,$command\" in\n+\t\t3,pick|3,p)\n \t\t\t# pick a commit whose parent is current $ONTO -> skip\n-\t\t\tONTO=$sha1\n+\t\t\tsha1=$(echo \"$rest\" | cut -d ' ' -f 1)\n+\t\t\tcase \"$(git rev-parse --verify --quiet \"$sha1\"^)\" in\n+\t\t\t\"$ONTO\"*)\n+\t\t\t\tONTO=$sha1\n+\t\t\t\t;;\n+\t\t\t*)\n+\t\t\t\tfd=1\n+\t\t\t\t;;\n+\t\t\tesac\n \t\t\t;;\n-\t\t3,#*|3,,*)\n+\t\t3,#*|3,)\n \t\t\t# copy comments\n \t\t\t;;\n \t\t*)\n \t\t\tfd=1\n \t\t\t;;\n \t\tesac\n-\t\techo \"$line\" >&$fd\n+\t\techo \"$command${rest:+ }$rest\" >&$fd\n \tdone <\"$TODO\" >\"$TODO.new\" 3>>\"$DONE\" &&\n \tmv -f \"$TODO\".new \"$TODO\" &&\n \tcase \"$(peek_next_command)\" in\n-- \n1.7.2.1\n"},{"id":"148033","messageId":"vFgTzGXLhalxcMpLoOFhqi1W6sU5I3lJ9CWjrrJjoRmkMjHSswmpLXU2vVL8PS5JJNEO727l9q8@cipher.nrlssc.navy.mil","threadId":"24731","inReplyTo":"D1A252AE-5D4C-4E51-9359-F4A443BB8A2E@silverinsanity.com","subject":"[PATCH 2/2] git-rebase--interactive.sh: use printf instead of echo to print commit message","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-08-13T20:47:35Z","receivedAt":"2010-08-13T20:47:35Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nReplace the echo statements that operate on $rest with printf's to restore\nwhat was lost from 938791cd.  This avoids any mangling that XSI-conformant\necho's may introduce.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nJunio,\n\nFeel free to squash this into 1/2 if desired.\n\n-Brandon\n\n\n git-rebase--interactive.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 2e5bed0..3419247 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -625,7 +625,7 @@ skip_unnecessary_picks () {\n \t\tcase \"$fd,$command\" in\n \t\t3,pick|3,p)\n \t\t\t# pick a commit whose parent is current $ONTO -> skip\n-\t\t\tsha1=$(echo \"$rest\" | cut -d ' ' -f 1)\n+\t\t\tsha1=$(printf '%s' \"$rest\" | cut -d ' ' -f 1)\n \t\t\tcase \"$(git rev-parse --verify --quiet \"$sha1\"^)\" in\n \t\t\t\"$ONTO\"*)\n \t\t\t\tONTO=$sha1\n@@ -642,7 +642,7 @@ skip_unnecessary_picks () {\n \t\t\tfd=1\n \t\t\t;;\n \t\tesac\n-\t\techo \"$command${rest:+ }$rest\" >&$fd\n+\t\tprintf '%s\\n' \"$command${rest:+ }$rest\" >&$fd\n \tdone <\"$TODO\" >\"$TODO.new\" 3>>\"$DONE\" &&\n \tmv -f \"$TODO\".new \"$TODO\" &&\n \tcase \"$(peek_next_command)\" in\n-- \n1.7.2.1\n"},{"id":"148040","messageId":"4BE6DADC-6846-4942-B361-639DCD308F09@silverinsanity.com","threadId":"24731","inReplyTo":"vFgTzGXLhalxcMpLoOFhqi1W6sU5I3lJ9CWjrrJjoRmkMjHSswmpLXU2vVL8PS5JJNEO727l9q8@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 2/2] git-rebase--interactive.sh: use printf instead of echo to print commit message","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2010-08-13T21:34:07Z","receivedAt":"2010-08-13T21:34:07Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Aug 13, 2010, at 4:47 PM, Brandon Casey wrote:\n\n> Replace the echo statements that operate on $rest with printf's to restore\n> what was lost from 938791cd.  This avoids any mangling that XSI-conformant\n> echo's may introduce.\n\nACK, this was exactly the problem.  Thanks for the quick response!\n\n~~ Brian\n"},{"id":"148131","messageId":"vpqmxso45l6.fsf@bauges.imag.fr","threadId":"24731","inReplyTo":"4BE6DADC-6846-4942-B361-639DCD308F09@silverinsanity.com","subject":"Re: [PATCH 2/2] git-rebase--interactive.sh: use printf instead of echo to print commit message","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-15T08:56:21Z","receivedAt":"2010-08-15T08:56:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Brian Gernhardt <benji@silverinsanity.com> writes:\n\n> On Aug 13, 2010, at 4:47 PM, Brandon Casey wrote:\n>\n>> Replace the echo statements that operate on $rest with printf's to restore\n>> what was lost from 938791cd.  This avoids any mangling that XSI-conformant\n>> echo's may introduce.\n>\n> ACK, this was exactly the problem.  Thanks for the quick response!\n\nBoth patches sound good, yes.\n\nJunio: I think you can either apply them on top of mine, or squash\nthem all together.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"148698","messageId":"7vd3tb6rhp.fsf@alter.siamese.dyndns.org","threadId":"24731","inReplyTo":"vFgTzGXLhalxcMpLoOFhqi1W6sU5I3lJ9CWjrrJjoRmkMjHSswmpLXU2vVL8PS5JJNEO727l9q8@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 2/2] git-rebase--interactive.sh: use printf instead of echo to print commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-22T07:22:58Z","receivedAt":"2010-08-22T07:22:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Both makes sense; thanks.\n"}]}