[PATCH 0/2] checkout/restore: fix `git checkout -p HEAD...` bug

[PATCH 0/2] checkout/restore: fix `git checkout -p HEAD...` bug

[Denton Liu]

As reported earlier[0], `git checkout -p HEAD...` results in the following error:

	$ git checkout -p HEAD...
	usage: git diff-index [-m] [--cached] [<common-diff-options>] <tree-ish> [<path>...]
	common diff options:
	  -z            output diff-raw with lines terminated with NUL.
	  -p            output patch format.
	  -u            synonym for -p.
	  --patch-with-raw
			output both a patch and the diff-raw format.
	  --stat        show diffstat instead of patch.
	  --numstat     show numeric diffstat instead of patch.
	  --patch-with-stat
			output a patch and prepend its diffstat.
	  --name-only   show only names of changed files.
	  --name-status show names and status of changed files.
	  --full-index  show full object name on index lines.
	  --abbrev=<n>  abbreviate object names in diff-tree header and diff-raw.
	  -R            swap input file pairs.
	  -B            detect complete rewrites.
	  -M            detect renames.
	  -C            detect copies.
	  --find-copies-harder
			try unchanged files as candidate for copy detection.
	  -l<n>         limit rename attempts up to <n> paths.
	  -O<file>      reorder diffs according to the <file>.
	  -S<string>    find filepair whose only one side contains the string.
	  --pickaxe-all
			show all files diff when -S is used and hit is found.
	  -a  --text    treat all files as text.

	Cannot close git diff-index --cached --numstat --summary HEAD... -- () at <redacted>/libexec/git-core/git-add--interactive line 183.

Since it was decided that <tree-ish> should also officially support the
merge-base revs, fix `git checkout -p <rev>...` and officially document the
support.

[0]: https://lore.kernel.org/git/20200929061830.GA40759@generichostname/

Denton Liu (2):
  builtin/checkout: fix `git checkout -p HEAD...` bug
  Doc: document "A...B" form for <tree-ish> in checkout and switch

 Documentation/git-checkout.txt |  4 ++++
 Documentation/git-restore.txt  |  4 ++++
 builtin/checkout.c             | 14 +++++++++++++-
 t/t2016-checkout-patch.sh      |  7 +++++++
 t/t2071-restore-patch.sh       |  8 ++++++++
 5 files changed, 36 insertions(+), 1 deletion(-)

-- 
2.28.0.942.g77c4c6094c

[PATCH 1/2] builtin/checkout: fix `git checkout -p HEAD...` bug

[Denton Liu]

Running `git checkout -p` with a merge-base rev results in an error:

	$ git checkout -p HEAD...
	usage: git diff-index [-m] [--cached] [<common-diff-options>] <tree-ish> [<path>...]
	common diff options:
	  -z            output diff-raw with lines terminated with NUL.
	  -p            output patch format.
	  -u            synonym for -p.
	  --patch-with-raw
			output both a patch and the diff-raw format.
	  --stat        show diffstat instead of patch.
	  --numstat     show numeric diffstat instead of patch.
	  --patch-with-stat
			output a patch and prepend its diffstat.
	  --name-only   show only names of changed files.
	  --name-status show names and status of changed files.
	  --full-index  show full object name on index lines.
	  --abbrev=<n>  abbreviate object names in diff-tree header and diff-raw.
	  -R            swap input file pairs.
	  -B            detect complete rewrites.
	  -M            detect renames.
	  -C            detect copies.
	  --find-copies-harder
			try unchanged files as candidate for copy detection.
	  -l<n>         limit rename attempts up to <n> paths.
	  -O<file>      reorder diffs according to the <file>.
	  -S<string>    find filepair whose only one side contains the string.
	  --pickaxe-all
			show all files diff when -S is used and hit is found.
	  -a  --text    treat all files as text.

	Cannot close git diff-index --cached --numstat --summary HEAD... -- () at <redacted>/libexec/git-core/git-add--interactive line 183.

This happens because checkout passes the literal argument (in the
example, `HEAD...`) to diff-index which does not recognise merge-base
revs.

