[PATCH 0/7] Introduce rebase.autostash

[PATCH 0/7] Introduce rebase.autostash

[Ramkumar Ramachandra]

Yes!

After struggling with shell for a couple of days, I finally managed to
achieve what I set out to do: implement a rebase.autostash without
leaking the autostash detail into specific rebases.  And I'm
absolutely elated with my approach.  Patches [1/7] to [6/7] make no
reference to autostash, and pretend to be nice by-the-way changes
unrelated to any new feature.  [7/7] suddenly introduces a little bit
of code in git-rebase.sh, and everything just starts working
magically.

Okay, now for the bad news.  I've mostly just worked backward from
tests, so I can't justify all my changs fully.  Also, there are no new
tests, because I don't know what to test: we pretty much have to run
the entire rebase testsuite with rebase.autostash turned on, right?

If you can't wait for a few weeks (reviews, iterations) and want to
use this immediately, just pull in the rebase.autostash branch from
gh:artagnon/git and enjoy.

Cheers.

Ramkumar Ramachandra (7):
  am: suppress error output from a conditional
  rebase -i: don't error out if $state_dir already exists
  am: tighten a conditional that checks for $dotest
  am: don't do housekeeping when rebasing
  rebase -i: return control to the caller, for housekeeping
  sh-setup: introduce require_clean_work_tree --quiet
  rebase: implement --[no-]autostash and rebase.autostash

 Documentation/config.txt     |  8 ++++++++
 Documentation/git-rebase.txt | 10 ++++++++++
 git-am.sh                    | 15 ++++++++++-----
 git-rebase--am.sh            |  8 ++++----
 git-rebase--interactive.sh   | 14 ++++++--------
 git-rebase.sh                | 41 ++++++++++++++++++++++++++++++++++++++++-
 git-sh-setup.sh              | 10 +++++++---
 7 files changed, 85 insertions(+), 21 deletions(-)

-- 
1.8.2.1.578.ga933817

[PATCH 1/7] am: suppress error output from a conditional

[Ramkumar Ramachandra]

When testing if the $dotest directory exists, and if $next is greater
than $last, the script currently executes `cat` on files that might
not exist.  So, suppress the error output from `cat`.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-am.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index c092855..88aa438 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -446,8 +446,8 @@ done
 # If the dotest directory exists, but we have finished applying all the
 # patches in them, clear it out.
 if test -d "$dotest" &&
-   last=$(cat "$dotest/last") &&
-   next=$(cat "$dotest/next") &&
+   last=$(cat "$dotest/last" 2>/dev/null) &&
+   next=$(cat "$dotest/next" 2>/dev/null) &&
    test $# != 0 &&
    test "$next" -gt "$last"
 then
-- 
1.8.2.1.578.ga933817

[PATCH 2/7] rebase -i: don't error out if $state_dir already exists

[Ramkumar Ramachandra]

Practically speaking, the only reason why a `mkdir $state_dir` would
fail is because $state_dir already exists.  There is no problem in
this case, and we can proceed as usual.  So, change the `mkdir` call
to `mkdir -p`, and strip the `|| die`.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-rebase--interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 048a140..cc3a9a7 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -837,7 +837,7 @@ then
 fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"
-mkdir "$state_dir" || die "Could not create temporary $state_dir"
+mkdir -p "$state_dir"
 
 : > "$state_dir"/interactive || die "Could not mark as interactive"
 write_basic_state
-- 
1.8.2.1.578.ga933817

[PATCH 3/7] am: tighten a conditional that checks for $dotest

[Ramkumar Ramachandra]

We currently assume that, if a $dotest directory exists, an am had
been called earlier.  This assumption might get our conditional to
match a stray $dotest directory created somewhere else, and result in
failures down the line. So, tighten the conditional by additionally
looking for the file $dotest/last.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-am.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index 88aa438..f4ef8fc 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -454,7 +454,7 @@ then
    rm -fr "$dotest"
 fi
 
-if test -d "$dotest"
+if test -d "$dotest" && test -f "$dotest/last"
 then
 	case "$#,$skip$resolved$abort" in
 	0,*t*)
-- 
1.8.2.1.578.ga933817

[PATCH 4/7] am: don't do housekeeping when rebasing

[Ramkumar Ramachandra]

Perform these two tasks:

    git gc --auto
    rm -fr "$dotest"

only when not called with --rebasing (from git-rebase--am.sh).
Otherwise, return control to the caller so that it can do the needful.
The advantage of doing this is that the caller can implement a generic
cleanup routine (that possibly does other things) independent of
specific rebases.

Patch the ultimate caller, git-rebase.sh, to perform these two steps
for the moment.  Later patches will add functionality.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-am.sh         | 9 +++++++--
 git-rebase--am.sh | 8 ++++----
 git-rebase.sh     | 7 +++++++
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index f4ef8fc..47c1021 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -904,5 +904,10 @@ if test -s "$dotest"/rewritten; then
     fi
 fi
 
