Re: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping
Martin von Zweigbergk wrote: > Normally one would break if unsuccessful. What would fail if this was > replaced by "do_next || break" and the above ".. && return 1" was ".. > && return". I assume that was your first attempt, but why did it not > work? Thanks. This was a major thinko on my part, but I don't remember exactly why I got confused. I'll explain this patch properly in the next iteration. -- 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: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping
Ramkumar Ramachandra writes:
> On a successful interactive rebase, git-rebase--interactive.sh
> currently cleans up and exits on its own. Instead of doing these
> two things ourselves:
>
> rm -fr "$dotest"
> git gc --auto
>
> let us return control to the caller (git-rebase.sh), to do the
> needful. The advantage of doing this is that the caller can implement
> a generic cleanup routine (and possibly other things) independent of
> specific rebases.
>
> Signed-off-by: Ramkumar Ramachandra
> ---
And this answers the question in my review for [4/7]. It would make
sense to have these two patch subseries asn three patches (prepare
git-rebase.sh, and then convert each backends separately), or a
single patch; two patches like this does not make much sense to me.
> git-rebase--interactive.sh | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index cc3a9a7..9514e31 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -597,7 +597,7 @@ do_next () {
> fi
> ;;
> esac
> - test -s "$todo" && return
> + test -s "$todo" && return 1
>
> comment_for_reflog finish &&
> newhead=$(git rev-parse HEAD) &&
> @@ -623,17 +623,15 @@ do_next () {
> "$GIT_DIR"/hooks/post-rewrite rebase < "$rewritten_list"
> true # we don't care if this hook failed
> fi &&
> - rm -rf "$state_dir" &&
> - git gc --auto &&
> warn "Successfully rebased and updated $head_name."
>
> - exit
> + return 0
> }
>
> do_rest () {
> while :
> do
> - do_next
> + do_next && break
> done
> }
This is somewhat suspicious. We used to die when do_next failed, or
let do_next exit with success.
But now you let do_rest return (what does it return???)...
>
> @@ -799,12 +797,12 @@ first and then run 'git rebase --continue' again."
> record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
>
> require_clean_work_tree "rebase"
> - do_rest
> + do_rest && return 0
... and its caller reports success here only when it succeeds. What
happens do_rest returns a failure?
> ;;
> skip)
> git rerere clear
>
> - do_rest
> + do_rest && return 0
> ;;
> edit-todo)
> git stripspace --strip-comments <"$todo" >"$todo".new
--
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: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping
On Tue, Apr 23, 2013 at 7:02 AM, Ramkumar Ramachandra
wrote:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index cc3a9a7..9514e31 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -597,7 +597,7 @@ do_next () {
> fi
> ;;
> esac
> - test -s "$todo" && return
> + test -s "$todo" && return 1
Unlike the other cases below, this seems to be replacing success by
failure. What does it mean in practice that $todo is empty?
> comment_for_reflog finish &&
> newhead=$(git rev-parse HEAD) &&
> @@ -623,17 +623,15 @@ do_next () {
> "$GIT_DIR"/hooks/post-rewrite rebase < "$rewritten_list"
> true # we don't care if this hook failed
> fi &&
> - rm -rf "$state_dir" &&
> - git gc --auto &&
> warn "Successfully rebased and updated $head_name."
>
> - exit
> + return 0
So after this patch, the "warning" will coming before gc is run. It's
a change, but it seems fine. gc usually only prints a few line, right?
> }
>
> do_rest () {
> while :
> do
> - do_next
> + do_next && break
> done
> }
Normally one would break if unsuccessful. What would fail if this was
replaced by "do_next || break" and the above ".. && return 1" was "..
&& return". I assume that was your first attempt, but why did it not
work?
--
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
[PATCH 5/7] rebase -i: return control to the caller, for housekeeping
On a successful interactive rebase, git-rebase--interactive.sh
currently cleans up and exits on its own. Instead of doing these
two things ourselves:
rm -fr "$dotest"
git gc --auto
let us return control to the caller (git-rebase.sh), to do the
needful. The advantage of doing this is that the caller can implement
a generic cleanup routine (and possibly other things) independent of
specific rebases.
Signed-off-by: Ramkumar Ramachandra
---
git-rebase--interactive.sh | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index cc3a9a7..9514e31 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -597,7 +597,7 @@ do_next () {
fi
;;
esac
- test -s "$todo" && return
+ test -s "$todo" && return 1
comment_for_reflog finish &&
newhead=$(git rev-parse HEAD) &&
@@ -623,17 +623,15 @@ do_next () {
"$GIT_DIR"/hooks/post-rewrite rebase < "$rewritten_list"
true # we don't care if this hook failed
fi &&
- rm -rf "$state_dir" &&
- git gc --auto &&
warn "Successfully rebased and updated $head_name."
- exit
+ return 0
}
do_rest () {
while :
do
- do_next
+ do_next && break
done
}
@@ -799,12 +797,12 @@ first and then run 'git rebase --continue' again."
record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
require_clean_work_tree "rebase"
- do_rest
+ do_rest && return 0
;;
skip)
git rerere clear
- do_rest
+ do_rest && return 0
;;
edit-todo)
git stripspace --strip-comments <"$todo" >"$todo".new
--
1.8.2.1.578.ga933817
--
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
