Add "git rebase --show-patch"

It is useful to see the full patch while resolving conflicts in a rebase. The only way to do it now is

less .git/rebase-*/patch

which could turn out to be a lot longer to type [1] if you are in a linked worktree, or not at top-dir. On top of that, an ordinary user should not need to peek into .git directory. The new option is provided to examine the patch.

[1] A conflict caused by git-rebase--am.sh does show the path to this patch file so you could copy/paste. But then after some time and lots of commands to resolve the conflict, that path is likely scrolled out of your terminal.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 Documentation/git-rebase.txt           |  5 +++-
 contrib/completion/git-completion.bash |  4 +--
 git-rebase--am.sh                      |  3 +++
 git-rebase--interactive.sh             |  4 +++
 git-rebase--merge.sh                   |  4 +++
 git-rebase.sh                          |  7 +++++-
 t/t3400-rebase.sh                      | 34 ++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  6 +++++
 8 files changed, 63 insertions(+), 4 deletions(-)

Documentation/git-rebase.txt +4 -1

index 8a861c1e0d..4fd571d393 100644
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream> [<branch>]]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort | --quit | --edit-todo
+'git rebase' --continue | --skip | --abort | --quit | --edit-todo | --show-patch
 
 DESCRIPTION
 -----------

@@ -250,6 +250,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --edit-todo::
 	Edit the todo list during an interactive rebase.
 
+--show-patch::
+	Show the current patch in an interactive rebase.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge

contrib/completion/git-completion.bash +2 -2

index 1e9105f6d5..b70da4990f 100644
@@ -1992,11 +1992,11 @@ _git_rebase ()
 {
 	__git_find_repo_path
 	if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
-		__gitcomp "--continue --skip --abort --quit --edit-todo"
+		__gitcomp "--continue --skip --abort --quit --edit-todo --show-patch"
 		return
 	elif [ -d "$__git_repo_path"/rebase-apply ] || \
 	     [ -d "$__git_repo_path"/rebase-merge ]; then
-		__gitcomp "--continue --skip --abort --quit"
+		__gitcomp "--continue --skip --abort --quit --show-patch"
 		return
 	fi
 	__git_complete_strategy && return

git-rebase--am.sh +3 -0

index 14c50782e0..564a4a5830 100644
@@ -27,6 +27,9 @@ skip)
 	move_to_original_branch
 	return
 	;;
+show-patch)
+	exec git am --show-patch
+	;;
 esac
 
 if test -z "$rebase_root"

git-rebase--interactive.sh +4 -0

index d47bd29593..01cc002efd 100644
@@ -840,6 +840,10 @@ To continue rebase after editing, run:
 
 	exit
 	;;
+show-patch)
+	cmt="$(cat "$state_dir/stopped-sha")"
+	exec git format-patch --subject-prefix= --stdout "${cmt}^!"
+	;;
 esac
 
 comment_for_reflog start

git-rebase--merge.sh +0 -0

index 06a4723d4d..5c513a9736 100644

Quoted 8 lines

git-rebase--merge.sh +0 -0

index 06a4723d4d..5c513a9736 100644
@@ -137,6 +137,10 @@ skip)
      finish_rb_merge
      return
      ;;

git-rebase--merge.sh skip) · line 144

Quoted 5 lines

git-rebase--merge.sh +4 -0

+show-patch)
+     cmt="$(cat "$state_dir/current")"
+     exec git format-patch --subject-prefix= --stdout "${cmt}^!"
+     ;;
 esac

Here and in the git-rebase--interactive you have access to the SHA of the failed pick so you could run git log --patch and git colored output

and it would use the pager in the same way as 'git am --show-patch' does

Quoted 8 lines · Phillip Wood

Quoted 8 lines

git-rebase--merge.sh +0 -0

index 06a4723d4d..5c513a9736 100644
@@ -137,6 +137,10 @@ skip)
      finish_rb_merge
      return
      ;;