Fix this by using the hex of the found commit instead of the given name.
Note that "HEAD" is handled specially in run_add_interactive() so it's
explicitly not changed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/checkout.c        | 14 +++++++++++++-
 t/t2016-checkout-patch.sh |  7 +++++++
 t/t2071-restore-patch.sh  |  8 ++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0951f8fee5..95122e16ed 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -471,6 +471,18 @@ static int checkout_paths(const struct checkout_opts *opts,
 
 	if (opts->patch_mode) {
 		const char *patch_mode;
+		const char *rev = new_branch_info->name;
+		char rev_oid[GIT_MAX_HEXSZ + 1];
+
+		/*
+		 * Since rev can be in the form of `<a>...<b>`, we must replace
+		 * the name with the hex of the commit for the
+		 * run_add_interactive() machinery to work properly. However,
+		 * there is special logic for the HEAD case so we mustn't
+		 * replace that.
+		 */
+		if (rev && strcmp(rev, "HEAD"))
+			rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);
 
 		if (opts->checkout_index && opts->checkout_worktree)
 			patch_mode = "--patch=checkout";
@@ -481,7 +493,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 		else
 			BUG("either flag must have been set, worktree=%d, index=%d",
 			    opts->checkout_worktree, opts->checkout_index);
-		return run_add_interactive(new_branch_info->name, patch_mode, &opts->pathspec);
+		return run_add_interactive(rev, patch_mode, &opts->pathspec);
 	}
 
 	repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index 47aeb0b167..999620e507 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -59,6 +59,13 @@ test_expect_success PERL 'git checkout -p HEAD with change already staged' '
 	verify_state dir/foo head head
 '
 
+test_expect_success PERL 'git checkout -p HEAD^...' '
+	# the third n is to get out in case it mistakenly does not apply
+	test_write_lines n y n | git checkout -p HEAD^... &&
+	verify_saved_state bar &&
+	verify_state dir/foo parent parent
+'
+
 test_expect_success PERL 'git checkout -p HEAD^' '
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines n y n | git checkout -p HEAD^ &&
diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh
index 98b2476e7c..b5c5c0ff7e 100755
--- a/t/t2071-restore-patch.sh
+++ b/t/t2071-restore-patch.sh
@@ -60,6 +60,14 @@ test_expect_success PERL 'git restore -p --source=HEAD^' '
 	verify_state dir/foo parent index
 '
 
+test_expect_success PERL 'git restore -p --source=HEAD^...' '
+	set_state dir/foo work index &&
+	# the third n is to get out in case it mistakenly does not apply
+	test_write_lines n y n | git restore -p --source=HEAD^... &&
+	verify_saved_state bar &&
+	verify_state dir/foo parent index
+'
+
 test_expect_success PERL 'git restore -p handles deletion' '
 	set_state dir/foo work index &&
 	rm dir/foo &&
-- 
2.28.0.942.g77c4c6094c

Re: [PATCH 1/2] builtin/checkout: fix `git checkout -p HEAD...` bug

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> Fix this by using the hex of the found commit instead of the given name.
> Note that "HEAD" is handled specially in run_add_interactive() so it's
> explicitly not changed.
> ...
> +		const char *rev = new_branch_info->name;
> +		char rev_oid[GIT_MAX_HEXSZ + 1];
> +
> +		/*
> +		 * Since rev can be in the form of `<a>...<b>`, we must replace
> +		 * the name with the hex of the commit for the
> +		 * run_add_interactive() machinery to work properly. However,
> +		 * there is special logic for the HEAD case so we mustn't
> +		 * replace that.
> +		 */
> +		if (rev && strcmp(rev, "HEAD"))
> +			rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);

What the comment explains and the if condition special cases are
different.  Surely, HEAD is treated differently, but the natural
implementation of what the comment wants to achieve would be to not
just make sure it is not HEAD, but to make sure that it is of <a>...<b>
form, e.g.

		if (rev && strstr(rev, "..."))

Having said that, I do like how your version looks like in two ways,
i.e. it makes it clear HEAD is special, and it does not limit the
special case to only the three-dot "merge-base" magic, which might
be more futureproof.  On the other hand, your version would give the
hexadecimal commit object name even when a plain-vanilla branch name
is given and the above comment does not say if and why it is a safe
thing to do.  We used to give "new_branch_info->name" to the helper
that invokes "add -p".  Now we always give a hexadecimal or "HEAD".

Come to think of it, perhaps "add -p" that special cases HEAD is a
mistake.  If a plain vanilla branch name can be safely replaced by
the hexadecimal commit object name, perhaps the code should be
prepared to special case not just string "HEAD" but any other way to
express the commit object referred to with "HEAD" the same way.  But
that is not a suggestion for this patch---it may be a good idea to
add NEEDSWORK: comment here to encourage our future selves to see if
"add -p" needs to be fixed.