-rm -fr "$dotest"
-git gc --auto
+# If am was called with --rebasing (from git-rebase--am), it's up to
+# the caller to take care of housekeeping.
+if ! test -f "$dotest/rebasing"
+then
+	rm -fr "$dotest"
+	git gc --auto
+fi
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index f84854f..8230094 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -7,12 +7,12 @@ case "$action" in
 continue)
 	git am --resolved --resolvemsg="$resolvemsg" &&
 	move_to_original_branch
-	exit
+	return $?
 	;;
 skip)
 	git am --skip --resolvemsg="$resolvemsg" &&
 	move_to_original_branch
-	exit
+	return $?
 	;;
 esac
 
@@ -56,7 +56,7 @@ else
 
 		As a result, git cannot rebase them.
 		EOF
-		exit $?
+		return $?
 	fi
 
 	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" <"$GIT_DIR/rebased-patches"
@@ -68,7 +68,7 @@ fi
 if test 0 != $ret
 then
 	test -d "$state_dir" && write_basic_state
-	exit $ret
+	return $ret
 fi
 
 move_to_original_branch
diff --git a/git-rebase.sh b/git-rebase.sh
index b2f1c76..8412d81 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -150,6 +150,13 @@ run_specific_rebase () {
 		autosquash=
 	fi
 	. git-rebase--$type
+	ret=$?
+	if test $ret = 0
+	then
+		git gc --auto &&
+		rm -rf "$state_dir"
+	fi
+	exit $ret
 }
 
 run_pre_rebase_hook () {
-- 
1.8.2.1.578.ga933817

[PATCH 5/7] rebase -i: return control to the caller, for housekeeping

[Ramkumar Ramachandra]

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 <artagnon@gmail.com>
---
 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

[PATCH 6/7] sh-setup: introduce require_clean_work_tree --quiet

[Ramkumar Ramachandra]

Some callers might want to know whether or not the worktree is clean,
and require_clean_work_tree() has the logic for this.  The current
implementation of the function prints a message and exits if the
worktree wasn't clean.  Introduce a --quiet switch to get it to report
the status of the worktree back to the caller.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-sh-setup.sh | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 2f78359..5fa22a8 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -172,6 +172,7 @@ require_clean_work_tree () {
 
 	if ! git diff-files --quiet --ignore-submodules
 	then
+		test "$1" = "--quiet" ||
 		echo >&2 "Cannot $1: You have unstaged changes."
 		err=1
 	fi
@@ -180,9 +181,11 @@ require_clean_work_tree () {
 	then
 		if [ $err = 0 ]
 		then
-		    echo >&2 "Cannot $1: Your index contains uncommitted changes."
+			test "$1" = "--quiet" ||
+			echo >&2 "Cannot $1: Your index contains uncommitted changes."
 		else
-		    echo >&2 "Additionally, your index contains uncommitted changes."
+			test "$1" = "--quiet" ||
+			echo >&2 "Additionally, your index contains uncommitted changes."
 		fi
 		err=1
 	fi
@@ -190,8 +193,9 @@ require_clean_work_tree () {
 	if [ $err = 1 ]
 	then
 		test -n "$2" && echo >&2 "$2"
-		exit 1
+		test "$1" = "--quiet" || exit 1
 	fi
+	return $err
 }
 
 # Generate a sed script to parse identities from a commit.
-- 
1.8.2.1.578.ga933817

[PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Ramkumar Ramachandra]

This new feature allows a rebase to be executed on a dirty worktree.
It works by creating a temporary stash and storing it in
$state_dir/autostash before the operation, and applying it after a
successful operation.  It will be removed along with the $state_dir if
the operation is aborted.

The feature creates a special stash that does not affect the normal
stash's reflogs, and will therefore be invisible to the end user.
This special stash is essentially a dangling merge commit which has
reasonable lifetime specified by gc.pruneexpire (default 2 weeks).

Most significantly, this feature means that a caller like pull (with
pull.rebase set to true) can easily be patched to remove the
require_clean_work_tree restriction.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/config.txt     |  8 ++++++++
 Documentation/git-rebase.txt | 10 ++++++++++
 git-rebase.sh                | 38 +++++++++++++++++++++++++++++++++++---
 3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c67038b..03ad701 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1867,6 +1867,14 @@ rebase.stat::
 rebase.autosquash::
 	If set to true enable '--autosquash' option by default.
 
+rebase.autostash::
+	When set to true, automatically create a temporary stash
+	before the operation begins, and apply it after the operation
+	ends.  This means that you can run rebase on a dirty worktree.
+	However, use with care: the final stash application after a
+	successful rebase might result in non-trivial conflicts.
+	Defaults to false.
+
 receive.autogc::
 	By default, git-receive-pack will run "git-gc --auto" after
 	receiving data from git-push and updating refs.  You can stop
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index aca8405..c84854a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -208,6 +208,9 @@ rebase.stat::
 rebase.autosquash::
 	If set to true enable '--autosquash' option by default.
 
+rebase.autostash::
+	If set to true enable '--autostash' option by default.
+
 OPTIONS
 -------
 --onto <newbase>::
@@ -394,6 +397,13 @@ If the '--autosquash' option is enabled by default using the
 configuration variable `rebase.autosquash`, this option can be
 used to override and disable this setting.
 
+--[no-]autostash::
+	Automatically create a temporary stash before the operation
+	begins, and apply it after the operation ends.  This means
+	that you can run rebase on a dirty worktree.  However, use
+	with care: the final stash application after a successful
+	rebase might result in non-trivial conflicts.
+
 --no-ff::
 	With --interactive, cherry-pick all rebased commits instead of
 	fast-forwarding over the unchanged ones.  This ensures that the
diff --git a/git-rebase.sh b/git-rebase.sh
index 8412d81..c8fddfe 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -13,6 +13,7 @@ git-rebase --continue | --abort | --skip | --edit-todo
  Available options are
 v,verbose!         display a diffstat of what changed upstream
 q,quiet!           be quiet. implies --no-stat
+autostash!         automatically stash/stash pop before and after
 onto=!             rebase onto given branch instead of upstream
 p,preserve-merges! try to recreate merges instead of ignoring them
 s,strategy=!       use the given merge strategy
@@ -64,6 +65,7 @@ apply_dir="$GIT_DIR"/rebase-apply
 verbose=
 diffstat=
 test "$(git config --bool rebase.stat)" = true && diffstat=t
+autostash="$(git config --bool rebase.autostash || echo false)"
 git_am_opt=
 rebase_root=
 force_rebase=
@@ -143,6 +145,24 @@ move_to_original_branch () {
 	esac
 }
 
+apply_autostash () {
+	if test -f "$state_dir/autostash"
+	then
+		stash_sha1=$(cat "$state_dir/autostash")
+		git stash apply $stash_sha1 2>&1 >/dev/null ||
+		die "
+$(eval_gettext 'Applying autostash resulted in conflicts.
+Either fix the conflicts now, or run
+	git reset --hard
+and apply the stash on your desired branch:
+	git stash apply $stash_sha1
+at any time.')" &&
+		echo "Applied autostash"
+	fi
+	git gc --auto &&
+	rm -rf "$state_dir"
+}
+
 run_specific_rebase () {
 	if [ "$interactive_rebase" = implied ]; then
 		GIT_EDITOR=:
@@ -153,8 +173,7 @@ run_specific_rebase () {
 	ret=$?
 	if test $ret = 0
 	then
-		git gc --auto &&
-		rm -rf "$state_dir"
+		apply_autostash
 	fi
 	exit $ret
 }
@@ -248,6 +267,9 @@ do
 	--stat)
 		diffstat=t
 		;;
+	--autostash)
+		autostash=true
+		;;
 	-v)
 		verbose=t
 		diffstat=t
@@ -348,7 +370,7 @@ abort)
 		;;
 	esac
 	output git reset --hard $orig_head
-	rm -r "$state_dir"
+	apply_autostash
 	exit
 	;;
 edit-todo)
