threads / patch / 47697

patch, 2 partsAdd "git rebase --show-patch"

Subject: [PATCH 0/2] Add "git rebase --show-patch"

## tl;dr

40 messages between Jan 26, 2018 and Feb 23, 2018. Diffs are folded; open one to read it.

replies: 39people: 7as markdown or json

Nguyễn Thái Ngọc Duy· Jan 26, 2018, 09:55 UTC · lore

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.

Nguyễn Thái Ngọc Duy (2):
  am: add --show-patch
  rebase: add --show-patch
 Documentation/git-am.txt               |  5 +++-
 Documentation/git-rebase.txt           |  5 +++-
 builtin/am.c                           | 23 ++++++++++++++---
 contrib/completion/git-completion.bash |  6 ++---
 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 +++++
 t/t4150-am.sh                          |  5 ++++
 11 files changed, 93 insertions(+), 9 deletions(-)
-- 
2.16.1.205.g271f633410
Nguyễn Thái Ngọc Duy· Jan 26, 2018, 09:55 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 1/2] am: add --show-patch

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. Bonus point for automatic pager.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-am.txt               |  5 ++++-
 builtin/am.c                           | 23 ++++++++++++++++++++---
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh                          |  5 +++++
 4 files changed, 30 insertions(+), 5 deletions(-)
Show changes to 4 files +30 −5

Documentation/git-am.txt, builtin/am.c, contrib/completion/git-completion.bash, t/t4150-am.sh

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..724095e921 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
 	 [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
 	 [(<mbox> | <Maildir>)...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --show-patch)
 
 DESCRIPTION
 -----------
@@ -167,6 +167,9 @@ default.   You can use `--no-utf8` to override this.
 --abort::
 	Restore the original branch and abort the patching operation.
 
+--show-patch::
+	Show the patch being applied.
+
 DISCUSSION
 ----------
 
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..aaaf32886a 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1831,8 +1831,7 @@ static void am_run(struct am_state *state, int resume)
 			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
 
 			if (advice_amworkdir)
-				printf_ln(_("The copy of the patch that failed is found in: %s"),
-						am_path(state, "patch"));
+				printf_ln(_("Use 'git am --show-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
@@ -2121,6 +2120,17 @@ static void am_abort(struct am_state *state)
 	am_destroy(state);
 }
 
+static void show_patch(struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+
+	if (strbuf_read_file(&sb, am_path(state, msgnum(state)), 0) >= 0) {
+		setup_pager();
+		write_in_full(1, sb.buf, sb.len);
+	}
+	strbuf_release(&sb);
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -2149,7 +2159,8 @@ enum resume_mode {
 	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
-	RESUME_ABORT
+	RESUME_ABORT,
+	RESUME_SHOW_PATCH
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2249,6 +2260,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &resume,
 			N_("restore the original branch and abort the patching operation."),
 			RESUME_ABORT),
+		OPT_CMDMODE(0, "show-patch", &resume,
+			N_("show the patch being applied."),
+			RESUME_SHOW_PATCH),
 		OPT_BOOL(0, "committer-date-is-author-date",
 			&state.committer_date_is_author_date,
 			N_("lie about committer date")),
@@ -2359,6 +2373,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_ABORT:
 		am_abort(&state);
 		break;
+	case RESUME_SHOW_PATCH:
+		show_patch(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3683c772c5..1e9105f6d5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort --show-patch"
 		return
 	fi
 	case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..eff6455f42 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -662,6 +662,11 @@ test_expect_success 'am pauses on conflict' '
 	test -d .git/rebase-apply
 '
 
+test_expect_success 'am --show-patch' '
+	git am --show-patch >actual.patch &&
+	test_cmp .git/rebase-apply/0001 actual.patch
+'
+
 test_expect_success 'am --skip works' '
 	echo goodbye >expected &&
 	git am --skip &&
-- 
2.16.1.205.g271f633410
Nguyễn Thái Ngọc Duy· Jan 26, 2018, 09:55 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 2/2] rebase: add --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 <pclouds@gmail.com>
---
 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(-)
Show changes to 8 files +63 −4

Documentation/git-rebase.txt, contrib/completion/git-completion.bash, git-rebase--am.sh, git-rebase--interactive.sh, git-rebase--merge.sh, git-rebase.sh, t/t3400-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d..4fd571d393 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -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
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1e9105f6d5..b70da4990f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -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
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e0..564a4a5830 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -27,6 +27,9 @@ skip)
 	move_to_original_branch
 	return
 	;;
+show-patch)
+	exec git am --show-patch
+	;;
 esac
 
 if test -z "$rebase_root"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593..01cc002efd 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -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
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4d..5c513a9736 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -137,6 +137,10 @@ skip)
 	finish_rb_merge
 	return
 	;;
+show-patch)
+	cmt="$(cat "$state_dir/current")"
+	exec git format-patch --subject-prefix= --stdout "${cmt}^!"
+	;;
 esac
 
 mkdir -p "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index fd72a35c65..ce17090520 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -45,6 +45,7 @@ abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
 quit!              abort but keep HEAD where it is
+show-patch!        show the patch file being applied or merged
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -245,7 +246,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--quit|--edit-todo)
+	--continue|--skip|--abort|--quit|--edit-todo|--show-patch)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -412,6 +413,10 @@ quit)
 edit-todo)
 	run_specific_rebase
 	;;
+show-patch)
+	run_specific_rebase
+	die "BUG: run_specific_rebase is not supposed to return here"
+	;;
 esac
 
 # Make sure no rebase is in progress
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 8ac58d5ea5..599f0d5606 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -277,4 +277,38 @@ EOF
 	test_cmp From_.msg out
 '
 
+test_expect_success 'rebase--am.sh and --show-patch' '
+	test_create_repo conflict-apply &&
+	(
+		cd conflict-apply &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --onto init HEAD^ &&
+		git rebase --show-patch >actual.patch &&
+		test_cmp .git/rebase-apply/0001 actual.patch
+	)
+'
+
+test_expect_success 'rebase--merge.sh and --show-patch' '
+	test_create_repo conflict-merge &&
+	(
+		cd conflict-merge &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --merge --onto init HEAD^ &&
+		git rebase --show-patch >actual.patch &&
+		cmt=$(cat .git/rebase-merge/current) &&
+		git format-patch --stdout --subject-prefix= ${cmt}^! >expected.patch &&
+		test_cmp expected.patch actual.patch
+	)
+'
+
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 481a350090..40f70a0333 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -225,6 +225,12 @@ test_expect_success 'stop on conflicting pick' '
 	test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
 '
 
+test_expect_success 'show conflicted patch' '
+	git rebase --show-patch >actual.patch &&
+	git format-patch --subject-prefix= --stdout new-branch1^! >expected.patch &&
+	test_cmp expected.patch actual.patch
+'
+
 test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
-- 
2.16.1.205.g271f633410
Phillip Wood· Jan 26, 2018, 11:12 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 2/2] rebase: add --show-patch