>  		if (opts->checkout_index && opts->checkout_worktree)
>  			patch_mode = "--patch=checkout";
> @@ -481,7 +493,7 @@ static int checkout_paths(const struct checkout_opts *opts,
>  		else
>  			BUG("either flag must have been set, worktree=%d, index=%d",
>  			    opts->checkout_worktree, opts->checkout_index);
> -		return run_add_interactive(new_branch_info->name, patch_mode, &opts->pathspec);
> +		return run_add_interactive(rev, patch_mode, &opts->pathspec);
>  	}
>  
>  	repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
> diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
> index 47aeb0b167..999620e507 100755
> --- a/t/t2016-checkout-patch.sh
> +++ b/t/t2016-checkout-patch.sh
> @@ -59,6 +59,13 @@ test_expect_success PERL 'git checkout -p HEAD with change already staged' '
>  	verify_state dir/foo head head
>  '
>  
> +test_expect_success PERL 'git checkout -p HEAD^...' '
> +	# the third n is to get out in case it mistakenly does not apply
> +	test_write_lines n y n | git checkout -p HEAD^... &&
> +	verify_saved_state bar &&
> +	verify_state dir/foo parent parent
> +'
> +
>  test_expect_success PERL 'git checkout -p HEAD^' '
>  	# the third n is to get out in case it mistakenly does not apply
>  	test_write_lines n y n | git checkout -p HEAD^ &&
> diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh
> index 98b2476e7c..b5c5c0ff7e 100755
> --- a/t/t2071-restore-patch.sh
> +++ b/t/t2071-restore-patch.sh
> @@ -60,6 +60,14 @@ test_expect_success PERL 'git restore -p --source=HEAD^' '
>  	verify_state dir/foo parent index
>  '
>  
> +test_expect_success PERL 'git restore -p --source=HEAD^...' '
> +	set_state dir/foo work index &&
> +	# the third n is to get out in case it mistakenly does not apply
> +	test_write_lines n y n | git restore -p --source=HEAD^... &&
> +	verify_saved_state bar &&
> +	verify_state dir/foo parent index
> +'
> +
>  test_expect_success PERL 'git restore -p handles deletion' '
>  	set_state dir/foo work index &&
>  	rm dir/foo &&

You are just mimicking what is already there, but I thought "add -p"
can use built-in replacement when GIT_TEST_ADD_I_USE_BUILTIN is
given these days.  Perhaps we need to replace these PERL
prerequisite with a new one, say ADD_I, which requires PERL if and
only if GIT_TEST_ADD_I_USE_BUILTIN is not set, or something.  Again,
that is not in the scope of this patch, but leaving NEEDSWORK:
comment in these test scripts where they first use PERL prerequisite
but they would use ADD_I prerequisite if available.

[PATCH 2/2] Doc: document "A...B" form for <tree-ish> in checkout and switch

[Denton Liu]

Using "A...B" has been supported for the <tree-ish> argument for a
while. However, its support has never been explicitly documented.

Explicitly document it so that users know that it is available.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-checkout.txt | 4 ++++
 Documentation/git-restore.txt  | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index afa5c11fd3..6ae8bf8dee 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -351,6 +351,10 @@ leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 <tree-ish>::
 	Tree to checkout from (when paths are given). If not specified,
 	the index will be used.
++
+As a special case, you may use `"A...B"` as a shortcut for the
+merge base of `A` and `B` if there is exactly one merge base. You can
+leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 
 \--::
 	Do not interpret any more arguments as options.
diff --git a/Documentation/git-restore.txt b/Documentation/git-restore.txt
index 84c6c40010..55bde91ef9 100644
--- a/Documentation/git-restore.txt
+++ b/Documentation/git-restore.txt
@@ -40,6 +40,10 @@ OPTIONS
 +
 If not specified, the contents are restored from `HEAD` if `--staged` is
 given, otherwise from the index.
++
+As a special case, you may use `"A...B"` as a shortcut for the
+merge base of `A` and `B` if there is exactly one merge base. You can
+leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 
 -p::
 --patch::
-- 
2.28.0.942.g77c4c6094c

[PATCH v2 0/4] checkout/restore: fix `git checkout -p HEAD...` bug