@@ -487,6 +509,16 @@ case "$#" in
 	;;
 esac
 
+if test "$autostash" = true && ! require_clean_work_tree --quiet
+then
+	stash_sha1=$(git stash create) || die "$(gettext "Cannot autostash")" &&
+	mkdir -p "$state_dir" &&
+	echo $stash_sha1 >"$state_dir/autostash" &&
+	stash_abbrev=$(git rev-parse --short $stash_sha1) &&
+	echo "$(gettext "Created autostash: $stash_abbrev")" &&
+	git reset --hard
+fi
+
 require_clean_work_tree "rebase" "$(gettext "Please commit or stash them.")"
 
 # Now we are rebasing commits $upstream..$orig_head (or with --root,
-- 
1.8.2.1.578.ga933817

Re: [PATCH 1/7] am: suppress error output from a conditional

[Martin von Zweigbergk]

On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> When testing if the $dotest directory exists, and if $next is greater
> than $last

When can that happen? If one edits the todo?

Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists

[Martin von Zweigbergk]

On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Practically speaking, the only reason why a `mkdir $state_dir` would
> fail is because $state_dir already exists.

Would we ever get to this point in the code if it already exists?

Also, I had the feeling that the check it might fail if the user does
not have permissions to create the directory. Is there always
something else that will fail first?

Re: [PATCH 1/7] am: suppress error output from a conditional

[Junio C Hamano]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra
> <artagnon@gmail.com> wrote:
>> When testing if the $dotest directory exists, and if $next is greater
>> than $last
>
> When can that happen? If one edits the todo?

