Hi,
The feature is finished with documentation and tests in this
iteration. I've written an extensive t3420 which proves that the
feature works flawlessly. Further, I've made every attempt to
actually explain what I'm doing: I've taken care to inspect all the
return values.
Overall, I'm elated with the design and interface. I think it is most
intuitive, while not trading off power/ flexibility.
One subtle detail that you might disagree with: I report success if
the rebase succeeds but the stash application fails. Are we okay with
this?
Also, does t3420 exercise all the cases sufficiently? Have I missed
anything?
Enjoy reading and reviewing this.
Ramkumar Ramachandra (8):
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
rebase: prepare to do generic housekeeping
am: return control to caller, for housekeeping
rebase -i: return control to caller, for housekeeping
rebase --merge: return control to caller, for housekeeping
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 | 11 ++--
git-rebase--merge.sh | 5 +-
git-rebase.sh | 46 +++++++++++++-
t/t3420-rebase-autostash.sh | 148 +++++++++++++++++++++++++++++++++++++++++++
8 files changed, 233 insertions(+), 18 deletions(-)
create mode 100755 t/t3420-rebase-autostash.sh
--
1.8.3.rc1.52.gc14258d
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 <redacted>
---
git-am.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
In preparation for a later patch that creates $dotest/autostash in
git-rebase.sh before anything else happens, don't assume that the
presence of a $dotest directory implies the existence of the $next and
$last files. The check for the files is in a conditional anyway, but
`cat` is executed on potentially non-existent files. Suppress this
error output.
Signed-off-by: Ramkumar Ramachandra <redacted>
---
git-am.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -446,8 +446,8 @@ done# If the dotest directory exists, but we have finished applying all the# patches in them, clear it out.iftest-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
We only need to do these two tasks
git gc --auto
rm -fr "$dotest"
ourselves if the script was invoked as a standalone program; when
invoked with --rebasing (from git-rebase--am.sh), cascade control back
to the ultimate caller git-rebase.sh to do this for us.
Signed-off-by: Ramkumar Ramachandra <redacted>
---
git-am.sh | 9 +++++++--
git-rebase--am.sh | 8 ++++----
2 files changed, 11 insertions(+), 6 deletions(-)
@@ -904,5 +904,10 @@ if test -s "$dotest"/rewritten; thenfifi-rm-fr"$dotest"-gitgc--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"+gitgc--auto+fi
Return control to the caller git-rebase.sh to get these two tasks
rm -fr "$dotest"
git gc --auto
done by it.
Signed-off-by: Ramkumar Ramachandra <redacted>
---
git-rebase--interactive.sh | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
@@ -628,17 +628,16 @@ do_next () {"$GIT_DIR"/hooks/post-rewriterebase<"$rewritten_list"true# we don't care if this hook failedfi&&-rm-rf"$state_dir"&&-gitgc--auto&&warn"Successfully rebased and updated $head_name."-exit+return1# not failure; just to break the do_rest loop}+# can only return 0, when the infinite loop breaks do_rest(){while:do-do_next+do_next||breakdone}
@@ -805,11 +804,13 @@ first and then run 'git rebase --continue' again."require_clean_work_tree"rebase"do_rest+return0;; skip)gitrererecleardo_rest+return0;; edit-todo)gitstripspace--strip-comments<"$todo">"$todo".new
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 <redacted>
---
Documentation/config.txt | 8 +++
Documentation/git-rebase.txt | 10 +++
git-rebase.sh | 43 ++++++++++++-
t/t3420-rebase-autostash.sh | 148 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 206 insertions(+), 3 deletions(-)
create mode 100755 t/t3420-rebase-autostash.sh
@@ -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
@@ -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
@@ -487,6 +513,17 @@ case "$#" in;;esac+iftest"$autostash"=true&&!(require_clean_work_tree)2>/dev/null+then+stash_sha1=$(gitstashcreate"autostash")\+||die"$(gettext'Cannot autostash')"&&+mkdir-p"$state_dir"&&+echo$stash_sha1>"$state_dir/autostash"&&+stash_abbrev=$(gitrev-parse--short$stash_sha1)&&+echo"$(eval_gettext'Created autostash: $stash_abbrev')"&&+gitreset--hard+fi+ require_clean_work_tree"rebase""$(gettext"Please commit or stash them.")"# Now we are rebasing commits $upstream..$orig_head (or with --root,
Return control to the caller git-rebase.sh to get these two tasks
rm -fr "$dotest"
git gc --auto
done by it.
Signed-off-by: Ramkumar Ramachandra <redacted>
---
git-rebase--merge.sh | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
On successful completion of a rebase in git-rebase--$backend.sh, the
$backend script cleans up on its own and exits. The cleanup routine
is however, independent of the $backend, and each $backend script
unnecessarily duplicates this work:
rm -rf "$state_dir"
git gc --auto
Prepare git-rebase.sh for later patches that return control from each
$backend script back to us, for performing this generic cleanup
routine.
Another advantage is that git-rebase.sh can implement a generic
finish_rebase() to possibly do additional tasks in addition to the
cleanup.
Signed-off-by: Ramkumar Ramachandra <redacted>
---
git-rebase.sh | 7 +++++++
1 file changed, 7 insertions(+)
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 <redacted>
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -842,7 +842,7 @@ thenfiorig_head=$(gitrev-parse--verifyHEAD)||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
From: Eric Sunshine <hidden> Date: 2016-06-15 22:57:12
On Fri, May 10, 2013 at 10:26 AM, Ramkumar Ramachandra
[off-list ref] wrote:
quoted hunk
On successful completion of a rebase in git-rebase--$backend.sh, the
$backend script cleans up on its own and exits. The cleanup routine
is however, independent of the $backend, and each $backend script
unnecessarily duplicates this work:
rm -rf "$state_dir"
git gc --auto
Prepare git-rebase.sh for later patches that return control from each
$backend script back to us, for performing this generic cleanup
routine.
Another advantage is that git-rebase.sh can implement a generic
finish_rebase() to possibly do additional tasks in addition to the
cleanup.
Signed-off-by: Ramkumar Ramachandra <redacted>
---
git-rebase.sh | 7 +++++++
1 file changed, 7 insertions(+)
From: Junio C Hamano <hidden> Date: 2016-06-15 22:57:12
Ramkumar Ramachandra [off-list ref] writes:
quoted hunk
On successful completion of a rebase in git-rebase--$backend.sh, the
$backend script cleans up on its own and exits. The cleanup routine
is however, independent of the $backend, and each $backend script
unnecessarily duplicates this work:
rm -rf "$state_dir"
git gc --auto
Prepare git-rebase.sh for later patches that return control from each
$backend script back to us, for performing this generic cleanup
routine.
Another advantage is that git-rebase.sh can implement a generic
finish_rebase() to possibly do additional tasks in addition to the
cleanup.
Signed-off-by: Ramkumar Ramachandra <redacted>
---
git-rebase.sh | 7 +++++++
1 file changed, 7 insertions(+)
Doesn't this exit look suspicious? The existing callsites of this
shell function has a lot of code after them (e.g. when "continue"
$action is given, run_specific_rebase is run) but there is no exit
after the call returns, so they may already be expecting the
function not to return, exiting by itself. But then the last step
of this function in the original code, ". git-rebase--$type", would
be the one that is causing us to exit, no?
So it is either (1) the added code is unreachable and unexercised at
this point in the series, or (2) my analysis above is incorrect and
". git-rebase--$type" does return to let the caller proceed, but
this patch changes the behaviour and breaks the caller. I think it
is the former but then the organization of the series does not make
sense.
Perhaps this should come a bit later in the series?
At least the log message should mention that this is adding an
unreachable cruft at this step, if the order is to be kept.
So it is either (1) the added code is unreachable and unexercised at
this point in the series, or
Yeah, it's (1).
Perhaps this should come a bit later in the series?
When exactly? I picked up on your suggestion to separate out the
preparation-for-$backend-to-return step. The next three patches
convert each of the $backend scripts to return.
At least the log message should mention that this is adding an
unreachable cruft at this step, if the order is to be kept.