[Denton Liu]

As reported earlier[0], `git checkout -p HEAD...` results in the following error:

	$ git checkout -p HEAD...
	usage: git diff-index [-m] [--cached] [<common-diff-options>] <tree-ish> [<path>...]
	common diff options:
	  -z            output diff-raw with lines terminated with NUL.
	  -p            output patch format.
	  -u            synonym for -p.
	  --patch-with-raw
			output both a patch and the diff-raw format.
	  --stat        show diffstat instead of patch.
	  --numstat     show numeric diffstat instead of patch.
	  --patch-with-stat
			output a patch and prepend its diffstat.
	  --name-only   show only names of changed files.
	  --name-status show names and status of changed files.
	  --full-index  show full object name on index lines.
	  --abbrev=<n>  abbreviate object names in diff-tree header and diff-raw.
	  -R            swap input file pairs.
	  -B            detect complete rewrites.
	  -M            detect renames.
	  -C            detect copies.
	  --find-copies-harder
			try unchanged files as candidate for copy detection.
	  -l<n>         limit rename attempts up to <n> paths.
	  -O<file>      reorder diffs according to the <file>.
	  -S<string>    find filepair whose only one side contains the string.
	  --pickaxe-all
			show all files diff when -S is used and hit is found.
	  -a  --text    treat all files as text.

	Cannot close git diff-index --cached --numstat --summary HEAD... -- () at <redacted>/libexec/git-core/git-add--interactive line 183.

Since it was decided that <tree-ish> should also officially support the
merge-base revs, fix `git checkout -p <rev>...` and officially document the
support.

Changes since v1:

* Add two NEEDSWORK commits

* Be more precise in the inline code comment

[0]: https://lore.kernel.org/git/20200929061830.GA40759@generichostname/

Denton Liu (4):
  builtin/checkout: fix `git checkout -p HEAD...` bug
  Doc: document "A...B" form for <tree-ish> in checkout and switch
  add-patch: add NEEDSWORK about comparing commits
  t2016: add a NEEDSWORK about the PERL prerequisite

 Documentation/git-checkout.txt |  4 ++++
 Documentation/git-restore.txt  |  4 ++++
 add-patch.c                    |  8 ++++++++
 builtin/checkout.c             | 15 ++++++++++++++-
 git-add--interactive.perl      |  7 +++++++
 t/t2016-checkout-patch.sh      | 11 +++++++++++
 t/t2071-restore-patch.sh       |  8 ++++++++
 7 files changed, 56 insertions(+), 1 deletion(-)

Range-diff against v1:
1:  54f221411f ! 1:  723d74febe builtin/checkout: fix `git checkout -p HEAD...` bug
    @@ builtin/checkout.c: static int checkout_paths(const struct checkout_opts *opts,
     +		char rev_oid[GIT_MAX_HEXSZ + 1];
     +
     +		/*
    -+		 * Since rev can be in the form of `<a>...<b>`, we must replace
    -+		 * the name with the hex of the commit for the
    -+		 * run_add_interactive() machinery to work properly. However,
    -+		 * there is special logic for the HEAD case so we mustn't
    -+		 * replace that.
    ++		 * Since rev can be in the form of `<a>...<b>` (which is not
    ++		 * recognized by diff-index), we will always replace the name
    ++		 * with the hex of the commit (whether it's in `...` form or
    ++		 * not) for the run_add_interactive() machinery to work
    ++		 * properly. However, there is special logic for the HEAD case
    ++		 * so we mustn't replace that.
     +		 */
     +		if (rev && strcmp(rev, "HEAD"))
     +			rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);
2:  75920c1a25 = 2:  a0ed2f313d Doc: document "A...B" form for <tree-ish> in checkout and switch
-:  ---------- > 3:  44ee78450e add-patch: add NEEDSWORK about comparing commits
-:  ---------- > 4:  a20544f18a t2016: add a NEEDSWORK about the PERL prerequisite
-- 
2.28.0.942.g77c4c6094c

[PATCH v2 1/4] builtin/checkout: fix `git checkout -p HEAD...` bug

[Denton Liu]