Quoted 8 lines

git-rebase--merge.sh skip) · line 144

Quoted 5 lines

git-rebase--merge.sh +4 -0

+show-patch)
+     cmt="$(cat "$state_dir/current")"
+     exec git format-patch --subject-prefix= --stdout "${cmt}^!"
+     ;;
 esac

Here and in the git-rebase--interactive you have access to the SHA of the failed pick so you could run git log --patch and git colored output

Yes. My first revision I actually did "git diff" here. The only problem is inconsistency because we can't color "git am --show-patch" the same way, the patch source is in text format, not in the repo. But if people are ok with that I sure would switch to "git show".

Quoted 1 line

and it would use the pager in the same way as 'git am --show-patch' does

format-patch does set up pager. If it does not I would be very annoyed. I added this for convenience after all.

It is useful to see the full patch while resolving conflicts in a rebase. The only way to do it now is

less .git/rebase-*/patch

which could turn out to be a lot longer to type [1] if you are in a linked worktree, or not at top-dir. On top of that, an ordinary user should not need to peek into .git directory. The new option is provided to examine the patch.
[...]

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---

Quoted 11 lines · Nguyễn Thái Ngọc Duy

It is useful to see the full patch while resolving conflicts in a rebase. The only way to do it now is

less .git/rebase-*/patch

which could turn out to be a lot longer to type [1] if you are in a linked worktree, or not at top-dir. On top of that, an ordinary user should not need to peek into .git directory. The new option is provided to examine the patch.
[...]

git-rebase--interactive.sh To continue rebase after editing, run: · line 843

Quoted 8 lines

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---

git-rebase--interactive.sh +4 -0

@@ -840,6 +840,10 @@ To continue rebase after editing, run:
+show-patch)
+       cmt="$(cat "$state_dir/stopped-sha")"
+       exec git format-patch --subject-prefix= --stdout "${cmt}^!"
+       ;;

What is the behavior if a rebase (or conflicted rebase) is not in progress? Stated differently, do we only make it this far if $state_dir/stopped_sha exists?

When a conflict happens during a rebase, you often need to look at the original patch to see what the changes are. This requires opening your favourite pager with some random path inside $GIT_DIR.

This series makes that experience a bit better, by providing a command to read the patch. This is along the line of --edit-todo and --quit where you can just tell git what to do and not bother with details.

My main focus is "git rebase", but because rebase uses "git am" behind the scene, "git am" gains --show-patch option too.

There was something more I wanted to do, like coloring to the patch. But that probably will come later. I'll try to merge these two 21-months-old patches first.

[…]

Quoted 8 lines · Nguyễn Thái Ngọc Duy

When a conflict happens during a rebase, you often need to look at the original patch to see what the changes are. This requires opening your favourite pager with some random path inside $GIT_DIR.

This series makes that experience a bit better, by providing a command to read the patch. This is along the line of --edit-todo and --quit where you can just tell git what to do and not bother with details.

Quoted 8 lines

My main focus is "git rebase", but because rebase uses "git am" behind the scene, "git am" gains --show-patch option too.

There was something more I wanted to do, like coloring to the patch. But that probably will come later. I'll try to merge these two 21-months-old patches first.

[…]

I dislike the approach to use a separate command/option. The nice thing about rebase-apply/original-commit is that you can use it in /any/ git command, i. e. you can do "git log $whatever..rebase-apply/original-commit".

What I would do instead is (besides documenting it :-)) to provide an alias that is more in line with ORIG_HEAD, FETCH_HEAD, etc.; i. e. something along the lines of (pseudo code, will probably not work):