More importantly, that condition is an unexpected error, which the
user may need to supply to help diagnosing the issue better, isn't
it?

Re: [PATCH 1/7] am: suppress error output from a conditional

[Ramkumar Ramachandra]

Martin von Zweigbergk wrote:
> On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra
> <artagnon@gmail.com> wrote:
>> When testing if the $dotest directory exists, and if $next is greater
>> than $last
>
> When can that happen? If one edits the todo?

When git-rebase.sh creates a $dotest directory with just an autostash file :)

Like I said in my cover letter, I really wrote [7/7] first, and kept
modifying things until all the tests passed.  So, I can't really
justify these changes independent of [7/7].

Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Practically speaking, the only reason why a `mkdir $state_dir` would
> fail is because $state_dir already exists.  There is no problem in
> this case, and we can proceed as usual.

That was started as a cheap way to detect and avoid running the same
rebase -i while another one is already in progress, way back when we
did not have a separate "git-rebase--*backend*" scripts, I think.

If we have a separate safety measure to prevent it, lifting it may
be safe, but in a sane case, $state_dir should _not_ exist when we
start the command, and mkdir should _not_ fail to create it.

So I do not see why this change could be an improvement.  I would
understand it if some future changes may have already created the
state dir from a separate codepath (which may be an indication of a
poor design of integrating that separate codepath, but that is a
different matter) and mkdir needs to be turned into "mkdir -p" here,
but I do not think removal of "|| die" is warranted even in such a
case.

>  So, change the `mkdir` call
> to `mkdir -p`, and strip the `|| die`.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  git-rebase--interactive.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 048a140..cc3a9a7 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -837,7 +837,7 @@ then
>  fi
>  
>  orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"
> -mkdir "$state_dir" || die "Could not create temporary $state_dir"
> +mkdir -p "$state_dir"
>  
>  : > "$state_dir"/interactive || die "Could not mark as interactive"
>  write_basic_state

Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> So I do not see why this change could be an improvement.  I would
> understand it if some future changes may have already created the
> state dir from a separate codepath (which may be an indication of a
> poor design of integrating that separate codepath, but that is a
> different matter) and mkdir needs to be turned into "mkdir -p" here,
> but I do not think removal of "|| die" is warranted even in such a
> case.

Yeah, it's [7/7] creating a $state_dir/autostash before anything else
can happen.  Okay, if I put the "|| die" back in, does it require any
more justification?

Re: [PATCH 3/7] am: tighten a conditional that checks for $dotest

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> We currently assume that, if a $dotest directory exists, an am had
> been called earlier.  This assumption might get our conditional to
> match a stray $dotest directory created somewhere else,...

Hmm. that explanation sounds like that is sweeping a real issue
under the rug.  Shouldn't your patch fixing that creator of the
stray one?  That stray one that errorneously creates $dotest, even
when it is not applying patch messages, may create the directory
with 'last' but without 'next' or the messages, so checking only
'last' does not sound like solving any problem.

If this were done as a part of an addition that has to create
$dotest even when it does not (yet) deposit patches in there, the
way that particular addition creates and uses $dotest may justify
why testing only 'last' is sufficient.  But as a standalone change,
neither the patch text or the above explanation makes much sense to
me.

> diff --git a/git-am.sh b/git-am.sh
> index 88aa438..f4ef8fc 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -454,7 +454,7 @@ then
>     rm -fr "$dotest"
>  fi
>  
> -if test -d "$dotest"
> +if test -d "$dotest" && test -f "$dotest/last"

>  then
>  	case "$#,$skip$resolved$abort" in
>  	0,*t*)

Re: [PATCH 4/7] am: don't do housekeeping when rebasing

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Perform these two tasks:
>
>     git gc --auto
>     rm -fr "$dotest"
>
> only when not called with --rebasing (from git-rebase--am.sh).
> Otherwise, return control to the caller so that it can do the needful.
> The advantage of doing this is that the caller can implement a generic
> cleanup routine (that possibly does other things) independent of
> specific rebases.

The above justification and the patch text make a lot of sense, but
they make me wonder why there is no corresponding removal from the
other git-rebase--*backend*.

Unconditional changes from "exit" to "return $?" looked somewhat
suspicious, but this is a sourced script so it should be safe.