On 26/01/18 09:55, Nguyễn Thái Ngọc Duy wrote:
Show 111 quoted lines
> 
> 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 <pclouds@gmail.com>
> ---
>  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(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 8a861c1e0d..4fd571d393 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -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
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 1e9105f6d5..b70da4990f 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -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
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 14c50782e0..564a4a5830 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -27,6 +27,9 @@ skip)
>  	move_to_original_branch
>  	return
>  	;;
> +show-patch)
> +	exec git am --show-patch
> +	;;
>  esac
>  
>  if test -z "$rebase_root"
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index d47bd29593..01cc002efd 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -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
> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
> index 06a4723d4d..5c513a9736 100644
> --- a/git-rebase--merge.sh
> +++ b/git-rebase--merge.sh
> @@ -137,6 +137,10 @@ skip)
>  	finish_rb_merge
>  	return
>  	;;
> +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

Best Wishes
Phillip
Duy Nguyen· Jan 26, 2018, 11:22 UTC · re: Phillip Wood · lore

Re: [PATCH 2/2] rebase: add --show-patch

On Fri, Jan 26, 2018 at 6:12 PM, Phillip Wood <phillip.wood@talktalk.net> wrote:
Show 16 quoted lines
>> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
>> index 06a4723d4d..5c513a9736 100644
>> --- a/git-rebase--merge.sh
>> +++ b/git-rebase--merge.sh
>> @@ -137,6 +137,10 @@ skip)
>>       finish_rb_merge
>>       return
>>       ;;
>> +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".

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

-- 
Duy
Phillip Wood· Jan 30, 2018, 11:15 UTC · re: Duy Nguyen · lore

Re: [PATCH 2/2] rebase: add --show-patch

On 26/01/18 11:22, Duy Nguyen wrote:
Show 28 quoted lines
> On Fri, Jan 26, 2018 at 6:12 PM, Phillip Wood <phillip.wood@talktalk.net> wrote:
>>> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
>>> index 06a4723d4d..5c513a9736 100644
>>> --- a/git-rebase--merge.sh
>>> +++ b/git-rebase--merge.sh
>>> @@ -137,6 +137,10 @@ skip)
>>>       finish_rb_merge
>>>       return
>>>       ;;
>>> +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".
> 
>> 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.
> 

Ah, I didn't realize that (now I come to think of it I've only ever used --stdout to redirect the output). As my perceived lack of pager was the main reason I suggested using log I'd ignore me.

I think the suggestion of having a ref for 'rebase -i' and 'rebase -m' could be good as it'd be more flexible though I'm not sure what you'd do about plain old rebase.

Best Wishes
Phillip
Eric Sunshine· Jan 26, 2018, 19:11 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 2/2] rebase: add --show-patch

On Fri, Jan 26, 2018 at 4:55 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
Show 19 quoted lines
> 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 <pclouds@gmail.com>
> ---
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> @@ -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?

Duy Nguyen· Jan 27, 2018, 01:42 UTC · re: Eric Sunshine · lore

Re: [PATCH 2/2] rebase: add --show-patch

On Sat, Jan 27, 2018 at 2:11 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 24 quoted lines
> On Fri, Jan 26, 2018 at 4:55 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> 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 <pclouds@gmail.com>
>> ---
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> @@ -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?

It belongs to the same --continue/--skip/--abort group and won't reach here unless a rebase is in progress (i.e. $state_dir exists).

-- 
Duy
Tim Landscheidt· Jan 26, 2018, 18:47 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> 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.
> […]

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.

Tim
Duy Nguyen· Jan 27, 2018, 01:45 UTC · re: Tim Landscheidt · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

On Sat, Jan 27, 2018 at 1:47 AM, Tim Landscheidt <tim@tim-landscheidt.de> wrote:
Show 50 quoted lines
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
>> 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.
>
>> […]
>
> 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.

Interesting. Taken out of context, I don't think it really works for "git am" because the source patches are in text form and "git am" only fails when it can't apply the patch (and won't be able to create a commit). But for rebase I can see how this is a good thing. Thanks. Let me sleep on it for a bit. It works for cherry-pick too.

-- 
Duy
Ævar Arnfjörð Bjarmason· Jan 27, 2018, 11:26 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

On Fri, Jan 26 2018, Nguyễn Thái Ngọc Duy jotted:
Show 14 quoted lines
> 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.

This is only tangentially related to what you're doing, but I've long wanted to add a commit.verbose config option to emulate `git commit --verbose`, and furthermore to show the patch in rebase under "reword", "squash" etc.

There's been so many times when I start editing the todo list, and reword this or that, only to forget (because I don't have good commit messages yet) what the patch is even about, and then switch to a terminal, "git show" etc.

I'm just mentioning that here because if and when we have such a feature, I think the --show-patch option is going to be very confusing, people might want to enable this thing I'm talking about, but find that --show-patch is something else entirely.

I don't know a good solution to that, just putting that out there.
Johannes Schindelin· Jan 29, 2018, 15:09 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

Hi Duy,
On Fri, 26 Jan 2018, Nguyễn Thái Ngọc Duy wrote:
Show 10 quoted lines
> 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.

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.

Ciao, Dscho

Duy Nguyen· Jan 30, 2018, 09:10 UTC · re: Johannes Schindelin · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

On Mon, Jan 29, 2018 at 10:09 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 17 quoted lines
> Hi Duy,
>
> On Fri, 26 Jan 2018, Nguyễn Thái Ngọc Duy wrote:
>
>> 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.
>
> 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).

-- 
Duy
Johannes Schindelin· Jan 30, 2018, 12:32 UTC · re: Duy Nguyen · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

Hi Duy,
On Tue, 30 Jan 2018, Duy Nguyen wrote:
Show 23 quoted lines
> On Mon, Jan 29, 2018 at 10:09 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Duy,
> >
> > On Fri, 26 Jan 2018, Nguyễn Thái Ngọc Duy wrote:
> >
> >> 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.
> >
> > 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.

Ciao, Dscho

Junio C Hamano· Jan 30, 2018, 20:19 UTC · re: Johannes Schindelin · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 5 quoted 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.

Tim Landscheidt· Feb 1, 2018, 02:06 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] Add "git rebase --show-patch"

Junio C Hamano <gitster@pobox.com> wrote:
>> 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.
Show 5 quoted lines
> 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.
I disagree with that as I had:
| [alias]
|         original-commit = !git show $(cat .git/rebase-apply/original-commit)

for a long time in my ~/.gitconfig :-) (until I realized that Git accepted "rebase-apply/original-commit" everywhere; for single-commit branches I always just used ORIG_HEAD).

This meant that whenever I wanted to look at the lay of the land upon which the original commit was built, I had to do "git original-commit" and copy & paste the SHA1 (without my alias, I had to "git log ORIG_HEAD", pick hopefully the cor- rect commit and copy & paste its SHA1). Only with this SHA1 I could then run "git diff", "git show", "git grep", "git blame", etc., etc., etc. because in my use cases looking at the patch alone was usually not sufficient: I needed to know why there was a conflict, i. e. how the file(s) the patch changed looked before they had been altered upstream.