| --- a/builtin/am.c
| +++ b/builtin/am.c
| @@ -1110,6 +1110,7 @@ static void am_next(struct am_state *state)
|
|         oidclr(&state->orig_commit);
|         unlink(am_path(state, "original-commit"));
| +       delete_ref(NULL, "ORIG_COMMIT", NULL, 0);
|
|         if (!get_oid("HEAD", &head))
|                 write_state_text(state, "abort-safety", oid_to_hex(&head));
| @@ -1441,6 +1442,7 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
|
|         oidcpy(&state->orig_commit, &commit_oid);
|         write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
| +       update_ref_oid("am", "ORIG_COMMIT", &commit_oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
|
|         return 0;
|  }

This (when working) would allow to use ORIG_COMMIT in place of the mouthful rebase-apply/original-commit.

When a conflict happens during a rebase, you often need to look at the original patch to see what the changes are. This requires opening your favourite pager with some random path inside $GIT_DIR.

This series makes that experience a bit better, by providing a command to read the patch. This is along the line of --edit-todo and --quit where you can just tell git what to do and not bother with details.

My main focus is "git rebase", but because rebase uses "git am" behind the scene, "git am" gains --show-patch option too.

There was something more I wanted to do, like coloring to the patch. But that probably will come later. I'll try to merge these two 21-months-old patches first.

When a conflict happens during a rebase, you often need to look at the original patch to see what the changes are. This requires opening your favourite pager with some random path inside $GIT_DIR.

This series makes that experience a bit better, by providing a command to read the patch. This is along the line of --edit-todo and --quit where you can just tell git what to do and not bother with details.

My main focus is "git rebase", but because rebase uses "git am" behind the scene, "git am" gains --show-patch option too.

Hi Duy,

Quoted 8 lines · Nguyễn Thái Ngọc Duy

When a conflict happens during a rebase, you often need to look at the original patch to see what the changes are. This requires opening your favourite pager with some random path inside $GIT_DIR.

This series makes that experience a bit better, by providing a command to read the patch. This is along the line of --edit-todo and --quit where you can just tell git what to do and not bother with details.

Quoted 2 lines

My main focus is "git rebase", but because rebase uses "git am" behind the scene, "git am" gains --show-patch option too.

Makes sense. I am not a 100% certain that 2/2 catches all rebase -i corner cases, but I think the patches are good enough even for next already.

Quoted 15 lines · Johannes Schindelin

Hi Duy,

Quoted 10 lines · Nguyễn Thái Ngọc Duy

When a conflict happens during a rebase, you often need to look at the original patch to see what the changes are. This requires opening your favourite pager with some random path inside $GIT_DIR.

This series makes that experience a bit better, by providing a command to read the patch. This is along the line of --edit-todo and --quit where you can just tell git what to do and not bother with details.

My main focus is "git rebase", but because rebase uses "git am" behind the scene, "git am" gains --show-patch option too.

Quoted 2 lines

Makes sense. I am not a 100% certain that 2/2 catches all rebase -i corner cases, but I think the patches are good enough even for next already.

Not so fast :) With Tim's suggestion about using a pseudo ref and AEvar complaint about potential confusion, I might actually go with pseudo ref for rebase (and leave "git am" in the cold for now).

The pseudo ref certainly has an appeal. For people very familiar with Git's peculiarities such as FETCH_HEAD. Such as myself.

For users, it is probably substantially worse an experience than having a cmdmode like --show-patch in the very command they were just running.

Quoted 5 lines

The pseudo ref certainly has an appeal. For people very familiar with Git's peculiarities such as FETCH_HEAD. Such as myself.

For users, it is probably substantially worse an experience than having a cmdmode like --show-patch in the very command they were just running.

I tend to agree with that assessment. FETCH_HEAD was a required mechanism for commands in the toolchain to communicate and wasn't meant as a mechanism for end-users. I do not think it is a good idea to add to the pile to these special files the users would need to look at, when we do not need to.

Even if the internal implementation uses such a file, wrapping it with a documented command mode would make a better UI.

Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess around in there, which is not a good thing. With this, the user does not have to keep the path around somewhere (because after a couple of commands, the path may be out of scrollback buffer) when they need to look at the patch.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---

