Junio C Hamano venit, vidit, dixit 07.09.2011 20:16:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>>
>>> Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes:
>>>
>>>> git merge ":/Merge branch 'jk/generation-numbers' into pu"
>>>> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
>>>> to a commit
>>>> # Huh?
>>>
>>> Interesting.
>>
>> This is because 1c7b76b (Build in merge, 2008-07-07) grabs the name of the
>> commit to be merged using peel_to_type(), which was defined in 8177631
>> (expose a helper function peel_to_type()., 2007-12-24) in terms of
>> get_sha1_1(). It understands $commit~$n, $commit^$n and $ref@{$nth}, but
>> does not understand :/$str, $treeish:$path, and :$stage:$path.
>
> -- >8 --
> Subject: Allow git merge ":/<pattern>"
>
> It probably is not such a good idea to use ":/<pattern>" to specify which
> commit to merge, as ":/<pattern>" can often hit unexpected commits, but
> somebody tried it and got a nonsense error message:
>
> fatal: ':/Foo bar' does not point to a commit
>
> So here is a for-the-sake-of-consistency update that is fairly useless
> that allows users to carefully try not shooting in the foot.
Shooting in the foot can be a good thing, depending on the feet...
My concern is: If a command expects a commit(tish), a user expects to be
able to specify it in any way which git rev-parse can parse. I had no
idea we distinguish even further then what the command itself requires
("branch" needs a branch refname, diff-tree a treeish etc.).
So, for systematic reasons I think the below is an improvement.
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> builtin/merge.c | 19 ++++++++++++++-----
> sha1_name.c | 6 ------
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index ab4077f..ee56974 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -403,6 +403,16 @@ static void finish(const unsigned char *new_head, const char *msg)
> strbuf_release(&reflog_message);
> }
>
> +static struct object *want_commit(const char *name)
> +{
> + struct object *obj;
> + unsigned char sha1[20];
> + if (get_sha1(name, sha1))
> + return NULL;
> + obj = parse_object(sha1);
> + return peel_to_type(name, 0, obj, OBJ_COMMIT);
> +}
> +
> /* Get the name for the merge commit's message. */
> static void merge_name(const char *remote, struct strbuf *msg)
> {
> @@ -418,7 +428,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
> remote = bname.buf;
>
> memset(branch_head, 0, sizeof(branch_head));
> - remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
> + remote_head = want_commit(remote);
> if (!remote_head)
> die(_("'%s' does not point to a commit"), remote);
>
> @@ -1124,7 +1134,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> if (!allow_fast_forward)
> die(_("Non-fast-forward commit does not make sense into "
> "an empty head"));
> - remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
> + remote_head = want_commit(argv[0]);
> if (!remote_head)
> die(_("%s - not something we can merge"), argv[0]);
> read_empty(remote_head->sha1, 0);
> @@ -1170,7 +1180,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> struct object *o;
> struct commit *commit;
>
> - o = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);
> + o = want_commit(argv[i]);
> if (!o)
> die(_("%s - not something we can merge"), argv[i]);
> commit = lookup_commit(o->sha1);
> @@ -1238,8 +1248,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> if (have_message)
> strbuf_addstr(&msg,
> " (no commit created; -m option ignored)");
> - o = peel_to_type(sha1_to_hex(remoteheads->item->object.sha1),
> - 0, NULL, OBJ_COMMIT);
> + o = want_commit(sha1_to_hex(remoteheads->item->object.sha1));
> if (!o)
> return 1;
>
> diff --git a/sha1_name.c b/sha1_name.c
> index ff5992a..653b065 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -501,12 +501,6 @@ struct object *peel_to_type(const char *name, int namelen,
> {
> if (name && !namelen)
> namelen = strlen(name);
> - if (!o) {
> - unsigned char sha1[20];
> - if (get_sha1_1(name, namelen, sha1))
> - return NULL;
> - o = parse_object(sha1);
> - }
> while (1) {
> if (!o || (!o->parsed && !parse_object(o->sha1)))
> return NULL;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html