Re: [PATCH v3 1/1] rebase: fix --fork-point with short refname

> Subject: rebase: --fork-point regression fix
> 
> "git rebase --fork-point master" used to work OK, as it internally
> called "git merge-base --fork-point" that knew how to handle short
> refname and dwim it to the full refname before calling the
> underlying get_fork_point() function.
> 
> This is no longer true after the command was rewritten in C, as its
> internall call made directly to get_fork_point() does not dwim a
> short ref.
> 
> Move the "dwim the refname argument to the full refname" logic that
> is used in "git merge-base" to the underlying get_fork_point()
> function, so that the other caller of the function in the
> implementation of "git rebase" behaves the same way to fix this
> regression.
> 
> ---
>  builtin/merge-base.c         | 12 +-----------
>  commit.c                     | 15 +++++++++++++--
>  t/t3431-rebase-fork-point.sh | 20 ++++++++++++++++++++
>  3 files changed, 34 insertions(+), 13 deletions(-)
> 
> diff --git a/builtin/merge-base.c b/builtin/merge-base.c
> index e3f8da13b6..6719ac198d 100644
> --- a/builtin/merge-base.c
> +++ b/builtin/merge-base.c
> @@ -114,26 +114,16 @@ static int handle_is_ancestor(int argc, const char **argv)
>  static int handle_fork_point(int argc, const char **argv)
>  {
>  	struct object_id oid;
> -	char *refname;
>  	struct commit *derived, *fork_point;
>  	const char *commitname;
>  
> -	switch (dwim_ref(argv[0], strlen(argv[0]), &oid, &refname)) {
> -	case 0:
> -		die("No such ref: '%s'", argv[0]);
> -	case 1:
> -		break; /* good */
> -	default:
> -		die("Ambiguous refname: '%s'", argv[0]);
> -	}
> -

>  	commitname = (argc == 2) ? argv[1] : "HEAD";
>  	if (get_oid(commitname, &oid))
>  		die("Not a valid object name: '%s'", commitname);
>  
>  	derived = lookup_commit_reference(the_repository, &oid);
>  
> -	fork_point = get_fork_point(refname, derived);
> +	fork_point = get_fork_point(argv[0], derived);
>  
>  	if (!fork_point)
>  		return 1;
> diff --git a/commit.c b/commit.c
> index 40890ae7ce..016f14fe95 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -903,12 +903,22 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
>  	struct commit_list *bases;
>  	int i;
>  	struct commit *ret = NULL;
> +	char *full_refname;
> +
> +	switch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {
> +	case 0:
> +		die("No such ref: '%s'", refname);
> +	case 1:
> +		break; /* good */
> +	default:
> +		die("Ambiguous refname: '%s'", refname);
> +	}

>  
>  	memset(&revs, 0, sizeof(revs));
>  	revs.initial = 1;
> -	for_each_reflog_ent(refname, collect_one_reflog_ent, &revs);
> +	for_each_reflog_ent(full_refname, collect_one_reflog_ent, &revs);
>  
> -	if (!revs.nr && !get_oid(refname, &oid))
> +	if (!revs.nr)
>  		add_one_commit(&oid, &revs);
>  
>  	for (i = 0; i < revs.nr; i++)
> @@ -934,6 +944,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
>  
>  cleanup_return:
>  	free_commit_list(bases);
> +	free(full_refname);
>  	return ret;
>  }
>  
> diff --git a/t/t3431-rebase-fork-point.sh b/t/t3431-rebase-fork-point.sh
> index 78851b9a2a..5b09aecd13 100755
> --- a/t/t3431-rebase-fork-point.sh
> +++ b/t/t3431-rebase-fork-point.sh
> @@ -47,11 +47,31 @@ test_rebase 'G F B A' --keep-base
>  test_rebase 'G F C E D B A' --no-fork-point
>  test_rebase 'G F C D B A' --no-fork-point --onto D
>  test_rebase 'G F C B A' --no-fork-point --keep-base
> +
>  test_rebase 'G F E D B A' --fork-point refs/heads/master
> +test_rebase 'G F E D B A' --fork-point master
> +
>  test_rebase 'G F D B A' --fork-point --onto D refs/heads/master
> +test_rebase 'G F D B A' --fork-point --onto D master
> +
>  test_rebase 'G F B A' --fork-point --keep-base refs/heads/master
> +test_rebase 'G F B A' --fork-point --keep-base master
> +
>  test_rebase 'G F C E D B A' refs/heads/master
> +test_rebase 'G F C E D B A' master
> +
>  test_rebase 'G F C D B A' --onto D refs/heads/master
> +test_rebase 'G F C D B A' --onto D master
> +
>  test_rebase 'G F C B A' --keep-base refs/heads/master
> +test_rebase 'G F C B A' --keep-base master
> +
> +test_expect_success "git rebase --fork-point with ambigous refname" "
> +	git checkout master &&
> +	git checkout -b one &&
> +	git checkout side &&
> +	git tag one &&
> +	test_must_fail git rebase --fork-point --onto D one
> +"