builtin/am.c +5 -0

+       setup_pager();
+       write_in_full(1, sb.buf, sb.len);
+       strbuf_release(&sb);
+       return 0;
+}

Quoted 6 lines · Nguyễn Thái Ngọc Duy

Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess around in there, which is not a good thing. With this, the user does not have to keep the path around somewhere (because after a couple of commands, the path may be out of scrollback buffer) when they need to look at the patch.

builtin/am.c static void am_abort(struct am_state *state) · line 2128

Quoted 13 lines

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---

builtin/am.c +9 -0

@@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
+static int show_patch(struct am_state *state)
+{
+       struct strbuf sb = STRBUF_INIT;
+       int len;
+
+       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
+       if (len < 0)
+               die_errno(_("failed to read '%s'"),
+                         am_path(state, msgnum(state)));

Isn't this am_path() invocation inside die_errno() likely to clobber the 'errno' from strbuf_read_file() which you want to be reporting?

Quoted 7 lines · Eric Sunshine

Quoted 6 lines · Nguyễn Thái Ngọc Duy

Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess around in there, which is not a good thing. With this, the user does not have to keep the path around somewhere (because after a couple of commands, the path may be out of scrollback buffer) when they need to look at the patch.

Quoted 16 lines

builtin/am.c static void am_abort(struct am_state *state) · line 2128

Quoted 13 lines

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---

builtin/am.c +9 -0

@@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
+static int show_patch(struct am_state *state)
+{
+       struct strbuf sb = STRBUF_INIT;
+       int len;
+
+       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
+       if (len < 0)
+               die_errno(_("failed to read '%s'"),
+                         am_path(state, msgnum(state)));

Isn't this am_path() invocation inside die_errno() likely to clobber the 'errno' from strbuf_read_file() which you want to be reporting?

True.

Quoted 10 lines · Junio C Hamano

Quoted 8 lines · Eric Sunshine

Review

Quoted 4 lines · Nguyễn Thái Ngọc Duy

(quoted diff) +4 -0

+       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
+       if (len < 0)
+               die_errno(_("failed to read '%s'"),
+                         am_path(state, msgnum(state)));

Isn't this am_path() invocation inside die_errno() likely to clobber the 'errno' from strbuf_read_file() which you want to be reporting?

True.

Thanks both. Good catch. Of course I will fix this in the re-roll, but should we also do something for the current code base with the following patch?

Quoted 16 lines · Duy Nguyen

Quoted 10 lines · Junio C Hamano

Quoted 8 lines · Eric Sunshine

Review

Quoted 4 lines · Nguyễn Thái Ngọc Duy

(quoted diff) +4 -0

+       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
+       if (len < 0)
+               die_errno(_("failed to read '%s'"),
+                         am_path(state, msgnum(state)));

Isn't this am_path() invocation inside die_errno() likely to clobber the 'errno' from strbuf_read_file() which you want to be reporting?

True.

Thanks both. Good catch. Of course I will fix this in the re-roll, but should we also do something for the current code base with the following patch?

builtin/am.c static int read_state_file(struct strbuf *sb, const struct am_state *state, · line 265

Quoted 5 lines

builtin/am.c +4 -1

-       die_errno(_("could not read '%s'"), am_path(state, file));
+       saved_errno = errno;
+       path = am_path(state, file);
+       errno = saved_errno;
+       die_errno(_("could not read '%s'"), path);

Rather than worrying about catching these at review time, I had been thinking about a solution which automates it using variadic macros. Something like:

    #define die_errno(...) do { \
        int saved_errno_ = errno; \
        die_errno_(saved_errno_, __VA_ARGS__); \
        } while (0);

Subject: [PATCH] Preserve errno in case case before calling sth_errno()

All these locations do something like this

    sth_errno(..., somefunc(...));

where somefunc() can potentially change errno, which will be read by sth_errno(). Call somefunc separately with errno preserved to avoid this.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 builtin/am.c      |  8 +++++++-
 builtin/commit.c  |  8 ++++++--
 builtin/init-db.c |  9 ++++++---
 rerere.c          |  9 ++++++---
 shallow.c         | 27 ++++++++++++++++++---------
 5 files changed, 43 insertions(+), 18 deletions(-)

builtin/init-db.c +0 -0

index c9b7946bad..e384749f73 100644
@@ -570,9 +570,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 			set_git_work_tree(work_tree);
 		else
 			set_git_work_tree(git_work_tree_cfg);

rerere.c +0 -0

index 1ce440f4bb..f19a53ff2c 100644
@@ -683,9 +683,12 @@ static int merge(const struct rerere_id *id, const char *path)
 	 * A successful replay of recorded resolution.
 	 * Mark that "postimage" was used to help gc.

shallow.c +0 -0

index df4d44ea7a..9da82f5292 100644
@@ -295,9 +295,12 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
 		temp = xmks_tempfile(git_path("shallow_XXXXXX"));
 

Nguyễn Thái Ngọc Duy <[email protected]> writes:

Quoted 4 lines

    `.gitignore` file).

- - Otherwise, Git treats the pattern as a shell glob suitable

Review

Quoted 10 lines

(quoted diff) +5 -5

-   for consumption by fnmatch(3) with the FNM_PATHNAME flag:
-   wildcards in the pattern will not match a / in the pathname.
-   For example, "Documentation/{asterisk}.html" matches
-   "Documentation/git.html" but not "Documentation/ppc/ppc.html"
-   or "tools/perf/Documentation/perf.html".
+ - Otherwise, Git treats the pattern as a shell glob: '{asterisk}'
+   matches anything except '/', '?' matches any one character except
+   '/' and '[]' matches one character in a selected range. See
+   fnmatch(3) and the FNM_PATHNAME flag for a more accurate
+   description.

Where the original did not quote single letters at all, this uses a pair of single quotes. Did you make sure it renders well in HTML and manpage already or should I do that for you before applying?

I think what you wrote is accurate enough already, and those who want to go to fnmatch(3) would do so not for accuracy but for authority ;-) Perhaps s/accurate/detailed/?