To me, this type of "algebra" is what makes Git so powerful: I do not have to build a pipe that gets the SHA1 of a branch's tip and xargs it to "git show", I can just say "git show $branch". Having a SHA1 with a special meaning that has no easy way to access it by "git rev-parse" breaks this UI IMHO.

Tim
Nguyễn Thái Ngọc Duy· Jan 31, 2018, 09:30 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 0/3] Add "git rebase --show-patch"

v2:
- keeps --show-patch, but it's renamed to be more specific and
  hopefully less misleading. Other names I thought of were
  --show-patch-in-progress, --show-stopped-patch or
  --show-conflict-patch but they don't look as good.
  Also, since this is a long option name, I don't think length
  matters. You either go with tab completion, alias or short option
  name when you want to type it fast.
- --show-patch now uses git show instead of format-patch, this
  gives coloring for all rebase cases
- people who are not happy with --show-patch can roll their own with
  the help from the pseudo ref ORIG_COMMIT
Nguyễn Thái Ngọc Duy (3):
  am: add --show-current-patch
  rebase: add --show-current-patch
  rebase: introduce and use pseudo-ref ORIG_COMMIT
 Documentation/git-am.txt               |  6 +++-
 Documentation/git-rebase.txt           |  7 +++-
 builtin/am.c                           | 46 +++++++++++++++++++++++---
 contrib/completion/git-completion.bash |  8 ++---
 git-rebase--am.sh                      |  3 ++
 git-rebase--interactive.sh             |  6 ++++
 git-rebase--merge.sh                   |  5 +++
 git-rebase.sh                          |  8 ++++-
 sequencer.c                            |  3 ++
 t/t3400-rebase.sh                      | 34 +++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  8 +++++
 t/t4150-am.sh                          |  5 +++
 12 files changed, 128 insertions(+), 11 deletions(-)
-- 
2.16.1.205.g271f633410
Nguyễn Thái Ngọc Duy· Jan 31, 2018, 09:30 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 1/3] am: add --show-current-patch

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 <pclouds@gmail.com>
---
 Documentation/git-am.txt               |  6 ++++-
 builtin/am.c                           | 31 ++++++++++++++++++++++----
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh                          |  5 +++++
 4 files changed, 38 insertions(+), 6 deletions(-)
Show changes to 4 files +38 −6

Documentation/git-am.txt, builtin/am.c, contrib/completion/git-completion.bash, t/t4150-am.sh

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..0f426ae874 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
 	 [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
 	 [(<mbox> | <Maildir>)...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --show-current-patch)
 
 DESCRIPTION
 -----------
@@ -167,6 +167,10 @@ default.   You can use `--no-utf8` to override this.
 --abort::
 	Restore the original branch and abort the patching operation.
 
+--show-current-patch::
+	Show the patch being applied when "git am" is stopped because
+	of conflicts.
+
 DISCUSSION
 ----------
 
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..5eff1a648d 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1831,8 +1831,7 @@ static void am_run(struct am_state *state, int resume)
 			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
 
 			if (advice_amworkdir)
-				printf_ln(_("The copy of the patch that failed is found in: %s"),
-						am_path(state, "patch"));
+				printf_ln(_("Use 'git am --show-current-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
@@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
 	am_destroy(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)));
+
+	setup_pager();
+	write_in_full(1, sb.buf, sb.len);
+	strbuf_release(&sb);
+	return 0;
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -2149,7 +2164,8 @@ enum resume_mode {
 	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
-	RESUME_ABORT
+	RESUME_ABORT,
+	RESUME_SHOW_PATCH
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2171,6 +2187,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	int patch_format = PATCH_FORMAT_UNKNOWN;
 	enum resume_mode resume = RESUME_FALSE;
 	int in_progress;
+	int ret = 0;
 
 	const char * const usage[] = {
 		N_("git am [<options>] [(<mbox> | <Maildir>)...]"),
@@ -2249,6 +2266,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &resume,
 			N_("restore the original branch and abort the patching operation."),
 			RESUME_ABORT),
+		OPT_CMDMODE(0, "show-current-patch", &resume,
+			N_("show the patch being applied."),
+			RESUME_SHOW_PATCH),
 		OPT_BOOL(0, "committer-date-is-author-date",
 			&state.committer_date_is_author_date,
 			N_("lie about committer date")),
@@ -2359,11 +2379,14 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_ABORT:
 		am_abort(&state);
 		break;
+	case RESUME_SHOW_PATCH:
+		ret = show_patch(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
 
 	am_state_release(&state);
 
-	return 0;
+	return ret;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3683c772c5..56ca445fa8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort --show-current-patch"
 		return
 	fi
 	case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..23abf42abc 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -662,6 +662,11 @@ test_expect_success 'am pauses on conflict' '
 	test -d .git/rebase-apply
 '
 
+test_expect_success 'am --show-current-patch' '
+	git am --show-current-patch >actual.patch &&
+	test_cmp .git/rebase-apply/0001 actual.patch
+'
+
 test_expect_success 'am --skip works' '
 	echo goodbye >expected &&
 	git am --skip &&
-- 
2.16.1.205.g271f633410
Eric Sunshine· Jan 31, 2018, 09:40 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v2 1/3] am: add --show-current-patch

On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
Show 19 quoted lines
> 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 <pclouds@gmail.com>
> ---
> diff --git a/builtin/am.c b/builtin/am.c
> @@ -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?

Show 5 quoted lines
> +       setup_pager();
> +       write_in_full(1, sb.buf, sb.len);
> +       strbuf_release(&sb);
> +       return 0;
> +}
Junio C Hamano· Jan 31, 2018, 22:59 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/3] am: add --show-current-patch

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 23 quoted lines
> On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> 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 <pclouds@gmail.com>
>> ---
>> diff --git a/builtin/am.c b/builtin/am.c
>> @@ -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. After coming up with the pathname to the current patch file, we are going to exit without ever calling am_path(), or underlying get_pathname() via mkpath(), again before exiting anyway, so perhaps it is sufficient to just do an am_path() just once upfront, feed it to strbuf_read_file() and also to die_errno().

Duy Nguyen· Feb 2, 2018, 09:25 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/3] am: add --show-current-patch

On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
Show 27 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> >> 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 <pclouds@gmail.com>
> >> ---
> >> diff --git a/builtin/am.c b/builtin/am.c
> >> @@ -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.

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?

I think this is something coccinelle can help catch, but my coccinelle-foo is way too low to come up with something like "die_errno() must not take any arguments as function calls, except _() and N_()".

-- 8< --
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 <pclouds@gmail.com>
---
 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(-)
Show changes to 5 files +43 −18

builtin/am.c, builtin/commit.c, builtin/init-db.c, rerere.c, shallow.c

diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..ba67e187a4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -244,6 +244,9 @@ static int am_in_progress(const struct am_state *state)
 static int read_state_file(struct strbuf *sb, const struct am_state *state,
 			const char *file, int trim)
 {
+	int saved_errno;
+	const char *path;
+
 	strbuf_reset(sb);
 
 	if (strbuf_read_file(sb, am_path(state, file), 0) >= 0) {
@@ -256,7 +259,10 @@ static int read_state_file(struct strbuf *sb, const struct am_state *state,
 	if (errno == ENOENT)
 		return -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);
 }
 
 /**
diff --git a/builtin/commit.c b/builtin/commit.c
index 4610e3d8e3..39836b3734 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -780,8 +780,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 
 	s->fp = fopen_for_writing(git_path_commit_editmsg());
-	if (s->fp == NULL)
-		die_errno(_("could not open '%s'"), git_path_commit_editmsg());
+	if (s->fp == NULL) {
+		int saved_errno = errno;
+		const char *path = git_path_commit_editmsg();
+		errno = saved_errno;
+		die_errno(_("could not open '%s'"), path);
+	}
 
 	/* Ignore status.displayCommentPrefix: we do need comments in COMMIT_EDITMSG. */
 	old_display_comment_prefix = s->display_comment_prefix;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index c9b7946bad..e384749f73 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -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);
-		if (access(get_git_work_tree(), X_OK))
-			die_errno (_("Cannot access work tree '%s'"),
-				   get_git_work_tree());
+		if (access(get_git_work_tree(), X_OK)) {
+			int saved_errno = errno;
+			const char *path = get_git_work_tree();
+			errno = saved_errno;
+			die_errno(_("Cannot access work tree '%s'"), path);
+		}
 	}
 	else {
 		if (work_tree)
diff --git a/rerere.c b/rerere.c
index 1ce440f4bb..f19a53ff2c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -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.
 	 */
-	if (utime(rerere_path(id, "postimage"), NULL) < 0)
-		warning_errno("failed utime() on %s",
-			      rerere_path(id, "postimage"));
+	if (utime(rerere_path(id, "postimage"), NULL) < 0) {
+		int saved_errno = errno;
+		const char *path = rerere_path(id, "postimage");
+		errno = saved_errno;
+		warning_errno("failed utime() on %s", path);
+	}
 
 	/* Update "path" with the resolution */
 	f = fopen(path, "w");
diff --git a/shallow.c b/shallow.c
index df4d44ea7a..9da82f5292 100644
--- a/shallow.c
+++ b/shallow.c
@@ -295,9 +295,12 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
 		temp = xmks_tempfile(git_path("shallow_XXXXXX"));
 
 		if (write_in_full(temp->fd, sb.buf, sb.len) < 0 ||
-		    close_tempfile_gently(temp) < 0)
-			die_errno("failed to write to %s",
-				  get_tempfile_path(temp));
+		    close_tempfile_gently(temp) < 0) {
+			int saved_errno = errno;
+			const char *path = get_tempfile_path(temp);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		strbuf_release(&sb);
 		return get_tempfile_path(temp);
 	}
@@ -319,9 +322,12 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
 				       LOCK_DIE_ON_ERROR);
 	check_shallow_file_for_update();
 	if (write_shallow_commits(&sb, 0, extra)) {
-		if (write_in_full(fd, sb.buf, sb.len) < 0)
-			die_errno("failed to write to %s",
-				  get_lock_file_path(shallow_lock));
+		if (write_in_full(fd, sb.buf, sb.len) < 0) {
+			int saved_errno = errno;
+			const char *path = get_lock_file_path(shallow_lock);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		*alternate_shallow_file = get_lock_file_path(shallow_lock);
 	} else
 		/*
@@ -366,9 +372,12 @@ void prune_shallow(int show_only)
 				       LOCK_DIE_ON_ERROR);
 	check_shallow_file_for_update();
 	if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) {
-		if (write_in_full(fd, sb.buf, sb.len) < 0)
-			die_errno("failed to write to %s",
-				  get_lock_file_path(&shallow_lock));
+		if (write_in_full(fd, sb.buf, sb.len) < 0) {
+			int saved_errno = errno;
+			const char *path = get_lock_file_path(&shallow_lock);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		commit_lock_file(&shallow_lock);
 	} else {
 		unlink(git_path_shallow());
-- 
2.16.1.205.g271f633410

-- 8< --
Eric Sunshine· Feb 2, 2018, 09:46 UTC · re: Duy Nguyen · lore

Re: [PATCH v2 1/3] am: add --show-current-patch

On Fri, Feb 2, 2018 at 4:25 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 21 quoted lines
> On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> >> +       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?
>
> -       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);
Duy Nguyen· Feb 2, 2018, 09:53 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/3] am: add --show-current-patch

On Fri, Feb 2, 2018 at 4:46 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 31 quoted lines
> On Fri, Feb 2, 2018 at 4:25 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
>>> Eric Sunshine <sunshine@sunshineco.com> writes:
>>> > On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>> >> +       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?
>>
>> -       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);

That would be best. Unfortunately we have HAVE_VARIADIC_MACROS so we need to deal with no variadic macro support too.

-- 
Duy
Junio C Hamano· Feb 2, 2018, 18:56 UTC · re: Duy Nguyen · lore

Re: [PATCH v2 1/3] am: add --show-current-patch

Duy Nguyen <pclouds@gmail.com> writes:
Show 18 quoted lines
> 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 <pclouds@gmail.com>
> ---
>  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(-)
Show 18 quoted lines
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index c9b7946bad..e384749f73 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -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);
> -		if (access(get_git_work_tree(), X_OK))
> -			die_errno (_("Cannot access work tree '%s'"),
> -				   get_git_work_tree());
> +		if (access(get_git_work_tree(), X_OK)) {
> +			int saved_errno = errno;
> +			const char *path = get_git_work_tree();
> +			errno = saved_errno;
> +			die_errno(_("Cannot access work tree '%s'"), path);
> +		}
>  	}

This one is the most faithful conversion from "mechanical rewrite" point of view, but I wonder if we should instead take the returned path from get_git_work_tree() and use it in both calls. After all, this is hardly performance sensitive codepath, so even "an obviously safe but wasteful with extra xstrdup/free" version

	work_tree_path = xstrdup(get_git_work_tree());
	if (access(work_tree_path, X_OK))
		die_errno(_("msg..."), work_tree_path);
	free(work_tree_path);
may be an improvement.
Show 17 quoted lines
> diff --git a/rerere.c b/rerere.c
> index 1ce440f4bb..f19a53ff2c 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -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.
>  	 */
> -	if (utime(rerere_path(id, "postimage"), NULL) < 0)
> -		warning_errno("failed utime() on %s",
> -			      rerere_path(id, "postimage"));
> +	if (utime(rerere_path(id, "postimage"), NULL) < 0) {
> +		int saved_errno = errno;
> +		const char *path = rerere_path(id, "postimage");
> +		errno = saved_errno;
> +		warning_errno("failed utime() on %s", path);
> +	}
Likewise.
Show 17 quoted lines
> diff --git a/shallow.c b/shallow.c
> index df4d44ea7a..9da82f5292 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -295,9 +295,12 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
>  		temp = xmks_tempfile(git_path("shallow_XXXXXX"));
>  
>  		if (write_in_full(temp->fd, sb.buf, sb.len) < 0 ||
> -		    close_tempfile_gently(temp) < 0)
> -			die_errno("failed to write to %s",
> -				  get_tempfile_path(temp));
> +		    close_tempfile_gently(temp) < 0) {
> +			int saved_errno = errno;
> +			const char *path = get_tempfile_path(temp);
> +			errno = saved_errno;
> +			die_errno("failed to write to %s", path);
> +		}

