threads / patch / 50324

patchrebase: move state_dir to tmp prior to deletion

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

## tl;dr

3 messages between Jan 26, 2019 and Jan 26, 2019. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Ben Woosley· Jan 26, 2019, 07:41 UTC · lore

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(-)

Show changes to 2 files +15 −5

git-legacy-rebase.sh, git-rebase--preserve-merges.sh

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

Ben Woosley· Jan 26, 2019, 10:16 UTC · re: Ben Woosley · lore

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

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(-)

Show changes to 2 files +15 −5

git-legacy-rebase.sh, git-rebase--preserve-merges.sh

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

Alban Gruin· Jan 26, 2019, 13:59 UTC · re: Ben Woosley · lore

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

Hi Ben,

Le 26/01/2019 à 11:16, Ben Woosley a écrit :

Show 31 quoted lines

> 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.

Show 63 quoted lines

> 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