The new command git rebase --show-current-patch is useful for seeing the commit related to the current rebase state. Some however may find the "git show" command behind it too limiting. You may want to increase context lines, do a diff that ignores whitespaces...

For these advanced use cases, the user can execute any command they want with the new pseudo ref ORIG_COMMIT.

This also helps show where the stopped commit is from, which is hard to see from the previous patch which implements --show-current-patch.

Helped-by Tim Landscheidt <[email protected]>

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---

 Documentation/git-rebase.txt           | 3 ++-
 builtin/am.c                           | 4 ++++
 contrib/completion/git-completion.bash | 2 +-
 git-rebase--interactive.sh             | 5 ++++-
 git-rebase--merge.sh                   | 4 +++-
 git-rebase.sh                          | 1 +
 sequencer.c                            | 3 +++
 t/t3400-rebase.sh                      | 3 ++-
 t/t3404-rebase-interactive.sh          | 5 ++++-
 9 files changed, 24 insertions(+), 6 deletions(-)

Documentation/git-rebase.txt +2 -1

index 7ef9577472..6da9296bf8 100644
@@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 
 --show-current-patch::
 	Show the current patch in an interactive rebase or when rebase
-	is stopped because of conflicts.
+	is stopped because of conflicts. This is the equivalent of
+	`git show ORIG_COMMIT`.
 
 -m::
 --merge::

builtin/am.c +4 -0

index caec50cba9..bf9b356340 100644
@@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 
 	if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
 		die_errno(_("failed to create directory '%s'"), state->dir);
+	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 
 	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
 		am_destroy(state);

