git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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: Fri, 05 Oct 2012 13:17:55 -0700	[thread overview]
Message-ID: <7vipaou0zw.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1349412790-6087-2-git-send-email-andrew.kw.w@gmail.com> (Andrew Wong's message of "Fri, 5 Oct 2012 00:53:10 -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
>
> 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.kw.w@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=$?

  reply	other threads:[~2012-10-05 20:18 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 [this message]
2012-10-08 19:36                 ` Andrew Wong
2012-10-08 19:36                   ` Andrew Wong
2012-10-08 22:38                     ` Junio C Hamano
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=7vipaou0zw.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).