>  git-am.sh         | 9 +++++++--
>  git-rebase--am.sh | 8 ++++----
>  git-rebase.sh     | 7 +++++++
>  3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index f4ef8fc..47c1021 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -904,5 +904,10 @@ if test -s "$dotest"/rewritten; then
>      fi
>  fi
>  
> -rm -fr "$dotest"
> -git gc --auto
> +# If am was called with --rebasing (from git-rebase--am), it's up to
> +# the caller to take care of housekeeping.
> +if ! test -f "$dotest/rebasing"
> +then
> +	rm -fr "$dotest"
> +	git gc --auto
> +fi
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index f84854f..8230094 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -7,12 +7,12 @@ case "$action" in
>  continue)
>  	git am --resolved --resolvemsg="$resolvemsg" &&
>  	move_to_original_branch
> -	exit
> +	return $?
>  	;;
>  skip)
>  	git am --skip --resolvemsg="$resolvemsg" &&
>  	move_to_original_branch
> -	exit
> +	return $?
>  	;;
>  esac
>  
> @@ -56,7 +56,7 @@ else
>  
>  		As a result, git cannot rebase them.
>  		EOF
> -		exit $?
> +		return $?
>  	fi
>  
>  	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" <"$GIT_DIR/rebased-patches"
> @@ -68,7 +68,7 @@ fi
>  if test 0 != $ret
>  then
>  	test -d "$state_dir" && write_basic_state
> -	exit $ret
> +	return $ret
>  fi
>  
>  move_to_original_branch
> diff --git a/git-rebase.sh b/git-rebase.sh
> index b2f1c76..8412d81 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -150,6 +150,13 @@ run_specific_rebase () {
>  		autosquash=
>  	fi
>  	. git-rebase--$type
> +	ret=$?
> +	if test $ret = 0
> +	then
> +		git gc --auto &&
> +		rm -rf "$state_dir"
> +	fi
> +	exit $ret
>  }
>  
>  run_pre_rebase_hook () {

Re: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping

[Martin von Zweigbergk]

On Tue, Apr 23, 2013 at 7:02 AM, Ramkumar Ramachandra
<artagnon@gmail.com> 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?

Re: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> 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 <artagnon@gmail.com>
> ---

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

Re: [PATCH 6/7] sh-setup: introduce require_clean_work_tree --quiet

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Some callers might want to know whether or not the worktree is clean,
> and require_clean_work_tree() has the logic for this.  The current
> implementation of the function prints a message and exits if the
> worktree wasn't clean.  Introduce a --quiet switch to get it to report
> the status of the worktree back to the caller.

This makes 75% sense, but I find it an extremely bad taste to tie
exit vs return with --quiet.  They are orthogonal and unrelated
concepts.

Doing this

	if (require_clean_work_tree) 2>/dev/null
        then
		: happy, the working tree is clean
	else
		... let's stash ...
	fi

without changing anything else may be better than adding such a
conflating --quiet option.

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  git-sh-setup.sh | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 2f78359..5fa22a8 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -172,6 +172,7 @@ require_clean_work_tree () {
>  
>  	if ! git diff-files --quiet --ignore-submodules
>  	then
> +		test "$1" = "--quiet" ||
>  		echo >&2 "Cannot $1: You have unstaged changes."
>  		err=1
>  	fi
> @@ -180,9 +181,11 @@ require_clean_work_tree () {
>  	then
>  		if [ $err = 0 ]
>  		then
> -		    echo >&2 "Cannot $1: Your index contains uncommitted changes."
> +			test "$1" = "--quiet" ||
> +			echo >&2 "Cannot $1: Your index contains uncommitted changes."
>  		else
> -		    echo >&2 "Additionally, your index contains uncommitted changes."
> +			test "$1" = "--quiet" ||
> +			echo >&2 "Additionally, your index contains uncommitted changes."
>  		fi
>  		err=1
>  	fi
> @@ -190,8 +193,9 @@ require_clean_work_tree () {
>  	if [ $err = 1 ]
>  	then
>  		test -n "$2" && echo >&2 "$2"
> -		exit 1
> +		test "$1" = "--quiet" || exit 1
>  	fi
> +	return $err
>  }
>  
>  # Generate a sed script to parse identities from a commit.

Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> +apply_autostash () {
> +	if test -f "$state_dir/autostash"
> +	then
> +		stash_sha1=$(cat "$state_dir/autostash")
> +		git stash apply $stash_sha1 2>&1 >/dev/null ||
> +		die "
> +$(eval_gettext 'Applying autostash resulted in conflicts.
> +Either fix the conflicts now, or run
> +	git reset --hard
> +and apply the stash on your desired branch:
> +	git stash apply $stash_sha1
> +at any time.')" &&
> +		echo "Applied autostash"

That && looks funny.  What does it even mean for die to succeed and
give control back to you for a chance to say "Applied"?

	stash apply || die
	echo applied

would be far more logical.

> +	fi
> +	git gc --auto &&
> +	rm -rf "$state_dir"
> +}

This is somewhat worrysome.  After getting the "oops, we cannot
apply" message, the "$stash_sha1" in the message on the terminal is
the ONLY indication of where the (presumably precious) local change
the user used to have can be recovered from.

You do not "rm -fr $state_dir" in such a case, so perhaps telling
the user the location of that "autostash" file may help her
somewhat.

For that matter, wouldn't it be a lot simpler to put the autostash
ref somewhere in refs/ hierarchy, instead of storing an object name
of the stash that keeps (presumably precious) local change of the
user in a plain-text file that is not at all known by "gc"?

> +
>  run_specific_rebase () {
>  	if [ "$interactive_rebase" = implied ]; then
>  		GIT_EDITOR=:
> @@ -153,8 +173,7 @@ run_specific_rebase () {
>  	ret=$?
>  	if test $ret = 0
>  	then
> -		git gc --auto &&
> -		rm -rf "$state_dir"
> +		apply_autostash
>  	fi
>  	exit $ret
>  }
> @@ -248,6 +267,9 @@ do
>  	--stat)
>  		diffstat=t
>  		;;
> +	--autostash)
> +		autostash=true
> +		;;
>  	-v)
>  		verbose=t
>  		diffstat=t
> @@ -348,7 +370,7 @@ abort)
>  		;;
>  	esac
>  	output git reset --hard $orig_head
> -	rm -r "$state_dir"
> +	apply_autostash
>  	exit
>  	;;
>  edit-todo)
> @@ -487,6 +509,16 @@ case "$#" in
>  	;;
>  esac
>  
> +if test "$autostash" = true && ! require_clean_work_tree --quiet
> +then
> +	stash_sha1=$(git stash create) || die "$(gettext "Cannot autostash")" &&
> +	mkdir -p "$state_dir" &&
> +	echo $stash_sha1 >"$state_dir/autostash" &&
> +	stash_abbrev=$(git rev-parse --short $stash_sha1) &&
> +	echo "$(gettext "Created autostash: $stash_abbrev")" &&
> +	git reset --hard
> +fi
> +
>  require_clean_work_tree "rebase" "$(gettext "Please commit or stash them.")"
>  
>  # Now we are rebasing commits $upstream..$orig_head (or with --root,