@@ -1110,6 +1111,7 @@ static void am_next(struct am_state *state)
 
 	oidclr(&state->orig_commit);
 	unlink(am_path(state, "original-commit"));
+	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 
 	if (!get_oid("HEAD", &head))
 		write_state_text(state, "abort-safety", oid_to_hex(&head));

@@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
 
 	oidcpy(&state->orig_commit, &commit_oid);
 	write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
+	update_ref("am", "ORIG_COMMIT", &commit_oid,
+		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 
 	return 0;
 }

contrib/completion/git-completion.bash +1 -1

index 2bd30d68cf..deea688e0e 100644
@@ -439,7 +439,7 @@ __git_refs ()
 			track=""
 			;;
 		*)
-			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
+			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD ORIG_COMMIT; do
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then

git-rebase--interactive.sh +4 -1

index 0c0f8abbf9..ef72bd5871 100644
@@ -199,12 +199,14 @@ make_patch () {
 
 die_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref ORIG_COMMIT "$1"
 	make_patch "$1"
 	die "$2"
 }
 
 exit_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref ORIG_COMMIT "$1"
 	make_patch $1
 	git rev-parse --verify HEAD > "$amend"
 	gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}

@@ -841,7 +843,7 @@ To continue rebase after editing, run:
 	exit
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/stopped-sha")" --
+	exec git show ORIG_COMMIT --
 	;;
 esac
 

@@ -858,6 +860,7 @@ fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
 mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
+rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
 write_basic_state

git-rebase--merge.sh +3 -1

index 0a96dfae37..70966c32c3 100644
@@ -57,6 +57,7 @@ call_merge () {
 	echo "$msgnum" >"$state_dir/msgnum"
 	cmt="$(cat "$state_dir/cmt.$msgnum")"
 	echo "$cmt" > "$state_dir/current"
+	git update-ref ORIG_COMMIT "$cmt"
 	hd=$(git rev-parse --verify HEAD)
 	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
 	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'

@@ -138,13 +139,14 @@ skip)
 	return
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/current")" --
+	exec git show ORIG_COMMIT --
 	;;
 esac
 
 mkdir -p "$state_dir"
 echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
+rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 
 msgnum=0
 for cmt in $(git rev-list --reverse --no-merges "$revisions")

git-rebase.sh +1 -0

