Re: [PATCH v3 3/4] name-rev: provide debug output

2 messages, 2 authors, 2017-03-31 · open the first message on its own page

Re: [PATCH v3 3/4] name-rev: provide debug output

From: Junio C Hamano <hidden>
Date: 2017-03-31 16:52:25

Michael J Gruber [off-list ref] writes:
Currently, `git describe --contains --debug` does not create any debug
output because it does not pass the flag down to `git name-rev`, which
does not know that flag.

Teach the latter that flag, so that the former can pass it down (in
the following commit).

The output is patterned after that of `git describe --debug`, with the
following differences:

describe loops over all args to describe, then over all possible
descriptions; name-rev does it the other way round. Therefore, we need
to amend each possible description by the arg that it is for (and we
leave out the "searching to describe" header).

The date cut-off for name-rev kicks in way more often than the candidate
number cut-off of describe, so we do not clutter the output with the
cut-off.

Signed-off-by: Michael J Gruber <redacted>
---
quoted hunk
 static void name_rev(struct commit *commit,
 		const char *tip_name, unsigned long taggerdate,
 		int generation, int distance, int from_tag,
-		int deref)
+		int deref, struct name_ref_data *data)
 {
 	struct rev_name *name = (struct rev_name *)commit->util;
 	struct commit_list *parents;
@@ -75,6 +88,7 @@ static void name_rev(struct commit *commit,
 
 	if (deref) {
 		tip_name = xstrfmt("%s^0", tip_name);
+		from_tag += 1;
Why this change?  I didn't see it explained in the proposed log
message.  "deref" is true only when our immediate caller is the one
that inspected the object at the tip and found it to be a tag object
(i.e. not a lightweight tag or a branch).  from_tag is about "is the
tip within refs/tags/ hierarchy?  Yes/No?" and such a caller will
set it appropriately when calling us.  This function just passes it
down when it recursively calls itself.  

We shouldn't be mucking with that value ourselves here, should we?

The only case that this change may make a difference I can think of
is when you have a tag object pointed at from outside refs/tags
(e.g. refs/heads/foo is a tag object); if you are trying to change
the definition of "from_tag" from the current "Is the tip inside
refs/tags/?" to "Is the tip either inside refs/tags/ or is it a tag
object anywhere?", that may be a good change (I didn't think things
through, though), but that shouldn't be hidden inside a commit that
claims to only add support for debugging.

What problem are you solving?  
quoted hunk
@@ -236,7 +273,6 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 	}
 
 	add_to_tip_table(oid->hash, path, can_abbreviate_output);
-
 	while (o && o->type == OBJ_TAG) {
 		struct tag *t = (struct tag *) o;
 		if (!t->tagged)
This is a patch noise we can do without.

Thanks.

Re: [PATCH v3 3/4] name-rev: provide debug output

From: Michael J Gruber <hidden>
Date: 2017-03-31 18:03:20

Am 31. März 2017 18:52:16 MESZ schrieb Junio C Hamano [off-list ref]:
Michael J Gruber [off-list ref] writes:
quoted
Currently, `git describe --contains --debug` does not create any
debug
quoted
output because it does not pass the flag down to `git name-rev`,
which
quoted
does not know that flag.

Teach the latter that flag, so that the former can pass it down (in
the following commit).

The output is patterned after that of `git describe --debug`, with
the
quoted
following differences:

describe loops over all args to describe, then over all possible
descriptions; name-rev does it the other way round. Therefore, we
need
quoted
to amend each possible description by the arg that it is for (and we
leave out the "searching to describe" header).

The date cut-off for name-rev kicks in way more often than the
candidate
quoted
number cut-off of describe, so we do not clutter the output with the
cut-off.

Signed-off-by: Michael J Gruber <redacted>
---
quoted
 static void name_rev(struct commit *commit,
 		const char *tip_name, unsigned long taggerdate,
 		int generation, int distance, int from_tag,
-		int deref)
+		int deref, struct name_ref_data *data)
 {
 	struct rev_name *name = (struct rev_name *)commit->util;
 	struct commit_list *parents;
@@ -75,6 +88,7 @@ static void name_rev(struct commit *commit,
 
 	if (deref) {
 		tip_name = xstrfmt("%s^0", tip_name);
+		from_tag += 1;
Why this change?  I didn't see it explained in the proposed log
message.  "deref" is true only when our immediate caller is the one
that inspected the object at the tip and found it to be a tag object
(i.e. not a lightweight tag or a branch).  from_tag is about "is the
tip within refs/tags/ hierarchy?  Yes/No?" and such a caller will
set it appropriately when calling us.  This function just passes it
down when it recursively calls itself.  

We shouldn't be mucking with that value ourselves here, should we?

The only case that this change may make a difference I can think of
is when you have a tag object pointed at from outside refs/tags
(e.g. refs/heads/foo is a tag object); if you are trying to change
the definition of "from_tag" from the current "Is the tip inside
refs/tags/?" to "Is the tip either inside refs/tags/ or is it a tag
object anywhere?", that may be a good change (I didn't think things
through, though), but that shouldn't be hidden inside a commit that
claims to only add support for debugging.

What problem are you solving?  
Sorry, I forgot about that change and failed to mention it.

It makes no difference in the non-debug case which cares about the Boolean only. In the debug case, I want to distinguish between annotated and lightweight tags, just like describe --debug does. By adding 1 via deref and passing this down, I know that an annotated tag gets the value 2, a lightweight tag 1 and everything else 0, just like describe --tags.
quoted
@@ -236,7 +273,6 @@ static int name_ref(const char *path, const
struct object_id *oid, int flags, vo
quoted
 	}
 
 	add_to_tip_table(oid->hash, path, can_abbreviate_output);
-
 	while (o && o->type == OBJ_TAG) {
 		struct tag *t = (struct tag *) o;
 		if (!t->tagged)
This is a patch noise we can do without.

Thanks.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help