From: Junio C Hamano <gitster@pobox.com>
To: Andrew Wong <andrew.kw.w@gmail.com>
Cc: git@vger.kernel.org, alex.kostikov@gmail.com
Subject: Re: [RFC] rebase: Handle cases where format-patch fails
Date: Mon, 08 Oct 2012 15:38:35 -0700 [thread overview]
Message-ID: <7vtxu4io7o.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1349724988-14625-2-git-send-email-andrew.kw.w@gmail.com> (Andrew Wong's message of "Mon, 8 Oct 2012 15:36:28 -0400")
Andrew Wong <andrew.kw.w@gmail.com> writes:
> 'format-patch' could fail due to reasons such as out of memory. Such
> failures are not detected or handled, which causes rebase to incorrectly
> think that it completed successfully and continue with cleanup. i.e.
> calling move_to_original_branch
>
> Instead of using a pipe, we separate 'format-patch' and 'am' by using an
> intermediate file. This gurantees that we can invoke 'am' with the
> complete input, or not invoking 'am' at all if 'format-patch' failed.
>
> Also print messages to help user with how to recover from such failures.
>
> Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
> ---
> git-rebase--am.sh | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 392ebc9..a955b38 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -26,10 +26,32 @@ then
> # makes this easy
> git cherry-pick --allow-empty "$revisions"
> else
> - git format-patch -k --stdout --full-index --ignore-if-in-upstream \
> + rm -f "$GIT_DIR/format-patch"
> + if ! git format-patch -k --stdout --full-index --ignore-if-in-upstream \
> --src-prefix=a/ --dst-prefix=b/ \
> - --no-renames $root_flag "$revisions" |
> - git am $git_am_opt --rebasing --resolvemsg="$resolvemsg"
> + --no-renames $root_flag "$revisions" > "$GIT_DIR/format-patch" && ret=$?
> + then
Is it just me? I find this construct
if ! cmd && ret=$?
then
very hard to wrap my mind around. Why not
git format-patch ... just as before ... \
... >"$GIT_DIR/formatted-patches" || {
# error handling or advices come here...
rm -f "$GIT_DIR/formatted-patches"
exit 1
}
git am ... just as before ... "$GIT_DIR/formatted-patches" || {
# possibly another error handling or advices come here...
rm -f "$GIT_DIR/formatted-patches"
exit 1
}
without changing anything else?
> + rm "$GIT_DIR/format-patch"
> + echo
> + echo "'git format-patch' seems to have failed."
> + echo "It is impossible to continue or abort rebasing."
> + echo "You have to use the following to return to your original head:"
> + echo
> + case "$head_name" in
> + refs/*)
> + echo " git checkout $head_name"
> + ;;
> + *)
> + echo " git checkout $orig_head"
> + ;;
> + esac
You _know_ format-patch failed, not just "seems to have", at this
point, no? Why is it impossible to abort?
What have we done before reaching to this point? We know we are
doing the basic "git rebase", without any funny "-m/-i/-p" business,
so the only thing we have done are (1) detached HEAD at the new
onto, (2) set ORIG_HEAD to point at the original tip of the branch
being rebased (or the commit we were sitting at, if we are rebasing
a detached history), and (3) head_name has the refname of the
original branch (or detached HEAD) and branch_name has the name of
the branch (or HEAD).
Shouldn't we be just rewinding what we have done so far and error
the whole thing out instead? Perhaps the first "# error handling or
advises come here..." part may simply be
case "$head_name" in
refs/heads/*)
git checkout "$head_name"
;;
*)
git checkout "$orig_head"
;;
esac
cat >&2 <<-\EOF
Error was found while preparing the patches ($revisions) to
replay on the rewound head. You cannot rebase this history.
EOF
or something like that. The format-patch output (and its error) may
be of interest in getting help going forward.
next prev parent reply other threads:[~2012-10-08 22:38 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-03 19:47 Rebase doesn't restore branch pointer back on out of memory Alexander Kostikov
2012-10-03 21:52 ` Andrew Wong
[not found] ` <CAGAhT3mVn-W5P-n_YeafZ_7bntkJGArJ3o6+dA5GO_H44=KHFg@mail.gmail.com>
2012-10-04 15:13 ` Andrew Wong
2012-10-04 21:09 ` Alexander Kostikov
2012-10-04 21:39 ` Alexander Kostikov
2012-10-04 22:52 ` Andrew Wong
2012-10-04 23:59 ` Alexander Kostikov
2012-10-05 4:53 ` Andrew Wong
2012-10-05 4:53 ` [RFC] rebase: Handle cases where format-patch fails Andrew Wong
2012-10-05 20:17 ` Junio C Hamano
2012-10-08 19:36 ` Andrew Wong
2012-10-08 19:36 ` Andrew Wong
2012-10-08 22:38 ` Junio C Hamano [this message]
2012-10-11 3:54 ` Rebase doesn't restore branch pointer back on out of memory Andrew Wong
2012-10-11 3:54 ` [PATCH] rebase: Handle cases where format-patch fails Andrew Wong
2012-10-19 21:49 ` Rebase doesn't restore branch pointer back on out of memory Alexander Kostikov
2012-10-19 22:24 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vtxu4io7o.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=alex.kostikov@gmail.com \
--cc=andrew.kw.w@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).