[RFC] rebase: Handle cases where format-patch fails
'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.k...@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 + 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 + echo + exit $ret + fi + + git am $git_am_opt --rebasing --resolvemsg=$resolvemsg $GIT_DIR/format-patch || ret=$? + rm -f $GIT_DIR/format-patch + test 0 != ret ( exit $ret ) fi move_to_original_branch ret=$? -- 1.8.0.rc0.19.ga19ab82.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] rebase: Handle cases where format-patch fails
Andrew Wong andrew.k...@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.k...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] rebase: Handle cases where format-patch fails
'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 Since only the exit status of the last command in the pipeline is available, we rely on || to detect whether 'format-patch' has failed. Also print messages to help user with how to recover from such failures. Signed-off-by: Andrew Wong andrew.k...@gmail.com --- git-rebase--am.sh | 37 +++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 392ebc9..8dae804 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -26,10 +26,43 @@ then # makes this easy git cherry-pick --allow-empty $revisions else - git format-patch -k --stdout --full-index --ignore-if-in-upstream \ + ( git format-patch -k --stdout --full-index --ignore-if-in-upstream \ --src-prefix=a/ --dst-prefix=b/ \ - --no-renames $root_flag $revisions | + --no-renames $root_flag $revisions || + echo $? $GIT_DIR/format-patch-failed ) | git am $git_am_opt --rebasing --resolvemsg=$resolvemsg + ret=$? + if test -f $GIT_DIR/format-patch-failed + then + ret=1 + rm -f $GIT_DIR/format-patch-failed + if test -d $state_dir + then + echo + echo 'git format-patch' seems to have failed in the middle of 'git am'. + echo If you continue rebasing, you will likely be losing some commits. + echo It is recommended that you abort rebasing by running: + echo + echo git rebase --abort + echo + else + echo + echo 'git format-patch' seems to have failed before 'git am' started. + 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 + echo + fi + fi + test 0 != $ret false fi move_to_original_branch ret=$? -- 1.8.0.rc0.18.gf84667d -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] rebase: Handle cases where format-patch fails
Andrew Wong andrew.k...@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 Since only the exit status of the last command in the pipeline is available, we rely on || to detect whether 'format-patch' has failed. Also print messages to help user with how to recover from such failures. Signed-off-by: Andrew Wong andrew.k...@gmail.com --- git-rebase--am.sh | 37 +++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 392ebc9..8dae804 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -26,10 +26,43 @@ then # makes this easy git cherry-pick --allow-empty $revisions else - git format-patch -k --stdout --full-index --ignore-if-in-upstream \ + ( git format-patch -k --stdout --full-index --ignore-if-in-upstream \ --src-prefix=a/ --dst-prefix=b/ \ - --no-renames $root_flag $revisions | + --no-renames $root_flag $revisions || + echo $? $GIT_DIR/format-patch-failed ) | Please make sure there is no marker-file that was leftover from previous invocation or whatever reason, e.g. rm -f $GIT_DIR/format-patch-failed ( git format-patch -k --stdout --full-index --ignore-if-in-upstream \ --src-prefix=a/ --dst-prefix=b/ \ --no-renames $root_flag $revisions || echo $? $GIT_DIR/format-patch-failed ) | git am $git_am_opt --rebasing --resolvemsg=$resolvemsg But when format-patch dies for whatever reason, it is likely that the partial output will cause am to barf on the last part of it (either missing patch text if it stops in the middle of commit log message, or corrupt patch if it stops in the middle of a hunk). It may make sense to make this all-or-none, i.e. when format-patch fails, you do not even start am, something like... rm -f $GIT_DIR/patch-input if ! git format-patch -k --stdout $GIT_DIR/patch-input \ --full-index --ignore-if-in-upstream \ --src-prefix=a/ --dst-prefix=b/ \ --no-renames $root_flag $revisions then ... format-patch barfed, here is how to deal with it... else git am $GIT_DIR/patch-input $git_am_opt ... fi rm -f $GIT_DIR/patch-input but I wonder what the performance implication would be for normal cases. + ret=$? + if test -f $GIT_DIR/format-patch-failed + then + ret=1 + rm -f $GIT_DIR/format-patch-failed + if test -d $state_dir + then + echo + echo 'git format-patch' seems to have failed in the middle of 'git am'. + echo If you continue rebasing, you will likely be losing some commits. + echo It is recommended that you abort rebasing by running: + echo + echo git rebase --abort + echo + else + echo + echo 'git format-patch' seems to have failed before 'git am' started. + 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 + echo + fi + fi + test 0 != $ret false fi move_to_original_branch ret=$? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