Running `git checkout -p` with a merge-base rev results in an error:

	$ git checkout -p HEAD...
	usage: git diff-index [-m] [--cached] [<common-diff-options>] <tree-ish> [<path>...]
	common diff options:
	  -z            output diff-raw with lines terminated with NUL.
	  -p            output patch format.
	  -u            synonym for -p.
	  --patch-with-raw
			output both a patch and the diff-raw format.
	  --stat        show diffstat instead of patch.
	  --numstat     show numeric diffstat instead of patch.
	  --patch-with-stat
			output a patch and prepend its diffstat.
	  --name-only   show only names of changed files.
	  --name-status show names and status of changed files.
	  --full-index  show full object name on index lines.
	  --abbrev=<n>  abbreviate object names in diff-tree header and diff-raw.
	  -R            swap input file pairs.
	  -B            detect complete rewrites.
	  -M            detect renames.
	  -C            detect copies.
	  --find-copies-harder
			try unchanged files as candidate for copy detection.
	  -l<n>         limit rename attempts up to <n> paths.
	  -O<file>      reorder diffs according to the <file>.
	  -S<string>    find filepair whose only one side contains the string.
	  --pickaxe-all
			show all files diff when -S is used and hit is found.
	  -a  --text    treat all files as text.

	Cannot close git diff-index --cached --numstat --summary HEAD... -- () at <redacted>/libexec/git-core/git-add--interactive line 183.

This happens because checkout passes the literal argument (in the
example, `HEAD...`) to diff-index which does not recognise merge-base
revs.

Fix this by using the hex of the found commit instead of the given name.
Note that "HEAD" is handled specially in run_add_interactive() so it's
explicitly not changed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/checkout.c        | 15 ++++++++++++++-
 t/t2016-checkout-patch.sh |  7 +++++++
 t/t2071-restore-patch.sh  |  8 ++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0951f8fee5..2e69ed24f2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -471,6 +471,19 @@ static int checkout_paths(const struct checkout_opts *opts,
 
 	if (opts->patch_mode) {
 		const char *patch_mode;
+		const char *rev = new_branch_info->name;
+		char rev_oid[GIT_MAX_HEXSZ + 1];
+
+		/*
+		 * Since rev can be in the form of `<a>...<b>` (which is not
+		 * recognized by diff-index), we will always replace the name
+		 * with the hex of the commit (whether it's in `...` form or
+		 * not) for the run_add_interactive() machinery to work
+		 * properly. However, there is special logic for the HEAD case
+		 * so we mustn't replace that.
+		 */
+		if (rev && strcmp(rev, "HEAD"))
+			rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);
 
 		if (opts->checkout_index && opts->checkout_worktree)
 			patch_mode = "--patch=checkout";
@@ -481,7 +494,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 		else
 			BUG("either flag must have been set, worktree=%d, index=%d",
 			    opts->checkout_worktree, opts->checkout_index);
-		return run_add_interactive(new_branch_info->name, patch_mode, &opts->pathspec);
+		return run_add_interactive(rev, patch_mode, &opts->pathspec);
 	}
 
 	repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index 47aeb0b167..999620e507 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -59,6 +59,13 @@ test_expect_success PERL 'git checkout -p HEAD with change already staged' '
 	verify_state dir/foo head head
 '
 
+test_expect_success PERL 'git checkout -p HEAD^...' '
+	# the third n is to get out in case it mistakenly does not apply
+	test_write_lines n y n | git checkout -p HEAD^... &&
+	verify_saved_state bar &&
+	verify_state dir/foo parent parent
+'
+
 test_expect_success PERL 'git checkout -p HEAD^' '
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines n y n | git checkout -p HEAD^ &&
diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh
index 98b2476e7c..b5c5c0ff7e 100755
--- a/t/t2071-restore-patch.sh
+++ b/t/t2071-restore-patch.sh
@@ -60,6 +60,14 @@ test_expect_success PERL 'git restore -p --source=HEAD^' '
 	verify_state dir/foo parent index
 '
 
+test_expect_success PERL 'git restore -p --source=HEAD^...' '
+	set_state dir/foo work index &&
+	# the third n is to get out in case it mistakenly does not apply
+	test_write_lines n y n | git restore -p --source=HEAD^... &&
+	verify_saved_state bar &&
+	verify_state dir/foo parent index
+'
+
 test_expect_success PERL 'git restore -p handles deletion' '
 	set_state dir/foo work index &&
 	rm dir/foo &&
-- 
2.28.0.942.g77c4c6094c

[PATCH v2 2/4] Doc: document "A...B" form for <tree-ish> in checkout and switch

[Denton Liu]