It feels a bit questionable to my taste to pretend that we are truly oblivious to how trivial get_tempfile_path() is, i.e. no more than just a few field accesses to "tempfile" struct. It buries more important thing that is happening in the code in noise.

> @@ -319,9 +322,12 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
Likewise.
> @@ -366,9 +372,12 @@ void prune_shallow(int show_only)
Likewise.
All others I snipped looked like good changes.  Thanks.
Nguyễn Thái Ngọc Duy· Jan 31, 2018, 09:30 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH] gitignore.txt: elaborate shell glob syntax

`fnmatch(3)` is a great mention if the intended audience is programmers. For normal users it's probably better to spell out what a shell glob is.

This paragraph is updated to roughly tell what the main wildcards are supposed to do. All the details are still hidden away behind the `fnmatch(3)` wall because bringing the whole specification here may be too much.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/gitignore.txt | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
Show changes to Documentation/gitignore.txt +5 −6
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 63260f0056..0f4b1360bd 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -102,12 +102,11 @@ PATTERN FORMAT
    (relative to the toplevel of the work tree if not from a
    `.gitignore` file).
 
- - Otherwise, Git treats the pattern as a shell glob suitable
-   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.
 
  - A leading slash matches the beginning of the pathname.
    For example, "/{asterisk}.c" matches "cat-file.c" but not
-- 
2.16.1.205.g271f633410
Junio C Hamano· Jan 31, 2018, 23:22 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] gitignore.txt: elaborate shell glob syntax

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 13 quoted lines
>     `.gitignore` file).
>  
> - - Otherwise, Git treats the pattern as a shell glob suitable
> -   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/?

Duy Nguyen· Feb 1, 2018, 09:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitignore.txt: elaborate shell glob syntax

On Wed, Jan 31, 2018 at 03:22:46PM -0800, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Argh.. stray patches strike again. It's not supposed to be in this thread but in

https://public-inbox.org/git/%3C20180130101351.GA761@ash%3E/
Show 18 quoted lines
> 
> >     `.gitignore` file).
> >  
> > - - Otherwise, Git treats the pattern as a shell glob suitable
> > -   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 didn't. I thought I didn't add any weird symbols. I was wrong. These are now wrapped as "`stuff`" to be displayed the same way as in nearby paragraphs. Verified both man and HTML pages are rendered well.

> 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/?

Well there are rooms for guessing, for example "matches anything" does not tell you straight that it can match multiple characters. Anyway fixed too.

-- 8< --
Subject: [PATCH v2] gitignore.txt: elaborate shell glob syntax

`fnmatch(3)` is a great mention if the intended audience is programmers. For normal users it's probably better to spell out what a shell glob is.

This paragraph is updated to roughly tell (or remind) what the main wildcards are supposed to do. All the details are still hidden away behind the `fnmatch(3)` wall because bringing the whole specification here may be too much.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/gitignore.txt | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
Show changes to Documentation/gitignore.txt +5 −6
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 63260f0056..ff5d7f9ed6 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -102,12 +102,11 @@ PATTERN FORMAT
    (relative to the toplevel of the work tree if not from a
    `.gitignore` file).
 
- - Otherwise, Git treats the pattern as a shell glob suitable
-   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: "`*`" 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 detailed
+   description.
 
  - A leading slash matches the beginning of the pathname.
    For example, "/{asterisk}.c" matches "cat-file.c" but not
-- 
2.16.1.205.g271f633410

-- 8< --
Nguyễn Thái Ngọc Duy· Jan 31, 2018, 09:30 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 2/3] rebase: add --show-current-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 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 <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           |  6 ++++-
 builtin/am.c                           | 11 +++++++++
 contrib/completion/git-completion.bash |  4 ++--
 git-rebase--am.sh                      |  3 +++
 git-rebase--interactive.sh             |  3 +++
 git-rebase--merge.sh                   |  3 +++
 git-rebase.sh                          |  7 +++++-
 t/t3400-rebase.sh                      | 33 ++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  5 ++++
 9 files changed, 71 insertions(+), 4 deletions(-)
Show changes to 9 files +71 −4

Documentation/git-rebase.txt, builtin/am.c, contrib/completion/git-completion.bash, git-rebase--am.sh, git-rebase--interactive.sh, git-rebase--merge.sh, git-rebase.sh, t/t3400-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d..7ef9577472 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -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-current-patch
 
 DESCRIPTION
 -----------
@@ -250,6 +250,10 @@ 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-current-patch::
+	Show the current patch in an interactive rebase or when rebase
+	is stopped because of conflicts.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/builtin/am.c b/builtin/am.c
index 5eff1a648d..caec50cba9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2125,6 +2125,17 @@ static int show_patch(struct am_state *state)
 	struct strbuf sb = STRBUF_INIT;
 	int len;
 
+	if (!is_null_oid(&state->orig_commit)) {
+		const char *av[4] = { "show", NULL, "--", NULL };
+		char *new_oid_str;
+		int ret;
+
+		av[1] = new_oid_str = xstrdup(oid_to_hex(&state->orig_commit));
+		ret = run_command_v_opt(av, RUN_GIT_CMD);
+		free(new_oid_str);
+		return ret;
+	}
+
 	len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
 	if (len < 0)
 		die_errno(_("failed to read '%s'"),
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 56ca445fa8..2bd30d68cf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -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-current-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-current-patch"
 		return
 	fi
 	__git_complete_strategy && return
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e0..c931891cbc 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -27,6 +27,9 @@ skip)
 	move_to_original_branch
 	return
 	;;
+show-current-patch)
+	exec git am --show-current-patch
+	;;
 esac
 
 if test -z "$rebase_root"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593..0c0f8abbf9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -840,6 +840,9 @@ To continue rebase after editing, run:
 
 	exit
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/stopped-sha")" --
+	;;
 esac
 
 comment_for_reflog start
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4d..0a96dfae37 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -137,6 +137,9 @@ skip)
 	finish_rb_merge
 	return
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/current")" --
+	;;
 esac
 
 mkdir -p "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index fd72a35c65..41c915d18c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -45,6 +45,7 @@ abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
 quit!              abort but keep HEAD where it is
+show-current-patch! show the patch file being applied or merged
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -245,7 +246,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--quit|--edit-todo)
+	--continue|--skip|--abort|--quit|--edit-todo|--show-current-patch)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -412,6 +413,10 @@ quit)
 edit-todo)
 	run_specific_rebase
 	;;