Re: [PATCH 1/7] am: suppress error output from a conditional

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Martin von Zweigbergk wrote:
>> On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra
>> <artagnon@gmail.com> wrote:
>>> When testing if the $dotest directory exists, and if $next is greater
>>> than $last
>>
>> When can that happen? If one edits the todo?
>
> When git-rebase.sh creates a $dotest directory with just an autostash file :)
>
> Like I said in my cover letter, I really wrote [7/7] first, and kept
> modifying things until all the tests passed.  So, I can't really
> justify these changes independent of [7/7].

That explains a lot of mystery ;-)

Perhaps all of these "oops, 7/7 breaks this and that so let's work
them around" can be avoided if the series did not store the object
name of the stash that records the local changes as a plain text
file inside a $dotest/ directory, and instead used a dedicated ref
somewhere under refs/ hierarchy, no?

Re: [PATCH 1/7] am: suppress error output from a conditional

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Perhaps all of these "oops, 7/7 breaks this and that so let's work
> them around" can be avoided if the series did not store the object
> name of the stash that records the local changes as a plain text
> file inside a $dotest/ directory, and instead used a dedicated ref
> somewhere under refs/ hierarchy, no?

Exactly.  Which is why I was asking for named stashes on the other
thread.  Much cleaner than keeping track of it yourself.

Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> +apply_autostash () {
>> +	if test -f "$state_dir/autostash"
>> +	then
>> +		stash_sha1=$(cat "$state_dir/autostash")
>> +		git stash apply $stash_sha1 2>&1 >/dev/null ||
>> +		die "
>> +$(eval_gettext 'Applying autostash resulted in conflicts.
>> +Either fix the conflicts now, or run
>> +	git reset --hard
>> +and apply the stash on your desired branch:
>> +	git stash apply $stash_sha1
>> +at any time.')" &&
>> +		echo "Applied autostash"
>
> That && looks funny.  What does it even mean for die to succeed and
> give control back to you for a chance to say "Applied"?
>
> 	stash apply || die
> 	echo applied
>
> would be far more logical.
>
>> +	fi
>> +	git gc --auto &&
>> +	rm -rf "$state_dir"
>> +}
>
> This is somewhat worrisome.

One more thing on this function.  [4/7] (and [5/7]) justified nicely
why it is a good idea to have a central place to do the clean-up
tasks.  apply_autostash is a poor name for that "clean-up" function.
The central clean-up may happen to involve applying a stash in this
version, but applying stash will not stay the entirety of the
clean-up work forever.  The entirety of the clean-up work used to be
just 'git gc --auto && rm -fr "$state_dir"' for eternity, and this
series is adding something else. It is not hard to imagine somebody
else would want to add other kinds of clean-up tasks in here.

Perhaps "finish_rebase" or something?

Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Perhaps "finish_rebase" or something?

Sure.  That makes sense.

I was too busy struggling with shell code to notice these things ;)

Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Phil Hord]

