[PATCH] rebase: move state_dir to tmp prior to deletion

[PATCH] rebase: move state_dir to tmp prior to deletion

[Ben Woosley]

From: Ben Woosley <ben.woosley@gmail.com>

To avoid partial deletion / zombie rebases.

Example behavior under partial deletion, after
Ctrl-Cing out of a standard rebase:

    $ git rebase target
    First, rewinding head to replay your work on top of it...
    Applying: [...]
    ^C
    $ git status
    rebase in progress; onto (null)
    You are currently rebasing.
      (all conflicts fixed: run "git rebase --continue")

    Changes to be committed:
      (use "git reset HEAD <file>..." to unstage)
    [...]
    $ git rebase --continue
    error: could not read '.git/rebase-apply/head-name': No such file or directory
    $ git rebase --abort
    error: could not read '.git/rebase-apply/head-name': No such file or directory

Others report this issue here:
https://stackoverflow.com/questions/3685001/git-how-to-fix-corrupted-interactive-rebase
---
 git-legacy-rebase.sh           | 17 ++++++++++++++---
 git-rebase--preserve-merges.sh |  2 +-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index b4c7dbfa575d3..878c0e42054d7 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -128,11 +128,22 @@ read_basic_state () {
 	}
 }
 
+remove_rebase_state () {
+  state_tmpdir=$(mktemp -d -t "git-rebase-state-XXXXXX")
+  if test -d state_tmpdir
+  then
+    exec mv "$state_dir" "$state_tmpdir"
+    exec rm -rf "$state_tmpdir"
+  else
+    exec rm -rf "$state_dir"
+  fi
+}
+
 finish_rebase () {
 	rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
-	rm -rf "$state_dir"
+	remove_rebase_state
 }
 
 run_interactive () {
@@ -194,7 +205,7 @@ run_specific_rebase () {
 	elif test $ret -eq 2 # special exit status for rebase -p
 	then
 		apply_autostash &&
-		rm -rf "$state_dir" &&
+		remove_rebase_state &&
 		die "Nothing to do"
 	fi
 	exit $ret
@@ -439,7 +450,7 @@ abort)
 	exit
 	;;
 quit)
-	exec rm -rf "$state_dir"
+	remove_rebase_state
 	;;
 edit-todo)
 	run_specific_rebase
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index afbb65765d461..146b52df14928 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -226,7 +226,7 @@ Once you are satisfied with your changes, run
 
 die_abort () {
 	apply_autostash
-	rm -rf "$state_dir"
+	remove_rebase_state
 	die "$1"
 }
 

--
https://github.com/git/git/pull/569

[PATCH v2] rebase: move state_dir to tmp prior to deletion

[Ben Woosley]

From: Ben Woosley <ben.woosley@gmail.com>

To avoid partial deletion / zombie rebases.

Example behavior under partial deletion, after
Ctrl-Cing out of a standard rebase:

    $ git rebase target
    First, rewinding head to replay your work on top of it...
    Applying: [...]
    ^C
    $ git status
    rebase in progress; onto (null)
    You are currently rebasing.
      (all conflicts fixed: run "git rebase --continue")

    Changes to be committed:
      (use "git reset HEAD <file>..." to unstage)
    [...]
    $ git rebase --continue
    error: could not read '.git/rebase-apply/head-name': No such file or directory
    $ git rebase --abort
    error: could not read '.git/rebase-apply/head-name': No such file or directory

Others report this issue here:
https://stackoverflow.com/questions/3685001/git-how-to-fix-corrupted-interactive-rebase
---
 git-legacy-rebase.sh           | 17 ++++++++++++++---
 git-rebase--preserve-merges.sh |  2 +-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index b4c7dbfa575d3..832a211c925c3 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -128,11 +128,22 @@ read_basic_state () {
 	}
 }
 
+remove_rebase_state () {
+  removal_dir=$(mktemp -d -t "git-rebase-state-XXXXXX")
+  if test -d "$removal_dir"
+  then
+    mv "$state_dir" "$removal_dir"
+  else
+    removal_dir="$state_dir"
+  fi
+  rm -rf "$removal_dir"
+}
+
 finish_rebase () {
 	rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
-	rm -rf "$state_dir"
+	remove_rebase_state
 }
 
 run_interactive () {
@@ -194,7 +205,7 @@ run_specific_rebase () {
 	elif test $ret -eq 2 # special exit status for rebase -p
 	then
 		apply_autostash &&
-		rm -rf "$state_dir" &&
+		remove_rebase_state &&
 		die "Nothing to do"
 	fi
 	exit $ret
@@ -439,7 +450,7 @@ abort)
 	exit
 	;;
 quit)
-	exec rm -rf "$state_dir"
+	remove_rebase_state
 	;;
 edit-todo)
 	run_specific_rebase
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index afbb65765d461..146b52df14928 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -226,7 +226,7 @@ Once you are satisfied with your changes, run
 
 die_abort () {
 	apply_autostash
-	rm -rf "$state_dir"
+	remove_rebase_state
 	die "$1"
 }
 