+show-current-patch)
+	run_specific_rebase
+	die "BUG: run_specific_rebase is not supposed to return here"
+	;;
 esac
 
 # Make sure no rebase is in progress
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 8ac58d5ea5..09943d6a9b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -277,4 +277,37 @@ EOF
 	test_cmp From_.msg out
 '
 
+test_expect_success 'rebase--am.sh and --show-current-patch' '
+	test_create_repo conflict-apply &&
+	(
+		cd conflict-apply &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --onto init HEAD^ &&
+		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+		grep "show.*$(git rev-parse two)" stderr
+	)
+'
+
+test_expect_success 'rebase--merge.sh and --show-current-patch' '
+	test_create_repo conflict-merge &&
+	(
+		cd conflict-merge &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		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
+	)
+'
+
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 481a350090..3af6f149a9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -225,6 +225,11 @@ test_expect_success 'stop on conflicting pick' '
 	test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
 '
 
+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
+'
+
 test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
-- 
2.16.1.205.g271f633410
Nguyễn Thái Ngọc Duy· Jan 31, 2018, 09:30 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT

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 <tim@tim-landscheidt.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 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(-)
Show changes to 9 files +24 −6

Documentation/git-rebase.txt, builtin/am.c, contrib/completion/git-completion.bash, git-rebase--interactive.sh, git-rebase--merge.sh, git-rebase.sh, sequencer.c, t/t3400-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 7ef9577472..6da9296bf8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -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::
diff --git a/builtin/am.c b/builtin/am.c
index caec50cba9..bf9b356340 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -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;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2bd30d68cf..deea688e0e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -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
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c0f8abbf9..ef72bd5871 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -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
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 0a96dfae37..70966c32c3 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -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")
diff --git a/git-rebase.sh b/git-rebase.sh
index 41c915d18c..1db4301b90 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -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"
diff --git a/sequencer.c b/sequencer.c
index 4d3f60594c..fe907a0701 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -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))
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 09943d6a9b..2be4abcb7b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -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)"
 	)
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3af6f149a9..c0fe0193bb 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -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' '
-- 
2.16.1.205.g271f633410
Junio C Hamano· Jan 31, 2018, 23:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 14 quoted lines
> 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 <tim@tim-landscheidt.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

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

Show 194 quoted lines
>  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(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 7ef9577472..6da9296bf8 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -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::
> diff --git a/builtin/am.c b/builtin/am.c
> index caec50cba9..bf9b356340 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -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;
>  }
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2bd30d68cf..deea688e0e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -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
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0c0f8abbf9..ef72bd5871 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -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
> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
> index 0a96dfae37..70966c32c3 100644
> --- a/git-rebase--merge.sh
> +++ b/git-rebase--merge.sh
> @@ -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")
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 41c915d18c..1db4301b90 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -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"
> diff --git a/sequencer.c b/sequencer.c
> index 4d3f60594c..fe907a0701 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -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))
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 09943d6a9b..2be4abcb7b 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -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)"
>  	)
>  '
>  
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 3af6f149a9..c0fe0193bb 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -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' '
Duy Nguyen· Feb 1, 2018, 10:02 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT

On Thu, Feb 1, 2018 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> 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 <tim@tim-landscheidt.de>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>
> 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?

-- 
Duy
Junio C Hamano· Feb 2, 2018, 19:00 UTC · re: Duy Nguyen · lore

Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT

Duy Nguyen <pclouds@gmail.com> writes:
Show 12 quoted lines
> On Thu, Feb 1, 2018 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> 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?

I just found "ORIG_COMMIT" too similar to "ORIG_HEAD" that is totally a different thing and feared unnecessary confusion. I think you are correct to suggest that REBASE_HEAD would be more in line with the naming convention with the sequencer-like operations.

Thanks.
Phillip Wood· Feb 1, 2018, 11:05 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT

On 31/01/18 09:30, Nguyễn Thái Ngọc Duy wrote:
Show 168 quoted lines
> 
> 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 <tim@tim-landscheidt.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  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(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 7ef9577472..6da9296bf8 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -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::
> diff --git a/builtin/am.c b/builtin/am.c
> index caec50cba9..bf9b356340 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -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;
>  }
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2bd30d68cf..deea688e0e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -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
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0c0f8abbf9..ef72bd5871 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -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
> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
> index 0a96dfae37..70966c32c3 100644
> --- a/git-rebase--merge.sh
> +++ b/git-rebase--merge.sh
> @@ -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")
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 41c915d18c..1db4301b90 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -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"
> diff --git a/sequencer.c b/sequencer.c
> index 4d3f60594c..fe907a0701 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -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);

The sequencer tries to return errors rather than dying so the caller can handle them. The other callers of update_ref pass UPDATE_REFS_MSG_ON_ERR

Best Wishes
Phillip
Nguyễn Thái Ngọc Duy· Feb 11, 2018, 09:43 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v3 0/3] Add "git rebase --show-current-patch"

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:
Show changes to 9 files +26 −24

Documentation/git-rebase.txt, builtin/am.c, contrib/completion/git-completion.bash, git-rebase--interactive.sh, git-rebase--merge.sh, git-rebase.sh, sequencer.c, t/t3400-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 6da9296bf8..0b29e48221 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -253,7 +253,7 @@ 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. This is the equivalent of
-	`git show ORIG_COMMIT`.
+	`git show REBASE_HEAD`.
 
 -m::
 --merge::
diff --git a/builtin/am.c b/builtin/am.c
index bf9b356340..21aedec41f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1011,7 +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);
+	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
 	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
 		am_destroy(state);
@@ -1111,7 +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);
+	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
 	if (!get_oid("HEAD", &head))
 		write_state_text(state, "abort-safety", oid_to_hex(&head));
@@ -1443,8 +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);
+	update_ref("am", "REBASE_HEAD", &commit_oid,
+		   NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
 
 	return 0;
 }
@@ -2127,6 +2127,7 @@ static void am_abort(struct am_state *state)
 static int show_patch(struct am_state *state)
 {
 	struct strbuf sb = STRBUF_INIT;
+	const char *patch_path;
 	int len;
 
 	if (!is_null_oid(&state->orig_commit)) {
@@ -2140,10 +2141,10 @@ static int show_patch(struct am_state *state)
 		return ret;
 	}
 
-	len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
+	patch_path = am_path(state, msgnum(state));
+	len = strbuf_read_file(&sb, patch_path, 0);
 	if (len < 0)
-		die_errno(_("failed to read '%s'"),
-			  am_path(state, msgnum(state)));
+		die_errno(_("failed to read '%s'"), patch_path);
 
 	setup_pager();
 	write_in_full(1, sb.buf, sb.len);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index deea688e0e..8777805c9f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -439,7 +439,7 @@ __git_refs ()
 			track=""
 			;;
 		*)
-			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD ORIG_COMMIT; do
+			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD REBASE_HEAD; do
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ef72bd5871..a613156bcb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -199,14 +199,14 @@ make_patch () {
 
 die_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
-	git update-ref ORIG_COMMIT "$1"
+	git update-ref REBASE_HEAD "$1"
 	make_patch "$1"
 	die "$2"
 }
 
 exit_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