Using "A...B" has been supported for the <tree-ish> argument for a
while. However, its support has never been explicitly documented.

Explicitly document it so that users know that it is available.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-checkout.txt | 4 ++++
 Documentation/git-restore.txt  | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index afa5c11fd3..6ae8bf8dee 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -351,6 +351,10 @@ leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 <tree-ish>::
 	Tree to checkout from (when paths are given). If not specified,
 	the index will be used.
++
+As a special case, you may use `"A...B"` as a shortcut for the
+merge base of `A` and `B` if there is exactly one merge base. You can
+leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 
 \--::
 	Do not interpret any more arguments as options.
diff --git a/Documentation/git-restore.txt b/Documentation/git-restore.txt
index 84c6c40010..55bde91ef9 100644
--- a/Documentation/git-restore.txt
+++ b/Documentation/git-restore.txt
@@ -40,6 +40,10 @@ OPTIONS
 +
 If not specified, the contents are restored from `HEAD` if `--staged` is
 given, otherwise from the index.
++
+As a special case, you may use `"A...B"` as a shortcut for the
+merge base of `A` and `B` if there is exactly one merge base. You can
+leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 
 -p::
 --patch::
-- 
2.28.0.942.g77c4c6094c

[PATCH v2 3/4] add-patch: add NEEDSWORK about comparing commits

[Denton Liu]

The two versions of add-patch has special-casing for the literal
revision "HEAD". However, we want to handle other ways of saying "HEAD"
in the same way.[0] Add a NEEDSWORK to the add-patch code that does this
so that it can be addressed later.

[0]: https://lore.kernel.org/git/xmqqsgat7ttf.fsf@gitster.c.googlers.com/

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 add-patch.c               | 8 ++++++++
 git-add--interactive.perl | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/add-patch.c b/add-patch.c
index bd94bd3a7c..be4cf6e9e5 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1695,6 +1695,14 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 	if (mode == ADD_P_STASH)
 		s.mode = &patch_mode_stash;
 	else if (mode == ADD_P_RESET) {
+		/*
+		 * NEEDSWORK: Instead of comparing to the literal "HEAD",
+		 * compare the commit objects instead so that other ways of
+		 * saying the same thing (such as "@") are also handled
+		 * appropriately.
+		 *
+		 * This applies to the cases below too.
+		 */
 		if (!revision || !strcmp(revision, "HEAD"))
 			s.mode = &patch_mode_reset_head;
 		else
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 8a72018712..e713fe3d02 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1830,6 +1830,13 @@ sub process_args {
 				$arg = shift @ARGV or die __("missing --");
 				if ($arg ne '--') {
 					$patch_mode_revision = $arg;
+
+					# NEEDSWORK: Instead of comparing to the literal "HEAD",
+					# compare the commit objects instead so that other ways of
+					# saying the same thing (such as "@") are also handled
+					# appropriately.
+					#
+					# This applies to the cases below too.
 					$patch_mode = ($arg eq 'HEAD' ?
 						       'reset_head' : 'reset_nothead');
 					$arg = shift @ARGV or die __("missing --");
-- 
2.28.0.942.g77c4c6094c

[PATCH v2 4/4] t2016: add a NEEDSWORK about the PERL prerequisite

[Denton Liu]

Since the builtin add-p is used when $GIT_TEST_ADD_I_USE_BUILTIN is
given, we should replace the PERL prerequisite with an ADD_I
prerequisite which first checks if $GIT_TEST_ADD_I_USE_BUILTIN is
defined before checking PERL.[0] Mark this in a NEEDSWORK so that it can
be addressed at a later time.

[0]: https://lore.kernel.org/git/xmqqsgat7ttf.fsf@gitster.c.googlers.com/

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2016-checkout-patch.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index 999620e507..d91a329eb3 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -18,6 +18,10 @@ test_expect_success PERL 'setup' '
 
 # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'
 
+# NEEDSWORK: Since the builtin add-p is used when $GIT_TEST_ADD_I_USE_BUILTIN
+# is given, we should replace the PERL prerequisite with an ADD_I prerequisite
+# which first checks if $GIT_TEST_ADD_I_USE_BUILTIN is defined before checking
+# PERL.
 test_expect_success PERL 'saying "n" does nothing' '
 	set_and_save_state dir/foo work head &&
 	test_write_lines n n | git checkout -p &&
-- 
2.28.0.942.g77c4c6094c