index 41c915d18c..1db4301b90 100755
@@ -182,6 +182,7 @@ You can run "git stash pop" or "git stash drop" at any time.
 }
 
 finish_rebase () {
+	rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
 	rm -rf "$state_dir"

sequencer.c +3 -0

index 4d3f60594c..fe907a0701 100644
@@ -1792,6 +1792,8 @@ static int make_patch(struct commit *commit, struct replay_opts *opts)
 	p = short_commit_name(commit);
 	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
 		return -1;
+	update_ref("rebase", "ORIG_COMMIT", &commit->object.oid,
+		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 
 	strbuf_addf(&buf, "%s/patch", get_dir(opts));
 	memset(&log_tree_opt, 0, sizeof(log_tree_opt));

@@ -2043,6 +2045,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			unlink(rebase_path_author_script());
 			unlink(rebase_path_stopped_sha());
 			unlink(rebase_path_amend());
+			delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 		}
 		if (item->command <= TODO_SQUASH) {
 			if (is_rebase_i(opts))

t/t3400-rebase.sh +2 -1

index 09943d6a9b..2be4abcb7b 100755
@@ -306,7 +306,8 @@ test_expect_success 'rebase--merge.sh and --show-current-patch' '
 		test_must_fail git rebase --merge --onto init HEAD^ &&
 		git rebase --show-current-patch >actual.patch &&
 		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-		grep "show.*$(git rev-parse two)" stderr
+		grep "show.*ORIG_COMMIT" stderr &&
+		test "$(git rev-parse ORIG_COMMIT)" = "$(git rev-parse two)"
 	)
 '
 

t/t3404-rebase-interactive.sh +4 -1

index 3af6f149a9..c0fe0193bb 100755
@@ -227,7 +227,10 @@ test_expect_success 'stop on conflicting pick' '
 
 test_expect_success 'show conflicted patch' '
 	GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-	grep "show.*$(cat "$state_dir/stopped-sha")" stderr
+	grep "show.*ORIG_COMMIT" stderr &&
+	# the original stopped-sha1 is abbreviated
+	stopped_sha1="$(git rev-parse $(cat ".git/rebase-merge/stopped-sha"))" &&
+	test "$(git rev-parse ORIG_COMMIT)" = "$stopped_sha1"
 '
 
 test_expect_success 'abort' '

Quoted 11 lines · Nguyễn Thái Ngọc Duy

The new command git rebase --show-current-patch is useful for seeing the commit related to the current rebase state. Some however may find the "git show" command behind it too limiting. You may want to increase context lines, do a diff that ignores whitespaces...

For these advanced use cases, the user can execute any command they want with the new pseudo ref ORIG_COMMIT.

This also helps show where the stopped commit is from, which is hard to see from the previous patch which implements --show-current-patch.

Quoted 3 lines

Helped-by Tim Landscheidt <[email protected]>

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---

Hmph, how is this new file conceptually different from existing ones like CHERRY_PICK_HEAD?

On Thu, Feb 1, 2018 at 6:18 AM, Junio C Hamano <[email protected]> wrote:
...

Quoted 2 lines

Hmph, how is this new file conceptually different from existing ones like CHERRY_PICK_HEAD?

Conceptually the same, except that CHERRY_PICK_HEAD can't be reused because it's specifically tied to git-cherry-pick (there's even code that delete this ref if cherry-pick is run as part of rebase, and git-status uses this ref to see if a cherry-pick is in progress). There's also REVERT_HEAD in sequencer.c, same purpose but for git-revert. Perhaps I should rename this new ref to REBASE_HEAD to follow the same naming?

The new command git rebase --show-current-patch is useful for seeing the commit related to the current rebase state. Some however may find the "git show" command behind it too limiting. You may want to increase context lines, do a diff that ignores whitespaces...

For these advanced use cases, the user can execute any command they want with the new pseudo ref ORIG_COMMIT.

This also helps show where the stopped commit is from, which is hard to see from the previous patch which implements --show-current-patch.

Helped-by Tim Landscheidt <[email protected]>

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 Documentation/git-rebase.txt           | 3 ++-
 builtin/am.c                           | 4 ++++
 contrib/completion/git-completion.bash | 2 +-
 git-rebase--interactive.sh             | 5 ++++-
 git-rebase--merge.sh                   | 4 +++-
 git-rebase.sh                          | 1 +
 sequencer.c                            | 3 +++
 t/t3400-rebase.sh                      | 3 ++-
 t/t3404-rebase-interactive.sh          | 5 ++++-
 9 files changed, 24 insertions(+), 6 deletions(-)

Documentation/git-rebase.txt +2 -1

index 7ef9577472..6da9296bf8 100644
@@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 
 --show-current-patch::
 	Show the current patch in an interactive rebase or when rebase
-	is stopped because of conflicts.
+	is stopped because of conflicts. This is the equivalent of
+	`git show ORIG_COMMIT`.
 
 -m::
 --merge::

builtin/am.c +4 -0

index caec50cba9..bf9b356340 100644
@@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 
 	if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
 		die_errno(_("failed to create directory '%s'"), state->dir);
+	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 
 	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
 		am_destroy(state);

@@ -1110,6 +1111,7 @@ static void am_next(struct am_state *state)
 
 	oidclr(&state->orig_commit);
 	unlink(am_path(state, "original-commit"));
+	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 
 	if (!get_oid("HEAD", &head))
 		write_state_text(state, "abort-safety", oid_to_hex(&head));

@@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
 
 	oidcpy(&state->orig_commit, &commit_oid);
 	write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
+	update_ref("am", "ORIG_COMMIT", &commit_oid,
+		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 
 	return 0;
 }

contrib/completion/git-completion.bash +1 -1

index 2bd30d68cf..deea688e0e 100644
@@ -439,7 +439,7 @@ __git_refs ()
 			track=""
 			;;
 		*)
-			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
+			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD ORIG_COMMIT; do
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then

git-rebase--interactive.sh +4 -1

index 0c0f8abbf9..ef72bd5871 100644
@@ -199,12 +199,14 @@ make_patch () {
 
 die_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref ORIG_COMMIT "$1"
 	make_patch "$1"
 	die "$2"
 }
 
 exit_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref ORIG_COMMIT "$1"
 	make_patch $1
 	git rev-parse --verify HEAD > "$amend"
 	gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}