-	git update-ref ORIG_COMMIT "$1"
+	git update-ref REBASE_HEAD "$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")}
@@ -843,7 +843,7 @@ To continue rebase after editing, run:
 	exit
 	;;
 show-current-patch)
-	exec git show ORIG_COMMIT --
+	exec git show REBASE_HEAD --
 	;;
 esac
 
@@ -860,7 +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)"
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
 write_basic_state
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 70966c32c3..957688f236 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -57,7 +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"
+	git update-ref REBASE_HEAD "$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))"'
@@ -139,14 +139,14 @@ skip)
 	return
 	;;
 show-current-patch)
-	exec git show ORIG_COMMIT --
+	exec git show REBASE_HEAD --
 	;;
 esac
 
 mkdir -p "$state_dir"
 echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
-rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 msgnum=0
 for cmt in $(git rev-list --reverse --no-merges "$revisions")
diff --git a/git-rebase.sh b/git-rebase.sh
index 1db4301b90..a13a581fe6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -182,7 +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)"
+	rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
 	rm -rf "$state_dir"
diff --git a/sequencer.c b/sequencer.c
index fe907a0701..f692221999 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1792,8 +1792,9 @@ 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);
+	if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
+		       NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
+		res |= error(_("could not update %s"), "REBASE_HEAD");
 
 	strbuf_addf(&buf, "%s/patch", get_dir(opts));
 	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@ -2045,7 +2046,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);
+			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 		}
 		if (item->command <= TODO_SQUASH) {
 			if (is_rebase_i(opts))
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 2be4abcb7b..72d9564747 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -306,8 +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.*ORIG_COMMIT" stderr &&
-		test "$(git rev-parse ORIG_COMMIT)" = "$(git rev-parse two)"
+		grep "show.*REBASE_HEAD" stderr &&
+		test "$(git rev-parse REBASE_HEAD)" = "$(git rev-parse two)"
 	)
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c0fe0193bb..23a54a4c49 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -227,10 +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.*ORIG_COMMIT" stderr &&
+	grep "show.*REBASE_HEAD" 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 "$(git rev-parse REBASE_HEAD)" = "$stopped_sha1"
 '
 
 test_expect_success 'abort' '
-- 
2.16.1.399.g632f88eed1
Nguyễn Thái Ngọc Duy· Feb 11, 2018, 09:43 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v3 1/3] am: add --show-current-patch

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 <pclouds@gmail.com>
---
 Documentation/git-am.txt               |  6 ++++-
 builtin/am.c                           | 32 ++++++++++++++++++++++----
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh                          |  5 ++++
 4 files changed, 39 insertions(+), 6 deletions(-)
Show changes to 4 files +39 −6

Documentation/git-am.txt, builtin/am.c, contrib/completion/git-completion.bash, t/t4150-am.sh

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..0f426ae874 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
 	 [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
 	 [(<mbox> | <Maildir>)...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --show-current-patch)
 
 DESCRIPTION
 -----------
@@ -167,6 +167,10 @@ default.   You can use `--no-utf8` to override this.
 --abort::
 	Restore the original branch and abort the patching operation.
 
+--show-current-patch::
+	Show the patch being applied when "git am" is stopped because
+	of conflicts.
+
 DISCUSSION
 ----------
 
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..07abfb8f83 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1831,8 +1831,7 @@ static void am_run(struct am_state *state, int resume)
 			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
 
 			if (advice_amworkdir)
-				printf_ln(_("The copy of the patch that failed is found in: %s"),
-						am_path(state, "patch"));
+				printf_ln(_("Use 'git am --show-current-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
@@ -2121,6 +2120,23 @@ static void am_abort(struct am_state *state)
 	am_destroy(state);
 }
 
+static int show_patch(struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+	const char *patch_path;
+	int len;
+
+	patch_path = am_path(state, msgnum(state));
+	len = strbuf_read_file(&sb, patch_path, 0);
+	if (len < 0)
+		die_errno(_("failed to read '%s'"), patch_path);
+
+	setup_pager();
+	write_in_full(1, sb.buf, sb.len);
+	strbuf_release(&sb);
+	return 0;
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -2149,7 +2165,8 @@ enum resume_mode {
 	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
-	RESUME_ABORT
+	RESUME_ABORT,
+	RESUME_SHOW_PATCH
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2171,6 +2188,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	int patch_format = PATCH_FORMAT_UNKNOWN;
 	enum resume_mode resume = RESUME_FALSE;
 	int in_progress;
+	int ret = 0;
 
 	const char * const usage[] = {
 		N_("git am [<options>] [(<mbox> | <Maildir>)...]"),
@@ -2249,6 +2267,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &resume,
 			N_("restore the original branch and abort the patching operation."),
 			RESUME_ABORT),
+		OPT_CMDMODE(0, "show-current-patch", &resume,
+			N_("show the patch being applied."),
+			RESUME_SHOW_PATCH),
 		OPT_BOOL(0, "committer-date-is-author-date",
 			&state.committer_date_is_author_date,
 			N_("lie about committer date")),
@@ -2359,11 +2380,14 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_ABORT:
 		am_abort(&state);
 		break;
+	case RESUME_SHOW_PATCH:
+		ret = show_patch(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
 
 	am_state_release(&state);
 
-	return 0;
+	return ret;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3683c772c5..56ca445fa8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort --show-current-patch"
 		return
 	fi
 	case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..23abf42abc 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -662,6 +662,11 @@ test_expect_success 'am pauses on conflict' '
 	test -d .git/rebase-apply
 '
 
+test_expect_success 'am --show-current-patch' '
+	git am --show-current-patch >actual.patch &&
+	test_cmp .git/rebase-apply/0001 actual.patch
+'
+
 test_expect_success 'am --skip works' '
 	echo goodbye >expected &&
 	git am --skip &&
-- 
2.16.1.399.g632f88eed1
Nguyễn Thái Ngọc Duy· Feb 11, 2018, 09:43 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v3 2/3] rebase: add --show-current-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 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 <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           |  6 ++++-
 builtin/am.c                           | 11 +++++++++
 contrib/completion/git-completion.bash |  4 ++--
 git-rebase--am.sh                      |  3 +++
 git-rebase--interactive.sh             |  3 +++
 git-rebase--merge.sh                   |  3 +++
 git-rebase.sh                          |  7 +++++-
 t/t3400-rebase.sh                      | 33 ++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  5 ++++
 9 files changed, 71 insertions(+), 4 deletions(-)
Show changes to 9 files +71 −4

Documentation/git-rebase.txt, builtin/am.c, contrib/completion/git-completion.bash, git-rebase--am.sh, git-rebase--interactive.sh, git-rebase--merge.sh, git-rebase.sh, t/t3400-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d..7ef9577472 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -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-current-patch
 
 DESCRIPTION
 -----------
@@ -250,6 +250,10 @@ 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-current-patch::
+	Show the current patch in an interactive rebase or when rebase
+	is stopped because of conflicts.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/builtin/am.c b/builtin/am.c
index 07abfb8f83..37219fceb0 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2126,6 +2126,17 @@ static int show_patch(struct am_state *state)
 	const char *patch_path;
 	int len;
 
+	if (!is_null_oid(&state->orig_commit)) {
+		const char *av[4] = { "show", NULL, "--", NULL };
+		char *new_oid_str;
+		int ret;
+
+		av[1] = new_oid_str = xstrdup(oid_to_hex(&state->orig_commit));
+		ret = run_command_v_opt(av, RUN_GIT_CMD);
+		free(new_oid_str);
+		return ret;
+	}
+
 	patch_path = am_path(state, msgnum(state));
 	len = strbuf_read_file(&sb, patch_path, 0);
 	if (len < 0)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 56ca445fa8..2bd30d68cf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -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-current-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-current-patch"
 		return
 	fi
 	__git_complete_strategy && return
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e0..c931891cbc 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -27,6 +27,9 @@ skip)
 	move_to_original_branch
 	return
 	;;
+show-current-patch)
+	exec git am --show-current-patch
+	;;
 esac
 
 if test -z "$rebase_root"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593..0c0f8abbf9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -840,6 +840,9 @@ To continue rebase after editing, run:
 
 	exit
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/stopped-sha")" --
+	;;
 esac
 
 comment_for_reflog start
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4d..0a96dfae37 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -137,6 +137,9 @@ skip)
 	finish_rb_merge
 	return
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/current")" --
+	;;
 esac
 
 mkdir -p "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index fd72a35c65..41c915d18c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -45,6 +45,7 @@ abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
 quit!              abort but keep HEAD where it is
+show-current-patch! show the patch file being applied or merged
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -245,7 +246,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--quit|--edit-todo)
+	--continue|--skip|--abort|--quit|--edit-todo|--show-current-patch)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -412,6 +413,10 @@ quit)
 edit-todo)
 	run_specific_rebase
 	;;