On Tue, Apr 23, 2013 at 10:02 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> This new feature allows a rebase to be executed on a dirty worktree.
> It works by creating a temporary stash and storing it in
> $state_dir/autostash before the operation, and applying it after a
> successful operation.  It will be removed along with the $state_dir if
> the operation is aborted.
>
> The feature creates a special stash that does not affect the normal
> stash's reflogs, and will therefore be invisible to the end user.
> This special stash is essentially a dangling merge commit which has
> reasonable lifetime specified by gc.pruneexpire (default 2 weeks).
>
> Most significantly, this feature means that a caller like pull (with
> pull.rebase set to true) can easily be patched to remove the
> require_clean_work_tree restriction.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/config.txt     |  8 ++++++++
>  Documentation/git-rebase.txt | 10 ++++++++++
>  git-rebase.sh                | 38 +++++++++++++++++++++++++++++++++++---
>  3 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index c67038b..03ad701 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1867,6 +1867,14 @@ rebase.stat::
>  rebase.autosquash::
>         If set to true enable '--autosquash' option by default.
>
> +rebase.autostash::
> +       When set to true, automatically create a temporary stash
> +       before the operation begins, and apply it after the operation
> +       ends.  This means that you can run rebase on a dirty worktree.
> +       However, use with care: the final stash application after a
> +       successful rebase might result in non-trivial conflicts.
> +       Defaults to false.
> +
>  receive.autogc::
>         By default, git-receive-pack will run "git-gc --auto" after
>         receiving data from git-push and updating refs.  You can stop
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index aca8405..c84854a 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -208,6 +208,9 @@ rebase.stat::
>  rebase.autosquash::
>         If set to true enable '--autosquash' option by default.
>
> +rebase.autostash::
> +       If set to true enable '--autostash' option by default.
> +
>  OPTIONS
>  -------
>  --onto <newbase>::
> @@ -394,6 +397,13 @@ If the '--autosquash' option is enabled by default using the
>  configuration variable `rebase.autosquash`, this option can be
>  used to override and disable this setting.
>
> +--[no-]autostash::
> +       Automatically create a temporary stash before the operation
> +       begins, and apply it after the operation ends.  This means
> +       that you can run rebase on a dirty worktree.  However, use
> +       with care: the final stash application after a successful
> +       rebase might result in non-trivial conflicts.
> +
>  --no-ff::
>         With --interactive, cherry-pick all rebased commits instead of
>         fast-forwarding over the unchanged ones.  This ensures that the
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 8412d81..c8fddfe 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -13,6 +13,7 @@ git-rebase --continue | --abort | --skip | --edit-todo
>   Available options are
>  v,verbose!         display a diffstat of what changed upstream
>  q,quiet!           be quiet. implies --no-stat
> +autostash!         automatically stash/stash pop before and after
>  onto=!             rebase onto given branch instead of upstream
>  p,preserve-merges! try to recreate merges instead of ignoring them
>  s,strategy=!       use the given merge strategy
> @@ -64,6 +65,7 @@ apply_dir="$GIT_DIR"/rebase-apply
>  verbose=
>  diffstat=
>  test "$(git config --bool rebase.stat)" = true && diffstat=t
> +autostash="$(git config --bool rebase.autostash || echo false)"
>  git_am_opt=
>  rebase_root=
>  force_rebase=
> @@ -143,6 +145,24 @@ move_to_original_branch () {
>         esac
>  }
>
> +apply_autostash () {
> +       if test -f "$state_dir/autostash"
> +       then
> +               stash_sha1=$(cat "$state_dir/autostash")
> +               git stash apply $stash_sha1 2>&1 >/dev/null ||
> +               die "
> +$(eval_gettext 'Applying autostash resulted in conflicts.
> +Either fix the conflicts now, or run
> +       git reset --hard
> +and apply the stash on your desired branch:
> +       git stash apply $stash_sha1
> +at any time.')" &&
> +               echo "Applied autostash"
> +       fi
> +       git gc --auto &&
> +       rm -rf "$state_dir"
> +}

Because I am in a git-rebase which has apparently failed, I would
expect 'git rebase --abort' would save me here.  But it does not and
you have given me some unique instructions to try to recover.  I
suppose rebase--abort cannot be made to recover in this case because
this is a rebase-wrapper and all of my rebase-state is already
discarded.  But I would much prefer to have the normal "undo"-ability
of git-rebase here, once I realize I have made a mistake or
encountered conflicts I am not prepared to handle right now.


> +
>  run_specific_rebase () {
>         if [ "$interactive_rebase" = implied ]; then
>                 GIT_EDITOR=:
> @@ -153,8 +173,7 @@ run_specific_rebase () {
>         ret=$?
>         if test $ret = 0
>         then
> -               git gc --auto &&
> -               rm -rf "$state_dir"
> +               apply_autostash
>         fi
>         exit $ret
>  }
> @@ -248,6 +267,9 @@ do
>         --stat)
>                 diffstat=t
>                 ;;
> +       --autostash)
> +               autostash=true
> +               ;;
>         -v)
>                 verbose=t
>                 diffstat=t
> @@ -348,7 +370,7 @@ abort)
>                 ;;
>         esac
>         output git reset --hard $orig_head
> -       rm -r "$state_dir"
> +       apply_autostash
>         exit
>         ;;
>  edit-todo)
> @@ -487,6 +509,16 @@ case "$#" in
>         ;;
>  esac
>
> +if test "$autostash" = true && ! require_clean_work_tree --quiet
> +then
> +       stash_sha1=$(git stash create) || die "$(gettext "Cannot autostash")" &&
> +       mkdir -p "$state_dir" &&
> +       echo $stash_sha1 >"$state_dir/autostash" &&
> +       stash_abbrev=$(git rev-parse --short $stash_sha1) &&
> +       echo "$(gettext "Created autostash: $stash_abbrev")" &&
> +       git reset --hard
> +fi
> +
>  require_clean_work_tree "rebase" "$(gettext "Please commit or stash them.")"
>
>  # Now we are rebasing commits $upstream..$orig_head (or with --root,
> --
> 1.8.2.1.578.ga933817
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Ramkumar Ramachandra]

