Re: [PATCH] Allow multiple merges to invalid HEAD

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Timothy Chen wrote:

>  builtin/merge.c |   57 +++++++++++++++++++++++++++++-------------------------
>  1 files changed, 31 insertions(+), 26 deletions(-)

Now for mechanics.

> --- a/builtin/merge.c
> +++ b/builtin/merge.c
[...]
> @@ -1101,36 +1098,44 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
>  		if (!remote_head)
>  			die(_("%s - not something we can merge"), argv[0]);
> -		read_empty(remote_head->sha1, 0);
>  		update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0,
>  				DIE_ON_ERR);
> -		return 0;
> +
> +		if (argc < 2)
> +			return 0;

When argc == 1, this means read_empty never gets called.  Is that
intended?

It breaks 7607.13.  Running "make test" is a good way to find some
breakages.

> +
> +		hashcpy(head, remote_head->sha1);
> +		read_empty(remote_head->sha1, 0);
> +		head_arg = argv[0];
> +		argc--;
> +		argv++;

As always when pretending something, I think a comment would be
helpful.  Something to the effect of:

	/*
	 * We were called as "git merge <branch1> <branch2> <branch3>...".
	 *
	 * Now HEAD has advanced to <branch1>, and we can pretend
	 * we were called as "git merge <branch2> <branch3>...".
	 */

Though I think I prefer the more explicit comment I suggested last
time[1].

> +	}
> +
> +	struct strbuf merge_names = STRBUF_INIT;
> +
> -	} else {
> -		struct strbuf merge_names = STRBUF_INIT;
> -
> -		/* We are invoked directly as the first-class UI. */
> +	/* We are invoked directly as the first-class UI. */

Won't this break

	git merge "message here" $(git rev-parse HEAD) foo bar

?  Previously this code was in an "else" block so it wasn't reached in
the is_old_style_invocation case.

> -	if (head_invalid || !argc)
> +	if (!argc)
>  		usage_with_options(builtin_merge_usage,
>  			builtin_merge_options);

What happens with

	git merge "message here" HEAD foo bar

from an unborn branch?

Hope that helps.
Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/170456