@@ -841,7 +843,7 @@ To continue rebase after editing, run:
 	exit
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/stopped-sha")" --
+	exec git show ORIG_COMMIT --
 	;;
 esac
 

@@ -858,6 +860,7 @@ fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
 mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
+rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
 write_basic_state

git-rebase--merge.sh +3 -1

index 0a96dfae37..70966c32c3 100644
@@ -57,6 +57,7 @@ call_merge () {
 	echo "$msgnum" >"$state_dir/msgnum"
 	cmt="$(cat "$state_dir/cmt.$msgnum")"
 	echo "$cmt" > "$state_dir/current"
+	git update-ref ORIG_COMMIT "$cmt"
 	hd=$(git rev-parse --verify HEAD)
 	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
 	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'

@@ -138,13 +139,14 @@ skip)
 	return
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/current")" --
+	exec git show ORIG_COMMIT --
 	;;
 esac
 
 mkdir -p "$state_dir"
 echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
+rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 
 msgnum=0
 for cmt in $(git rev-list --reverse --no-merges "$revisions")

git-rebase.sh +1 -0

index 41c915d18c..1db4301b90 100755
@@ -182,6 +182,7 @@ You can run "git stash pop" or "git stash drop" at any time.
 }
 
 finish_rebase () {
+	rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
 	rm -rf "$state_dir"

sequencer.c +0 -0

index 4d3f60594c..fe907a0701 100644

Compared to v2:

- the potential loss of errno before it's printed out in builtin/am.c is fixed.
- keep update_ref() in sequencer.c non-fatal like this rest
- rename ORIG_COMMIT to REBASE_HEAD

Interdiff:

Quoted 6 lines

Compared to v2:

- the potential loss of errno before it's printed out in builtin/am.c is fixed.
- keep update_ref() in sequencer.c non-fatal like this rest
- rename ORIG_COMMIT to REBASE_HEAD

Quoted 1 line

Interdiff:

This round hasn't seen any comments. Is everybody happy with it?

I personally do not have strong opinion for the feature but didn't spot anything against the execution, either, so...

Quoted 7 lines · Junio C Hamano

Quoted 6 lines

Compared to v2:

- the potential loss of errno before it's printed out in builtin/am.c is fixed.
- keep update_ref() in sequencer.c non-fatal like this rest
- rename ORIG_COMMIT to REBASE_HEAD

Quoted 6 lines

Quoted 1 line

Interdiff:

This round hasn't seen any comments. Is everybody happy with it?

I personally do not have strong opinion for the feature but didn't spot anything against the execution, either, so...

Sorry for the late reply: I dislike REBASE_/HEAD/ because ORIG_/HEAD/ refers to the tip of the original branch, and /ORIG/_HEAD refers to the original branch, so /REBASE/_/HEAD/ is doubly confusing IMHO. I consider ORIG_COMMIT easier to understand because ORIG_HEAD refers to the tip of the original branch, and ORIG_COMMIT would refer to one of the commits making up that original branch, but as I suggested it myself I might not be very objective in that regard :-).