+show-current-patch)
+	run_specific_rebase
+	die "BUG: run_specific_rebase is not supposed to return here"
+	;;
 esac
 
 # Make sure no rebase is in progress
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 8ac58d5ea5..09943d6a9b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -277,4 +277,37 @@ EOF
 	test_cmp From_.msg out
 '
 
+test_expect_success 'rebase--am.sh and --show-current-patch' '
+	test_create_repo conflict-apply &&
+	(
+		cd conflict-apply &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --onto init HEAD^ &&
+		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+		grep "show.*$(git rev-parse two)" stderr
+	)
+'
+
+test_expect_success 'rebase--merge.sh and --show-current-patch' '
+	test_create_repo conflict-merge &&
+	(
+		cd conflict-merge &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		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
+	)
+'
+
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 481a350090..3af6f149a9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -225,6 +225,11 @@ test_expect_success 'stop on conflicting pick' '
 	test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
 '
 
+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
+'
+
 test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
-- 
2.16.1.399.g632f88eed1
Nguyễn Thái Ngọc Duy· Feb 11, 2018, 09:43 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v3 3/3] rebase: introduce and use pseudo-ref REBASE_HEAD

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

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 <tim@tim-landscheidt.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 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                            | 4 ++++
 t/t3400-rebase.sh                      | 3 ++-
 t/t3404-rebase-interactive.sh          | 5 ++++-
 9 files changed, 25 insertions(+), 6 deletions(-)
Show changes to 9 files +25 −6

Documentation/git-rebase.txt, builtin/am.c, contrib/completion/git-completion.bash, git-rebase--interactive.sh, git-rebase--merge.sh, git-rebase.sh, sequencer.c, t/t3400-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 7ef9577472..0b29e48221 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -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 REBASE_HEAD`.
 
 -m::
 --merge::
diff --git a/builtin/am.c b/builtin/am.c
index 37219fceb0..21aedec41f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -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, "REBASE_HEAD", 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, "REBASE_HEAD", 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", "REBASE_HEAD", &commit_oid,
+		   NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
 
 	return 0;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2bd30d68cf..8777805c9f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -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 REBASE_HEAD; do
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c0f8abbf9..a613156bcb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -199,12 +199,14 @@ make_patch () {
 
 die_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref REBASE_HEAD "$1"
 	make_patch "$1"
 	die "$2"
 }
 
 exit_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref REBASE_HEAD "$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 REBASE_HEAD --
 	;;
 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 REBASE_HEAD)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
 write_basic_state
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 0a96dfae37..957688f236 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -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 REBASE_HEAD "$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 REBASE_HEAD --
 	;;
 esac
 
 mkdir -p "$state_dir"
 echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 msgnum=0
 for cmt in $(git rev-list --reverse --no-merges "$revisions")
diff --git a/git-rebase.sh b/git-rebase.sh
index 41c915d18c..a13a581fe6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -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 REBASE_HEAD)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
 	rm -rf "$state_dir"
diff --git a/sequencer.c b/sequencer.c
index 4d3f60594c..f692221999 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1792,6 +1792,9 @@ 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;
+	if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
+		       NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
+		res |= error(_("could not update %s"), "REBASE_HEAD");
 
 	strbuf_addf(&buf, "%s/patch", get_dir(opts));
 	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@ -2043,6 +2046,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, "REBASE_HEAD", NULL, REF_NO_DEREF);
 		}
 		if (item->command <= TODO_SQUASH) {
 			if (is_rebase_i(opts))
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 09943d6a9b..72d9564747 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -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.*REBASE_HEAD" stderr &&
+		test "$(git rev-parse REBASE_HEAD)" = "$(git rev-parse two)"
 	)
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3af6f149a9..23a54a4c49 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -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.*REBASE_HEAD" stderr &&
+	# the original stopped-sha1 is abbreviated
+	stopped_sha1="$(git rev-parse $(cat ".git/rebase-merge/stopped-sha"))" &&
+	test "$(git rev-parse REBASE_HEAD)" = "$stopped_sha1"
 '
 
 test_expect_success 'abort' '
-- 
2.16.1.399.g632f88eed1
Junio C Hamano· Feb 22, 2018, 00:21 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 8 quoted 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
>
> 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...

Tim Landscheidt· Feb 22, 2018, 00:54 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"

Junio C Hamano <gitster@pobox.com> wrote:
>> 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:
> 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 :-).

(BTW, thanks, Duy, for doing the actual work!)
Tim
Duy Nguyen· Feb 23, 2018, 10:20 UTC · re: Tim Landscheidt · lore

Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"

On Thu, Feb 22, 2018 at 7:54 AM, Tim Landscheidt <tim@tim-landscheidt.de> wrote:
Show 25 quoted lines
> Junio C Hamano <gitster@pobox.com> wrote:
>
>>> 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:
>
>> 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 :-).

I wonder if you could make a ref symlink named ORIG_COMMIT pointing to REBASE_HEAD. A bit more setup for you, but that'll make everybody happy.

-- 
Duy

← back to recent threads