Phil Hord wrote:
> Because I am in a git-rebase which has apparently failed, I would
> expect 'git rebase --abort' would save me here.  But it does not and
> you have given me some unique instructions to try to recover.  I
> suppose rebase--abort cannot be made to recover in this case because
> this is a rebase-wrapper and all of my rebase-state is already
> discarded.  But I would much prefer to have the normal "undo"-ability
> of git-rebase here, once I realize I have made a mistake or
> encountered conflicts I am not prepared to handle right now.

You're asking for a hammer solution, when I'm advocating a solution
that offers more flexibility and control.  Commits and worktree
changes are fundamentally two different things, and I treat them
differently.

rebase.autostash is simply a shortcut for:

    $ git stash && git rebase ... && git stash pop

Except that your stash is not blocked during the rebase process: we
use a special stash.  If the last 'git stash pop' fails, do you do
this?

    $ git reset --hard HEAD@{1}
    $ git stash pop
    # snip, snip ...
    # redo the entire rebase

I _never_ find myself doing this; in your hammer solution, you're
advocating that we always do this.

The stash is a powerful tool when used properly: a stash isn't
attached to any branch, and therefore perfectly designed to keep small
temporary worktree changes for a short period of time.
rebase.autostash is _not_ a way to take away power from the user, or
save the user from learning how to use stash.

That said, the current implementation is very rough and I will improve
it in the next iterations.  If the apply fails, I will push the
changes onto stash@{0}, and let the user do a 'git stash pop' instead
of having to remember (or copy out) a SHA-1 displayed in the terminal
output.  Essentially, this leaves the user in the exact same state as
if she had done a 'git stash && git rebase ... && git stash pop'.

Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Matthieu Moy]

Phil Hord <phil.hord@gmail.com> writes:

> Because I am in a git-rebase which has apparently failed, I would
> expect 'git rebase --abort' would save me here.  

More generally, if I "git rebase --abort" in the middle of a rebase (not
necessarily at the end), I'd expect the stash to be restored. Right now,
if I read correctly, "git rebase --abort" would discard the stashed
changes without giving me the sha1 I'd need to use to recover it :-(.

Recovering the stash should be doable with stg like

--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -341,6 +341,11 @@ abort)
                ;;
        esac
        output git reset --hard $orig_head
+       if test -f "$state_dir/autostash"
+       then
+           stash_sha1=$(cat "$state_dir/autostash")
+           git stash apply $stash_sha1
+       fi
        rm -r "$state_dir"
        exit
        ;;

the "stash apply" should succeed without conflict by construction
because the stash is applied on the commit it was created.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping

[Ramkumar Ramachandra]

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.

[PATCH 2/7] rebase -i: don't error out if $state_dir already exists

[Ramkumar Ramachandra]

In preparation for a later patch that will create $state_dir/autostash
in git-rebase.sh before anything else can happen, change a `mkdir
$state_dir` call to `mkdir -p $state_dir`.  The change is safe,
because this is not a test to detect an in-progress rebase (that is
already done much earlier in git-rebase.sh).

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-rebase--interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 5822b2c..3411139 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -842,7 +842,7 @@ then
 fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"
-mkdir "$state_dir" || die "Could not create temporary $state_dir"
+mkdir -p "$state_dir" || die "Could not create temporary $state_dir"
 
 : > "$state_dir"/interactive || die "Could not mark as interactive"
 write_basic_state
-- 
1.8.3.rc1.51.gd7a04de