Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
On Tue, Apr 02, 2013 at 08:02:13AM -0700, Junio C Hamano wrote:
> >> + if (remote_head->util) {
> >> + struct merge_remote_desc *desc;
> >> + desc = merge_remote_util(remote_head);
> >> + if (desc && desc->obj && desc->obj->type == OBJ_TAG) {
> >> + strbuf_addf(msg, "%s\t\t%s '%s'\n",
> >> + sha1_to_hex(desc->obj->sha1),
> >> + typename(desc->obj->type),
> >> + remote);
> >> + goto cleanup;
> >> + }
> >> + }
> >> +
> >>strbuf_addf(msg, "%s\t\tcommit '%s'\n",
> >>sha1_to_hex(remote_head->object.sha1), remote);
> >
> > I guess there is no other object type besides OBJ_TAG and OBJ_COMMIT
> > that would yield something we could merge, but it feels weird that you
> > check only for OBJ_TAG here, and otherwise still say "commit". Would the
> > intent be more clear if it just said:
> >
> > if (desc && desc->obj && desc->obj->type != OBJ_COMMIT) {
> > ...
> >
> > ?
>
> I suspect not.
>
> The point of the added code is that it knows we want to special case
> merging a tag object, and it wants to keep any other case behaving
> the same as before.
Ah. I read it as "if we have a random object, we do not want to just say
"commit X", because it is not a commit.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
Jeff King writes:
> On Mon, Apr 01, 2013 at 12:57:17PM -0700, Junio C Hamano wrote:
>
>> The user could have said "git merge $(git rev-parse v1.0.0)"; we
>> shouldn't mark it as "Merge commit '1598fb...'" as the merge
>> name, even though such an invocation might be crazy.
>>
>> We could even read the "tag " header from the tag object and replace
>> the object name the user gave us, but let's not lose the information
>> by doing so, at least not yet.
>>
>> Signed-off-by: Junio C Hamano
>> ---
>> builtin/merge.c | 13 +
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/builtin/merge.c b/builtin/merge.c
>> index 0ec8f0d..990e90c 100644
>> --- a/builtin/merge.c
>> +++ b/builtin/merge.c
>> @@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct
>> strbuf *msg)
>> strbuf_release(&line);
>> goto cleanup;
>> }
>> +
>> +if (remote_head->util) {
>> +struct merge_remote_desc *desc;
>> +desc = merge_remote_util(remote_head);
>> +if (desc && desc->obj && desc->obj->type == OBJ_TAG) {
>> +strbuf_addf(msg, "%s\t\t%s '%s'\n",
>> +sha1_to_hex(desc->obj->sha1),
>> +typename(desc->obj->type),
>> +remote);
>> +goto cleanup;
>> +}
>> +}
>> +
>> strbuf_addf(msg, "%s\t\tcommit '%s'\n",
>> sha1_to_hex(remote_head->object.sha1), remote);
>
> I guess there is no other object type besides OBJ_TAG and OBJ_COMMIT
> that would yield something we could merge, but it feels weird that you
> check only for OBJ_TAG here, and otherwise still say "commit". Would the
> intent be more clear if it just said:
>
> if (desc && desc->obj && desc->obj->type != OBJ_COMMIT) {
> ...
>
> ?
I suspect not.
The point of the added code is that it knows we want to special case
merging a tag object, and it wants to keep any other case behaving
the same as before.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
On Mon, Apr 01, 2013 at 12:57:17PM -0700, Junio C Hamano wrote:
> The user could have said "git merge $(git rev-parse v1.0.0)"; we
> shouldn't mark it as "Merge commit '1598fb...'" as the merge
> name, even though such an invocation might be crazy.
>
> We could even read the "tag " header from the tag object and replace
> the object name the user gave us, but let's not lose the information
> by doing so, at least not yet.
>
> Signed-off-by: Junio C Hamano
> ---
> builtin/merge.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 0ec8f0d..990e90c 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct strbuf
> *msg)
> strbuf_release(&line);
> goto cleanup;
> }
> +
> + if (remote_head->util) {
> + struct merge_remote_desc *desc;
> + desc = merge_remote_util(remote_head);
> + if (desc && desc->obj && desc->obj->type == OBJ_TAG) {
> + strbuf_addf(msg, "%s\t\t%s '%s'\n",
> + sha1_to_hex(desc->obj->sha1),
> + typename(desc->obj->type),
> + remote);
> + goto cleanup;
> + }
> + }
> +
> strbuf_addf(msg, "%s\t\tcommit '%s'\n",
> sha1_to_hex(remote_head->object.sha1), remote);
I guess there is no other object type besides OBJ_TAG and OBJ_COMMIT
that would yield something we could merge, but it feels weird that you
check only for OBJ_TAG here, and otherwise still say "commit". Would the
intent be more clear if it just said:
if (desc && desc->obj && desc->obj->type != OBJ_COMMIT) {
...
?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
Hi, Le lundi 01 avril 2013 à 12:57 -0700, Junio C Hamano a écrit : > The user could have said "git merge $(git rev-parse v1.0.0)"; we > shouldn't mark it as "Merge commit '1598fb...'" as the merge > name, even though such an invocation might be crazy. > > We could even read the "tag " header from the tag object and replace > the object name the user gave us, but let's not lose the information > by doing so, at least not yet. > > Signed-off-by: Junio C Hamano Thanks for the patch. I gave it a try and found the behavior rather good. Merging a tag object by its name or by its object-id are now using the same behavor: it is more consistent. Tested-by: Yann Droneaud PS: there's a typo in the commit title :) Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] merge: a random object may not necssarily be a commit
The user could have said "git merge $(git rev-parse v1.0.0)"; we
shouldn't mark it as "Merge commit '1598fb...'" as the merge
name, even though such an invocation might be crazy.
We could even read the "tag " header from the tag object and replace
the object name the user gave us, but let's not lose the information
by doing so, at least not yet.
Signed-off-by: Junio C Hamano
---
builtin/merge.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index 0ec8f0d..990e90c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct strbuf
*msg)
strbuf_release(&line);
goto cleanup;
}
+
+ if (remote_head->util) {
+ struct merge_remote_desc *desc;
+ desc = merge_remote_util(remote_head);
+ if (desc && desc->obj && desc->obj->type == OBJ_TAG) {
+ strbuf_addf(msg, "%s\t\t%s '%s'\n",
+ sha1_to_hex(desc->obj->sha1),
+ typename(desc->obj->type),
+ remote);
+ goto cleanup;
+ }
+ }
+
strbuf_addf(msg, "%s\t\tcommit '%s'\n",
sha1_to_hex(remote_head->object.sha1), remote);
cleanup:
--
1.8.2-480-g064f421
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