--
https://github.com/git/git/pull/569

Re: [PATCH v2] rebase: move state_dir to tmp prior to deletion

[Alban Gruin]

Hi Ben,

Le 26/01/2019 à 11:16, Ben Woosley a écrit :
> From: Ben Woosley <ben.woosley@gmail.com>
> 
> To avoid partial deletion / zombie rebases.
> 
> Example behavior under partial deletion, after
> Ctrl-Cing out of a standard rebase:
> 
>     $ git rebase target
>     First, rewinding head to replay your work on top of it...
>     Applying: [...]
>     ^C
>     $ git status
>     rebase in progress; onto (null)
>     You are currently rebasing.
>       (all conflicts fixed: run "git rebase --continue")
> 
>     Changes to be committed:
>       (use "git reset HEAD <file>..." to unstage)
>     [...]
>     $ git rebase --continue
>     error: could not read '.git/rebase-apply/head-name': No such file or directory
>     $ git rebase --abort
>     error: could not read '.git/rebase-apply/head-name': No such file or directory
> 
> Others report this issue here:
> https://stackoverflow.com/questions/3685001/git-how-to-fix-corrupted-interactive-rebase
> ---
>  git-legacy-rebase.sh           | 17 ++++++++++++++---
>  git-rebase--preserve-merges.sh |  2 +-
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 

This patch does not cover the new builtin rebase, even though it’s also
affected by this bug.  I ran in two bugs trying to reproduce this issue
with the builtin:

 1. Right after invoking `git rebase $target', $target was checked out
but the state directory has not been written yet, leaving the user on
the target without any possibility of aborting the rebase.  I think it’s
because the checkout is done by builtin/rebase.c, whereas the state
directory is created by git-rebase--am -- and it might not be created at
all!

 2. If I wait a little bit, I have the same bug as yours.

I tried to reproduce the issue on master (16a465bc01, "Third batch after
2.20") and builtin.userebase set to false, without success.  It seems
this issue has been fixed in the shell version, but not in the builtin
version -- and it seems that you are using the latter, so it won’t solve
your problem anyway.  Try `git -c rebase.usebuiltin=false rebase
--abort', and the state directory will be removed without any errors.

Things are a bit different with interactive rebase: it checks out the
target branch, then create its state directory.  Only the first issue
can happen, and you have to be very unlucky to run into it.  The same
goes for rebase -p.

> diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
> index b4c7dbfa575d3..832a211c925c3 100755
> --- a/git-legacy-rebase.sh
> +++ b/git-legacy-rebase.sh
> @@ -128,11 +128,22 @@ read_basic_state () {
>  	}
>  }
>  
> +remove_rebase_state () {
> +  removal_dir=$(mktemp -d -t "git-rebase-state-XXXXXX")
> +  if test -d "$removal_dir"
> +  then
> +    mv "$state_dir" "$removal_dir"
> +  else
> +    removal_dir="$state_dir"
> +  fi
> +  rm -rf "$removal_dir"
> +}
> +
>  finish_rebase () {
>  	rm -f "$(git rev-parse --git-path REBASE_HEAD)"
>  	apply_autostash &&
>  	{ git gc --auto || true; } &&
> -	rm -rf "$state_dir"
> +	remove_rebase_state
>  }
>  
>  run_interactive () {
> @@ -194,7 +205,7 @@ run_specific_rebase () {
>  	elif test $ret -eq 2 # special exit status for rebase -p
>  	then
>  		apply_autostash &&
> -		rm -rf "$state_dir" &&
> +		remove_rebase_state &&
>  		die "Nothing to do"
>  	fi
>  	exit $ret
> @@ -439,7 +450,7 @@ abort)
>  	exit
>  	;;
>  quit)
> -	exec rm -rf "$state_dir"
> +	remove_rebase_state
>  	;;
>  edit-todo)
>  	run_specific_rebase
> diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
> index afbb65765d461..146b52df14928 100644
> --- a/git-rebase--preserve-merges.sh
> +++ b/git-rebase--preserve-merges.sh
> @@ -226,7 +226,7 @@ Once you are satisfied with your changes, run
>  
>  die_abort () {
>  	apply_autostash
> -	rm -rf "$state_dir"
> +	remove_rebase_state
>  	die "$1"
>  }
>  
> 
> --
> https://github.com/git/git/pull/569
>
It seems that you tried to implement the solution proposed in the
stackoverflow thread you linked.  Unfortunately, if it fixed something,
it has a problem: it won’t bring you back to the commit where you called
`git rebase', unlike a `git rebase --abort' on a uncorrupted rebase
state.  This is what happens when aborting a corrupted rebase with the
shell version of rebase.  I think it’s because git-rebase--am will only
create a state directory if it has a problem (ie. a conflict or the user
hits ^C).

So, there is definitely a problem with git-rebase--am, but this does not
address it.

-- Alban