{"thread":{"id":"34887","subject":"[PATCH] rebase: fix run_specific_rebase's use of \"return\" on FreeBSD","startedAt":"2013-09-09T08:53:15Z","lastAt":"2013-09-09T15:44:59Z","messageCount":3,"participants":["Matthieu Moy","Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"227205","messageId":"1378716795-5420-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"34887","inReplyTo":null,"subject":"[PATCH] rebase: fix run_specific_rebase's use of \"return\" on FreeBSD","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-09-09T08:53:15Z","receivedAt":"2013-09-09T08:53:15Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Since a1549e10, git-rebase--am.sh uses the shell's \"return\" statement, to\nmean \"return from the current file inclusion\", which is POSIXly correct,\nbut badly interpreted on FreeBSD, which returns from the current\nfunction, hence skips the finish_rebase statement that follows the file\ninclusion.\n\nMake the use of \"return\" portable by using the file inclusion as the last\nstatement of a function.\n\nReported-by: Christoph Mallon <christoph.mallon@gmx.de>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nI sent the patch inline in a conversation and got no feedback.\nResending as a proper patch.\n\n git-rebase.sh | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 8d7659a..226752f 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -167,13 +167,22 @@ You can run \"git stash pop\" or \"git stash drop\" at any time.\n \trm -rf \"$state_dir\"\n }\n \n-run_specific_rebase () {\n+run_specific_rebase_internal () {\n \tif [ \"$interactive_rebase\" = implied ]; then\n \t\tGIT_EDITOR=:\n \t\texport GIT_EDITOR\n \t\tautosquash=\n \tfi\n+\t# On FreeBSD, the shell's \"return\" returns from the current\n+\t# function, not from the current file inclusion.\n+\t# run_specific_rebase_internal has the file inclusion as a\n+\t# last statement, so POSIX and FreeBSD's return will do the\n+\t# same thing.\n \t. git-rebase--$type\n+}\n+\n+run_specific_rebase () {\n+\trun_specific_rebase_internal\n \tret=$?\n \tif test $ret -eq 0\n \tthen\n-- \n1.8.4.5.g8688bea\n"},{"id":"227206","messageId":"CALkWK0nbcCuEfhagT4aUzqX8iynbhUvkRO9asmRHSGmEPRYZiA@mail.gmail.com","threadId":"34887","inReplyTo":"1378716795-5420-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] rebase: fix run_specific_rebase's use of \"return\" on FreeBSD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-09-09T09:00:17Z","receivedAt":"2013-09-09T09:00:17Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Matthieu Moy wrote:\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index 8d7659a..226752f 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -167,13 +167,22 @@ You can run \"git stash pop\" or \"git stash drop\" at any time.\n>         rm -rf \"$state_dir\"\n>  }\n>\n> -run_specific_rebase () {\n> +run_specific_rebase_internal () {\n>         if [ \"$interactive_rebase\" = implied ]; then\n>                 GIT_EDITOR=:\n>                 export GIT_EDITOR\n>                 autosquash=\n>         fi\n> +       # On FreeBSD, the shell's \"return\" returns from the current\n> +       # function, not from the current file inclusion.\n> +       # run_specific_rebase_internal has the file inclusion as a\n> +       # last statement, so POSIX and FreeBSD's return will do the\n> +       # same thing.\n>         . git-rebase--$type\n> +}\n\nMost excellent. Thanks for fixing this.\n"},{"id":"227234","messageId":"xmqqzjrm3sp0.fsf@gitster.dls.corp.google.com","threadId":"34887","inReplyTo":"1378716795-5420-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] rebase: fix run_specific_rebase's use of \"return\" on FreeBSD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-09T15:44:59Z","receivedAt":"2013-09-09T15:44:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Since a1549e10, git-rebase--am.sh uses the shell's \"return\" statement, to\n> mean \"return from the current file inclusion\", which is POSIXly correct,\n> but badly interpreted on FreeBSD, which returns from the current\n> function, hence skips the finish_rebase statement that follows the file\n> inclusion.\n>\n> Make the use of \"return\" portable by using the file inclusion as the last\n> statement of a function.\n>\n> Reported-by: Christoph Mallon <christoph.mallon@gmx.de>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> I sent the patch inline in a conversation and got no feedback.\n> Resending as a proper patch.\n\nI'll replace mm/rebase-continue-freebsd-WB with this version, but it\nwould be nice to hear from FreeBSD folks that this is sufficient for\ntheir platform to work around the issue (e.g. there could be other\n\"return\"s unfixed outside the codepath this patch fixes).\n\nThanks.\n\n>  git-rebase.sh | 11 ++++++++++-\n>  1 file changed, 10 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index 8d7659a..226752f 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -167,13 +167,22 @@ You can run \"git stash pop\" or \"git stash drop\" at any time.\n>  \trm -rf \"$state_dir\"\n>  }\n>  \n> -run_specific_rebase () {\n> +run_specific_rebase_internal () {\n>  \tif [ \"$interactive_rebase\" = implied ]; then\n>  \t\tGIT_EDITOR=:\n>  \t\texport GIT_EDITOR\n>  \t\tautosquash=\n>  \tfi\n> +\t# On FreeBSD, the shell's \"return\" returns from the current\n> +\t# function, not from the current file inclusion.\n> +\t# run_specific_rebase_internal has the file inclusion as a\n> +\t# last statement, so POSIX and FreeBSD's return will do the\n> +\t# same thing.\n>  \t. git-rebase--$type\n> +}\n> +\n> +run_specific_rebase () {\n> +\trun_specific_rebase_internal\n>  \tret=$?\n>  \tif test $ret -eq 0\n>  \tthen\n"}]}