Re: [PATCH] log: properly handle decorations with chained tags
brian m. carlson sand...@crustytoothpaste.net writes: On Tue, Dec 17, 2013 at 04:36:06PM -0800, Junio C Hamano wrote: I think all we need to do, in addition to what the existing code does, is to make sure that we _parse_ the object that the tag points at, to avoid this problem. Something like this, perhaps, instead? Yeah, that's the clean fix I was looking for, but couldn't quite come up with. I'm going to re-roll with your fix instead of mine and my tests. Any objections to adding your sign-off? I was actually planning to just squash 35a34ce281 as a fixup! to your 5e65a201 and call it a day. I think your proposed log message in the latter clearly describes why the fix is correct. -- 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] log: properly handle decorations with chained tags
On Thu, Dec 19, 2013 at 10:44:22AM -0800, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: Yeah, that's the clean fix I was looking for, but couldn't quite come up with. I'm going to re-roll with your fix instead of mine and my tests. Any objections to adding your sign-off? I was actually planning to just squash 35a34ce281 as a fixup! to your 5e65a201 and call it a day. I think your proposed log message in the latter clearly describes why the fix is correct. Okay, sounds good. Thanks. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] log: properly handle decorations with chained tags
On Tue, Dec 17, 2013 at 04:36:06PM -0800, Junio C Hamano wrote: I think all we need to do, in addition to what the existing code does, is to make sure that we _parse_ the object that the tag points at, to avoid this problem. Something like this, perhaps, instead? Yeah, that's the clean fix I was looking for, but couldn't quite come up with. I'm going to re-roll with your fix instead of mine and my tests. Any objections to adding your sign-off? -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] log: properly handle decorations with chained tags
brian m. carlson sand...@crustytoothpaste.net writes:
refname = prettify_refname(refname);
add_name_decoration(type, refname, obj);
while (obj-type == OBJ_TAG) {
- obj = ((struct tag *)obj)-tagged;
- if (!obj)
- break;
+ struct object *tagged = ((struct tag *)obj)-tagged;
+ if (!tagged) {
+ obj = parse_object(obj-sha1);
+ if (!obj)
+ break;
+ tagged = ((struct tag *)obj)-tagged;
+ if (!tagged)
+ break;
+ }
+ obj = tagged;
add_name_decoration(DECORATION_REF_TAG, refname, obj);
}
OK, the above is not wrong per-se but it took me three reads to
convince myself that I understood what was going on.
Before entering this loop, obj has already been parsed, it is known
to be an annotated tag object, and its obj-tagged field is valid,
but the object pointed at by the tag may still not be parsed yet.
The object given to add_name_decoration() before we enter the loop
has been parsed, but the one given at the end of this loop is not.
I think all we need to do, in addition to what the existing code
does, is to make sure that we _parse_ the object that the tag points
at, to avoid this problem. Something like this, perhaps, instead?
diff --git a/log-tree.c b/log-tree.c
index 8534d91..1982631 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -132,10 +132,12 @@ static int add_ref_decoration(const char *refname, const
unsigned char *sha1, in
add_name_decoration(type, refname, obj);
while (obj-type == OBJ_TAG) {
obj = ((struct tag *)obj)-tagged;
if (!obj)
break;
+ if (!obj-parsed)
+ parse_object(obj-sha1);
add_name_decoration(DECORATION_REF_TAG, refname, obj);
}
return 0;
}
It seems to me that the above is not just sufficient, but also shows
what the breakage was really about a lot more clearly, at least to
me.
Hmm?
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index fb00041..2a6278b 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -310,4 +310,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'log decoration properly follows tag chain' '
+ git tag -a tag1 -m tag1
+ git tag -a tag2 -m tag2 tag1
+ git tag -d tag1
+ git commit --amend -m shorter
+ git log --no-walk --tags --pretty=%H %d --decorate=full actual
+ cat EOF expected
+6a908c10688b2503073c39c9ba26322c73902bb5 (tag: refs/tags/tag2)
+9f716384d92283fb915a4eee5073f030638e05f9 (tag: refs/tags/message-one)
+b87e4cccdb77336ea79d89224737be7ea8e95367 (tag: refs/tags/message-two)
+EOF
+ sort actual actual1
+ test_cmp expected actual1
+'
+
test_done
--
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
