{"thread":{"id":"45385","subject":"[PATCH 0/3] describe --contains sanity","startedAt":"2017-03-15T13:15:23Z","lastAt":"2017-05-20T05:19:17Z","messageCount":46,"participants":["Michael J Gruber","Junio C Hamano","Stefan Beller","Jacob Keller","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"314130","messageId":"cover.1489581340.git.git@drmicha.warpmail.net","threadId":"45385","inReplyTo":null,"subject":"[PATCH 0/3] describe --contains sanity","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2017-03-15T13:15:07Z","receivedAt":"2017-03-15T13:15:23Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"2 patches and 1 RFD around describe (--contains). They are technically\nindependent, but happened along the same stroll in that area\nwhen I tried to match documentation, my expectations, and reality.\n\n1 and 2 should be no-brainers.\n\n3 is something to ponder for a while.\n\nMichael J Gruber (3):\n describe: debug is incompatible with contains\n git-prompt: add a describe style for any tags\n name-rev: Allow lightweight tags and branch refs\n\n Documentation/git-describe.txt | 2 +-\n builtin/describe.c | 2 ++\n builtin/name-rev.c | 2 ++\n contrib/completion/git-prompt.sh | 3 +++\n t/t9903-bash-prompt.sh | 2 +-\n 5 files changed, 9 insertions(+), 2 deletions(-)\n\n-- \n2.12.0.384.g157040b11f.dirty\n\n"},{"id":"314131","messageId":"db54f291407ef34080fe9e8c9dbdd482b4b3698d.1489581340.git.git@drmicha.warpmail.net","threadId":"45385","inReplyTo":"cover.1489581340.git.git@drmicha.warpmail.net","subject":"[RFD PATCH 3/3] name-rev: Allow lightweight tags and branch refs","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2017-03-15T13:15:10Z","receivedAt":"2017-03-15T13:15:25Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When name-rev constructs possible names for a rev, it assigns the\ntaggerdate to an annotated tag and ULONG_MAX to other names such as\nlightweight tags and branch names. Practically, this rules out even\nnaming a tagged commit by the tag if that is lightweight and there is\nanother possible indirect name (e.g. foo~5) coming from an annotated\ntag.\n\nInstead, assign the commit date to lightweight tags or branch refs so\nthat they get their fair chance of being picked up.\n\nSigned-off-by: Michael J Gruber \n---\n\nOriginally, I didn't even think of submitting this as is until I noticed that\nall tests succeed with it (bar one for git-prompt.h which looked strange\nanyways); rather, I thought about another switch \"--lightweight\" or so.\n\nI still submit it as RFD. This origin of my foray into name-rev came from\ngit describe, where a fair expectation should be that \"--contains\" mode\nis the same as the odinary mode, albeit with a different direction in\ntime/traversal. (Technically, describe --contains is name-rev.)\n\nConsider the following:\n\ngit init\necho past >tense\ngit add tense\ngit commit -m past\ngit tag -m past -a past\necho present >tense\ngit commit -am present\ngit tag present\necho future >tense\ngit commit -am future\n\n\"git describe past present future\" gives\n\npast\npast-1-g5ad942f\nfuture\n\nbecause (as documented) it does not consider lightweight tags, and thus\nhas to describe present from the past.\n\n\"git describe --tags past present future\" gives\n\npast\npresent\nfuture\n\nbecause (as documented) it does consider lightweight tags.\n\n\"git describe --contains past present future\" gives\n\npast^0\nfuture~1\nfuture^0\n\nand I have a hard time matching that with the documentation\n(describe doc claims that --tags is automatic; name-rev doc does not\ndistinguish between tag types).\n\"--tags\" does not make any difference here, nor does \"--all\"\n(besides fully qualifying the tags).\n\n\"git describe --contains past present future\" gives\n\npast^0\npresent\nfuture^0\n\nwith this patch (I'm tempted to say: with the present patch),\nwhich is what I would expect.\n\nI'm wondering whether I'm overlooking any side-effects that our test\nsuite doesn't cover, though. In any case, we may want to have\nlightweight tags allowed based on an extra flag (like the\nexisting --tags for describe, which means something else for name-rev).\n\n builtin/name-rev.c | 2 ++\n t/t9903-bash-prompt.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 8bdc3eaa6f..75ba7bbad5 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -207,6 +207,8 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n \n+\t\tif (taggerdate == ULONG_MAX) /* lightweight tag */\n+\t\t\ttaggerdate = commit->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n \t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);\n \t}\ndiff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh\nindex 97c9b32c2e..d467d5957d 100755\n--- a/t/t9903-bash-prompt.sh\n+++ b/t/t9903-bash-prompt.sh\n@@ -107,7 +107,7 @@ test_expect_success 'prompt - describe detached head - contains' '\n '\n \n test_expect_success 'prompt - describe detached head - branch' '\n-\tprintf \" ((tags/t2~1))\" >expected &&\n+\tprintf \" ((b1~1))\" >expected &&\n \tgit checkout b1^ &&\n \ttest_when_finished \"git checkout master\" &&\n \t(\n-- \n2.12.0.384.g157040b11f.dirty\n\n"},{"id":"314132","messageId":"d8477234d20fe3b4db43b23cc192c18af1ecdaff.1489581340.git.git@drmicha.warpmail.net","threadId":"45385","inReplyTo":"cover.1489581340.git.git@drmicha.warpmail.net","subject":"[PATCH 1/3] describe: debug is incompatible with contains","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2017-03-15T13:15:08Z","receivedAt":"2017-03-15T13:15:26Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"`git describe --contains` calls into `git name-rev` which does not have\nany searching to do and thus does not display any debug information.\n\nSay so in the documentation and catch the incompatible arguments.\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/git-describe.txt | 2 +-\n builtin/describe.c | 2 ++\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex 8755f3af7b..0f9adb6e9a 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -69,7 +69,7 @@ OPTIONS\n --debug::\n \tVerbosely display information about the searching strategy\n \tbeing employed to standard error. The tag name will still\n-\tbe printed to standard out.\n+\tbe printed to standard out. This is incompatible with --contains.\n \n --long::\n \tAlways output the long format (the tag, the number of commits\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 76c18059bf..01a6d159a0 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -462,6 +462,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \n \tif (longformat && abbrev == 0)\n \t\tdie(_(\"--long is incompatible with --abbrev=0\"));\n+\tif (contains && debug)\n+\t\tdie(_(\"--debug is incompatible with --contains\"));\n \n \tif (contains) {\n \t\tstruct string_list_item *item;\n-- \n2.12.0.384.g157040b11f.dirty\n\n"},{"id":"314133","messageId":"8d8fcdb4134090328662bf6fa5dfdba6c73fb6a5.1489581340.git.git@drmicha.warpmail.net","threadId":"45385","inReplyTo":"cover.1489581340.git.git@drmicha.warpmail.net","subject":"[PATCH 2/3] git-prompt: add a describe style for any tags","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2017-03-15T13:15:09Z","receivedAt":"2017-03-15T13:15:28Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"git-prompt has various describe styles, among them \"describe\" (by\nannotated tags) and \"default\" (by exact match with any tag).\n\nAdd a mode \"tag\" that describes by any tag, annotated or not.\n\nSigned-off-by: Michael J Gruber \n---\n contrib/completion/git-prompt.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex 97eacd7832..c6cbef38c2 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -82,6 +82,7 @@\n # contains relative to newer annotated tag (v1.6.3.2~35)\n # branch relative to newer tag or branch (master~4)\n # describe relative to older annotated tag (v1.6.3.1-13-gdd42c2f)\n+# tag relative to any older tag (v1.6.3.1-13-gdd42c2f)\n # default exactly matching tag\n #\n # If you would like a colored hint about the current dirty state, set\n@@ -443,6 +444,8 @@ __git_ps1 ()\n \t\t\t\t\tgit describe --contains HEAD ;;\n \t\t\t\t(branch)\n \t\t\t\t\tgit describe --contains --all HEAD ;;\n+\t\t\t\t(tag)\n+\t\t\t\t\tgit describe --tags HEAD ;;\n \t\t\t\t(describe)\n \t\t\t\t\tgit describe HEAD ;;\n \t\t\t\t(* | default)\n-- \n2.12.0.384.g157040b11f.dirty\n\n"},{"id":"314142","messageId":"xmqqmvcmfrf8.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"db54f291407ef34080fe9e8c9dbdd482b4b3698d.1489581340.git.git@drmicha.warpmail.net","subject":"Re: [RFD PATCH 3/3] name-rev: Allow lightweight tags and branch refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T16:14:35Z","receivedAt":"2017-03-15T16:15:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Consider the following:\n> ...\n> \"git describe past present future\" gives\n>\n> past\n> past-1-g5ad942f\n> future\n>\n> because (as documented) it does not consider lightweight tags, and thus\n> has to describe present from the past.\n>\n> \"git describe --tags past present future\" gives\n>\n> past\n> present\n> future\n> \n> because (as documented) it does consider lightweight tags.\n>\n> \"git describe --contains past present future\" gives\n>\n> past^0\n> future~1\n> future^0\n\nNice illustration to convince anybody that \"git describe --tags\n--contains\" should give \"past present future\" (or \"past^0 present\nfuture^0\"). I am not sure if it is a good idea in general to change\nthe output to include \"present\" when \"--tags\" is not given, though.\n\nFor _this_ example, it does give us a more useful result, but the\nexample was to constructed to illustrate the case where the new\nbehaviour is more useful, and does not serve to convince us that it\nwon't have negative impact X-<.\n"},{"id":"314159","messageId":"xmqqzigme474.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"d8477234d20fe3b4db43b23cc192c18af1ecdaff.1489581340.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 1/3] describe: debug is incompatible with contains","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T19:21:35Z","receivedAt":"2017-03-15T19:21:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> `git describe --contains` calls into `git name-rev` which does not have\n> any searching to do and thus does not display any debug information.\n>\n> Say so in the documentation and catch the incompatible arguments.\n\nI am not sure if this is worth it. Those who are really doing the\ndebugging would be staring at the code while running it anyway, so\nit is not like this new error condition would help anybody from\nwasting time scratching her head before viewing the source and\nrealize that the underlying name-rev does not honor the option.\n\nIf \"--debug\" is truly valuable, \"name-rev\" can gain \"--debug\" later\nand then we can pass it down if we want.\n\nAlso, it is not like \"--debug\" is incompatible. It is just the\n\"--contains\" codepath is overly silent and does not give any useful\ninformation when run in the debug mode. \"incompatible\" is more like\n\"would not work correctly when both are given\", which is not the\ncase here.\n\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> Documentation/git-describe.txt | 2 +-\n> builtin/describe.c | 2 ++\n> 2 files changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\n> index 8755f3af7b..0f9adb6e9a 100644\n> --- a/Documentation/git-describe.txt\n> +++ b/Documentation/git-describe.txt\n> @@ -69,7 +69,7 @@ OPTIONS\n> --debug::\n> \tVerbosely display information about the searching strategy\n> \tbeing employed to standard error. The tag name will still\n> -\tbe printed to standard out.\n> +\tbe printed to standard out. This is incompatible with --contains.\n> \n> --long::\n> \tAlways output the long format (the tag, the number of commits\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> index 76c18059bf..01a6d159a0 100644\n> --- a/builtin/describe.c\n> +++ b/builtin/describe.c\n> @@ -462,6 +462,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n> \n> \tif (longformat && abbrev == 0)\n> \t\tdie(_(\"--long is incompatible with --abbrev=0\"));\n> +\tif (contains && debug)\n> +\t\tdie(_(\"--debug is incompatible with --contains\"));\n> \n> \tif (contains) {\n> \t\tstruct string_list_item *item;\n"},{"id":"314160","messageId":"xmqqvarae3zy.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"8d8fcdb4134090328662bf6fa5dfdba6c73fb6a5.1489581340.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/3] git-prompt: add a describe style for any tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T19:25:53Z","receivedAt":"2017-03-15T19:26:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> git-prompt has various describe styles, among them \"describe\" (by\n> annotated tags) and \"default\" (by exact match with any tag).\n>\n> Add a mode \"tag\" that describes by any tag, annotated or not.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> contrib/completion/git-prompt.sh | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\n> index 97eacd7832..c6cbef38c2 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -82,6 +82,7 @@\n> # contains relative to newer annotated tag (v1.6.3.2~35)\n> # branch relative to newer tag or branch (master~4)\n> # describe relative to older annotated tag (v1.6.3.1-13-gdd42c2f)\n> +# tag relative to any older tag (v1.6.3.1-13-gdd42c2f)\n\nI guess this feature makes sense. \n\nI just wish we had a well-known unannotated tag we can use for such\nan example; using v1.6.3.1 that is annotated does not help to make\nthe distinctin between describe and tag stand out. We want to\nconvey \"both annotated one and unannotated one can be used\".\n\nI am wondering if it makes sense to do something like this instead:\n\n#\ttag \tsimilar to 'describe' but also allow unannotated tags\n\n> # default exactly matching tag\n> #\n> # If you would like a colored hint about the current dirty state, set\n> @@ -443,6 +444,8 @@ __git_ps1 ()\n> \t\t\t\t\tgit describe --contains HEAD ;;\n> \t\t\t\t(branch)\n> \t\t\t\t\tgit describe --contains --all HEAD ;;\n> +\t\t\t\t(tag)\n> +\t\t\t\t\tgit describe --tags HEAD ;;\n> \t\t\t\t(describe)\n> \t\t\t\t\tgit describe HEAD ;;\n> \t\t\t\t(* | default)\n"},{"id":"314162","messageId":"xmqqlgs6e2uv.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"db54f291407ef34080fe9e8c9dbdd482b4b3698d.1489581340.git.git@drmicha.warpmail.net","subject":"Re: [RFD PATCH 3/3] name-rev: Allow lightweight tags and branch refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T19:50:32Z","receivedAt":"2017-03-15T19:50:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> I'm wondering whether I'm overlooking any side-effects that our test\n> suite doesn't cover, though. In any case, we may want to have\n> lightweight tags allowed based on an extra flag (like the\n> existing --tags for describe, which means something else for name-rev).\n>\n> builtin/name-rev.c | 2 ++\n> t/t9903-bash-prompt.sh | 2 +-\n> 2 files changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n> index 8bdc3eaa6f..75ba7bbad5 100644\n> --- a/builtin/name-rev.c\n> +++ b/builtin/name-rev.c\n> @@ -207,6 +207,8 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n> \tif (o && o->type == OBJ_COMMIT) {\n> \t\tstruct commit *commit = (struct commit *)o;\n> \n> +\t\tif (taggerdate == ULONG_MAX) /* lightweight tag */\n> +\t\t\ttaggerdate = commit->date;\n> \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n> \t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);\n> \t}\n\nI think this comment indicates where this change will bite us ;-) \n\nThis could have been an invocation of \"name-rev\" without \"--tags\",\nwhere _any_ tip of ref can be used to name a revision, and in such a\ncase, retaining commit->date may still be valuable, but it is\nprobably wrong to use it for nothing more than tie-breaking.\n\nIn an extreme case, imagine that your repository does not have any\ntag and you have a commit that can be reached from both 'maint' and\n'master' branches. Because the current code assigns the same\nuseless taggerdate for these tips of refs, the commit is described\nsolely based on the distance from the tip of 'maint' or 'master'\nwith today's code.\n\nIf you assign commit date to taggerdate variable here, that date is\nused to trump the distance when deciding whether to name the commit\nbased on 'maint' and 'master', and I doubt it is what you want.\nPerhaps I regularly advance 'master', but still add a few selected\nfixes to 'maint' every few days---when these two branches both are\nactive like that, do you want the same commit that is topologically\nmuch closer to 'maint' than 'master' to be described in terms of\n'maint' on some days (i.e. those days I added fixes to it) and\n'master' on some other days?\n\nI think the expectation by end-users who do not use \"--tags\" is that\nthey still prefer a commit to be named in terms of the oldest tag\nthat is fewer number of hops away, and the naming to fall back to\nnon-tag refs only when there is no tags that reach the commit. And\nbetween two or more non-tag tips that are _known_ to be still active\n(iow, branch tips), they want \"fewer number of hops away\" to be the\nprimary deciding factor to choose among them. The ULONG_MAX setting\nthe code currently uses is very suitable for that purpose. It\npenalizes the non-tag refs, and it disables the use of timestamp for\ndeciding among these tips and uses only the topological distance.\n\nI think you can do this change _ONLY_ when we are operating under\nthe \"--tags\" option. That would place unannotated tags at a better\nlocation in the timestamp order than the current code does, without\nintroducing issues with refs that are actively moving.\n"},{"id":"314171","messageId":"xmqqd1die00j.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"xmqqlgs6e2uv.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFD PATCH 3/3] name-rev: Allow lightweight tags and branch refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T20:51:56Z","receivedAt":"2017-03-15T20:52:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This could have been an invocation of \"name-rev\" without \"--tags\",\n> where _any_ tip of ref can be used to name a revision, and in such a\n> case, retaining commit->date may still be valuable, but it is\n> probably wrong to use it for nothing more than tie-breaking.\n> ...\n> I think you can do this change _ONLY_ when we are operating under\n> the \"--tags\" option. That would place unannotated tags at a better\n> location in the timestamp order than the current code does, without\n> introducing issues with refs that are actively moving.\n\nI'll leave \"only do so under --tags\" as an exercise to the reader,\nbut if we want to use the timestamp for tie-breaking between names\nbased on two branches, a patch to do so may look like this one. \n\nI am presenting it as a single ball of wax, but in the real history\nI have (which I am not publishing, as I haven't decided if this is a\ngood direction to go in the first place), this is a two-patch\nseries, the first one that factors out is_better_name() without\ndoing anything else, followed by a commit that adds \"from_tag\" and\npasses it throughout the callpath to allow is_better_name() to use\nit to:\n\n - always favor names based on a tag;\n\n - between two names based on tags, favor name based on an older tag\n and tiebreak with distance;\n\n - between two names based on non-tags, favor name with shorter hops\n and tiebreak with age.\n\n builtin/name-rev.c | 43 ++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 36 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex cd89d48b65..ba30c9ca23 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -13,6 +13,7 @@ typedef struct rev_name {\n \tunsigned long taggerdate;\n \tint generation;\n \tint distance;\n+\tint from_tag;\n } rev_name;\n \n static long cutoff = LONG_MAX;\n@@ -20,9 +21,31 @@ static long cutoff = LONG_MAX;\n /* How many generations are maximally preferred over _one_ merge traversal? */\n #define MERGE_TRAVERSAL_WEIGHT 65535\n \n+static int is_better_name(struct rev_name *name,\n+\t\t\t const char *tip_name,\n+\t\t\t unsigned long taggerdate,\n+\t\t\t int generation,\n+\t\t\t int distance,\n+\t\t\t int from_tag)\n+{\n+\tif (name->from_tag > from_tag)\n+\t\treturn 0;\n+\telse if (name->from_tag < from_tag)\n+\t\treturn 1;\n+\n+\tif (from_tag)\n+\t\treturn (name->taggerdate > taggerdate ||\n+\t\t\t(name->taggerdate == taggerdate &&\n+\t\t\t name->distance > distance));\n+\telse\n+\t\treturn (name->distance > distance ||\n+\t\t\t(name->distance == distance &&\n+\t\t\t name->taggerdate > taggerdate));\n+}\n+\n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n-\t\tint generation, int distance,\n+\t\tint generation, int distance, int from_tag,\n \t\tint deref)\n {\n \tstruct rev_name *name = (struct rev_name *)commit->util;\n@@ -45,14 +68,14 @@ static void name_rev(struct commit *commit,\n \t\tname = xmalloc(sizeof(rev_name));\n \t\tcommit->util = name;\n \t\tgoto copy_data;\n-\t} else if (name->taggerdate > taggerdate ||\n-\t\t\t(name->taggerdate == taggerdate &&\n-\t\t\t name->distance > distance)) {\n+\t} else if (is_better_name(name, tip_name, taggerdate,\n+\t\t\t\t generation, distance, from_tag)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n \t\tname->generation = generation;\n \t\tname->distance = distance;\n+\t\tname->from_tag = from_tag;\n \t} else\n \t\treturn;\n \n@@ -72,10 +95,12 @@ static void name_rev(struct commit *commit,\n \t\t\t\t\t\t parent_number);\n \n \t\t\tname_rev(parents->item, new_name, taggerdate, 0,\n-\t\t\t\tdistance + MERGE_TRAVERSAL_WEIGHT, 0);\n+\t\t\t\t distance + MERGE_TRAVERSAL_WEIGHT,\n+\t\t\t\t from_tag, 0);\n \t\t} else {\n \t\t\tname_rev(parents->item, tip_name, taggerdate,\n-\t\t\t\tgeneration + 1, distance + 1, 0);\n+\t\t\t\t generation + 1, distance + 1,\n+\t\t\t\t from_tag, 0);\n \t\t}\n \t}\n }\n@@ -174,9 +199,13 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t}\n \tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n+\t\tint from_tag = starts_with(path, \"refs/tags/\");\n \n+\t\tif (taggerdate == ULONG_MAX)\n+\t\t\ttaggerdate = ((struct commit *)o)->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n-\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);\n+\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0,\n+\t\t\t from_tag, deref);\n \t}\n \treturn 0;\n }\n"},{"id":"314195","messageId":"20170315225045.15788-1-gitster@pobox.com","threadId":"45385","inReplyTo":"xmqqd1die00j.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 0/2] Teach name-rev to pay more attention to lightweight tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T22:50:43Z","receivedAt":"2017-03-15T22:59:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is a reroll of what I did in\n\n http://public-inbox.org/git/xmqqd1die00j.fsf@gitster.mtv.corp.google.com/\n\nJunio C Hamano (2):\n name-rev: refactor logic to see if a new candidate is a better name\n name-rev: favor describing with tags and use committer date to tiebreak\n\n builtin/name-rev.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++------\n t/t4202-log.sh | 2 +-\n 2 files changed, 57 insertions(+), 8 deletions(-)\n\n-- \n2.12.0-306-g4a9b9b32d4\n\n"},{"id":"314196","messageId":"20170315225045.15788-3-gitster@pobox.com","threadId":"45385","inReplyTo":"20170315225045.15788-1-gitster@pobox.com","subject":"[PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T22:50:45Z","receivedAt":"2017-03-15T22:59:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git name-rev\" assigned a phony \"far in the future\" date to tips of\nrefs that are not pointing at tag objects, and favored names based\non a ref with the oldest date. This made it almost impossible for\nan unannotated tags and branches to be counted as a viable base,\nwhich was especially problematic when the command is run with the\n\"--tags\" option. If an unannotated tag that points at an ancient\ncommit and an annotated tag that points at a much newer commit\nreaches the commit that is being named, the old unannotated tag was\nignored.\n\nUpdate the \"taggerdate\" field of the rev-name structure, which is\ninitialized from the tip of ref, to have the committer date if the\nobject at the tip of ref is a commit, not a tag, so that we can\noptionally take it into account when doing \"is this name better?\"\ncomparison logic.\n\nWhen \"name-rev\" is run without the \"--tags\" option, the general\nexpectation is still to name the commit based on a tag if possible,\nbut use non-tag refs as fallback, and tiebreak among these non-tag\nrefs by favoring names with shorter hops from the tip. The use of a\nphony \"far in the future\" date in the original code was an effective\nway to ensure this expectation is held: a non-tag tip gets the same\n\"far in the future\" timestamp, giving precedence to tags, and among\nnon-tag tips, names with shorter hops are preferred over longer\nhops, without taking the \"taggerdate\" into account. As we are\ntaking over the \"taggerdate\" field to store the committer date for\ntips with commits:\n\n (1) keep the original logic when comparing names based on two refs\n both of which are from refs/tags/;\n\n (2) favoring a name based on a ref in refs/tags/ hierarchy over\n a ref outside the hierarchy;\n\n (3) between two names based on a ref both outside refs/tags/, give\n precedence to a name with shorter hops and use \"taggerdate\"\n only to tie-break.\n\nA change to t4202 is a natural consequence. The test creates a\ncommit on a branch \"side\" and points at it with an unannotated tag\n\"refs/tags/side-2\". The original code couldn't decide which one to\nfavor at all, and gave a name based on a branch (simply because\nrefs/heads/side sorts earlier than refs/tags/side-2). Because the\nupdated logic is taught to favor refs in refs/tags/ hierarchy, the\nthe test is updated to expect to see tags/side-2 instead.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I am sure others can add better heurisitics in is_better_name()\n for comparing names based on non-tag refs, and I do not mind\n people disagreeing with the logic that this patch happens to\n implement. This is done primarily to illustrate the value of\n using a separate helper function is_better_name() instead of\n open-coding the selection logic in name_rev() function.\n---\n builtin/name-rev.c | 57 +++++++++++++++++++++++++++++++++++++++++++++---------\n t/t4202-log.sh | 2 +-\n 2 files changed, 49 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex f64c71d9bc..eac0180c62 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -13,6 +13,7 @@ typedef struct rev_name {\n \tunsigned long taggerdate;\n \tint generation;\n \tint distance;\n+\tint from_tag;\n } rev_name;\n \n static long cutoff = LONG_MAX;\n@@ -24,16 +25,47 @@ static int is_better_name(struct rev_name *name,\n \t\t\t const char *tip_name,\n \t\t\t unsigned long taggerdate,\n \t\t\t int generation,\n-\t\t\t int distance)\n+\t\t\t int distance,\n+\t\t\t int from_tag)\n {\n-\treturn (name->taggerdate > taggerdate ||\n-\t\t(name->taggerdate == taggerdate &&\n-\t\t name->distance > distance));\n+\t/*\n+\t * When comparing names based on tags, prefer names\n+\t * based on the older tag, even if it is farther away.\n+\t */\n+\tif (from_tag && name->from_tag)\n+\t\treturn (name->taggerdate > taggerdate ||\n+\t\t\t(name->taggerdate == taggerdate &&\n+\t\t\t name->distance > distance));\n+\n+#define COMPARE(attribute, smaller_is_better)\t \\\n+\tif (name->attribute > attribute) \\\n+\t\treturn smaller_is_better; \\\n+\tif (name->attribute < attribute) \\\n+\t\treturn !smaller_is_better\n+\n+\t/*\n+\t * We know that at least one of them is a non-tag at this point.\n+\t * favor a tag over a non-tag.\n+\t */\n+\tCOMPARE(from_tag, 0);\n+\n+\t/*\n+\t * We are now looking at two non-tags. Tiebreak to favor\n+\t * shorter hops.\n+\t */\n+\tCOMPARE(distance, 1);\n+\n+\t/* ... or tiebreak to favor older date */\n+\tCOMPARE(taggerdate, 1);\n+\n+\t/* keep the current one if we cannot decide */\n+\treturn 0;\n+#undef COMPARE\n }\n \n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n-\t\tint generation, int distance,\n+\t\tint generation, int distance, int from_tag,\n \t\tint deref)\n {\n \tstruct rev_name *name = (struct rev_name *)commit->util;\n@@ -57,12 +89,13 @@ static void name_rev(struct commit *commit,\n \t\tcommit->util = name;\n \t\tgoto copy_data;\n \t} else if (is_better_name(name, tip_name, taggerdate,\n-\t\t\t\t generation, distance)) {\n+\t\t\t\t generation, distance, from_tag)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n \t\tname->generation = generation;\n \t\tname->distance = distance;\n+\t\tname->from_tag = from_tag;\n \t} else\n \t\treturn;\n \n@@ -82,10 +115,12 @@ static void name_rev(struct commit *commit,\n \t\t\t\t\t\t parent_number);\n \n \t\t\tname_rev(parents->item, new_name, taggerdate, 0,\n-\t\t\t\tdistance + MERGE_TRAVERSAL_WEIGHT, 0);\n+\t\t\t\t distance + MERGE_TRAVERSAL_WEIGHT,\n+\t\t\t\t from_tag, 0);\n \t\t} else {\n \t\t\tname_rev(parents->item, tip_name, taggerdate,\n-\t\t\t\tgeneration + 1, distance + 1, 0);\n+\t\t\t\t generation + 1, distance + 1,\n+\t\t\t\t from_tag, 0);\n \t\t}\n \t}\n }\n@@ -184,9 +219,13 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t}\n \tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n+\t\tint from_tag = starts_with(path, \"refs/tags/\");\n \n+\t\tif (taggerdate == ULONG_MAX)\n+\t\t\ttaggerdate = ((struct commit *)o)->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n-\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);\n+\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0,\n+\t\t\t from_tag, deref);\n \t}\n \treturn 0;\n }\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 48b55bfd27..038911f230 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -398,7 +398,7 @@ cat > expect <<\\EOF\n | |\n | | Merge branch 'side'\n | |\n-| * commit side\n+| * commit tags/side-2\n | | Author: A U Thor \n | |\n | | side-2\n-- \n2.12.0-306-g4a9b9b32d4\n\n"},{"id":"314197","messageId":"20170315225045.15788-2-gitster@pobox.com","threadId":"45385","inReplyTo":"20170315225045.15788-1-gitster@pobox.com","subject":"[PATCH 1/2] name-rev: refactor logic to see if a new candidate is a better name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-15T22:50:44Z","receivedAt":"2017-03-15T22:59:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When we encounter a new ref that could describe the commit we are\nlooking at, we compare the name that is formed using that ref and\nthe name we found so far and pick a better one.\n\nFactor the comparison logic out to a separate helper function, while\nkeeping the current logic the same (i.e. a name that is based on an\nolder tag is better, and if two tags of the same age can reach the\ncommit, the one with fewer number of hops to reach the commit is\nbetter).\n\nSigned-off-by: Junio C Hamano \n---\n builtin/name-rev.c | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex cd89d48b65..f64c71d9bc 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -20,6 +20,17 @@ static long cutoff = LONG_MAX;\n /* How many generations are maximally preferred over _one_ merge traversal? */\n #define MERGE_TRAVERSAL_WEIGHT 65535\n \n+static int is_better_name(struct rev_name *name,\n+\t\t\t const char *tip_name,\n+\t\t\t unsigned long taggerdate,\n+\t\t\t int generation,\n+\t\t\t int distance)\n+{\n+\treturn (name->taggerdate > taggerdate ||\n+\t\t(name->taggerdate == taggerdate &&\n+\t\t name->distance > distance));\n+}\n+\n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n \t\tint generation, int distance,\n@@ -45,9 +56,8 @@ static void name_rev(struct commit *commit,\n \t\tname = xmalloc(sizeof(rev_name));\n \t\tcommit->util = name;\n \t\tgoto copy_data;\n-\t} else if (name->taggerdate > taggerdate ||\n-\t\t\t(name->taggerdate == taggerdate &&\n-\t\t\t name->distance > distance)) {\n+\t} else if (is_better_name(name, tip_name, taggerdate,\n+\t\t\t\t generation, distance)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n-- \n2.12.0-306-g4a9b9b32d4\n\n"},{"id":"314200","messageId":"CAGZ79kZEzdSbTpaZqmcSwTBAPTPoV4UpmoVH=tHZm1ij3vLrcw@mail.gmail.com","threadId":"45385","inReplyTo":"20170315225045.15788-1-gitster@pobox.com","subject":"Re: [PATCH 0/2] Teach name-rev to pay more attention to lightweight tags","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-16T00:14:21Z","receivedAt":"2017-03-16T00:15:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 15, 2017 at 3:50 PM, Junio C Hamano wrote:\n> Here is a reroll of what I did in\n>\n> http://public-inbox.org/git/xmqqd1die00j.fsf@gitster.mtv.corp.google.com/\n>\n\nBoth patches look good to me.\n\nThanks,\nStefan\n"},{"id":"314215","messageId":"CA+P7+xqa4Ku7M5ZjrHNpeetnAguZpjz920L=Q5+7mb2A0rFywQ@mail.gmail.com","threadId":"45385","inReplyTo":"20170315225045.15788-1-gitster@pobox.com","subject":"Re: [PATCH 0/2] Teach name-rev to pay more attention to lightweight tags","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-03-16T10:28:02Z","receivedAt":"2017-03-16T10:28:30Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Mar 15, 2017 at 3:50 PM, Junio C Hamano wrote:\n> Here is a reroll of what I did in\n>\n> http://public-inbox.org/git/xmqqd1die00j.fsf@gitster.mtv.corp.google.com/\n>\n> Junio C Hamano (2):\n> name-rev: refactor logic to see if a new candidate is a better name\n> name-rev: favor describing with tags and use committer date to tiebreak\n>\n> builtin/name-rev.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++------\n> t/t4202-log.sh | 2 +-\n> 2 files changed, 57 insertions(+), 8 deletions(-)\n>\n> --\n> 2.12.0-306-g4a9b9b32d4\n>\n\nThis looks like an improvement to me. It may be that someone comes up\nwith a better definition for is_better_name() but it's definitely much\neasier to do so now as it's easier to understand what the logic is\nwhen it's separated out.\n\nThanks,\nJake\n"},{"id":"314303","messageId":"9AE7ADCA-97F9-4857-AC55-76C4BD822C25@gmail.com","threadId":"45385","inReplyTo":"20170315225045.15788-3-gitster@pobox.com","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-03-17T04:07:06Z","receivedAt":"2017-03-17T04:08:48Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 16 Mar 2017, at 06:50, Junio C Hamano wrote:\n> \n> \"git name-rev\" assigned a phony \"far in the future\" date to tips of\n> refs that are not pointing at tag objects, and favored names based\n> on a ref with the oldest date. This made it almost impossible for\n> an unannotated tags and branches to be counted as a viable base,\n> which was especially problematic when the command is run with the\n> \"--tags\" option. If an unannotated tag that points at an ancient\n> commit and an annotated tag that points at a much newer commit\n> reaches the commit that is being named, the old unannotated tag was\n> ignored.\n> \n> Update the \"taggerdate\" field of the rev-name structure, which is\n> initialized from the tip of ref, to have the committer date if the\n> object at the tip of ref is a commit, not a tag, so that we can\n> optionally take it into account when doing \"is this name better?\"\n> comparison logic.\n> \n> When \"name-rev\" is run without the \"--tags\" option, the general\n> expectation is still to name the commit based on a tag if possible,\n> but use non-tag refs as fallback, and tiebreak among these non-tag\n> refs by favoring names with shorter hops from the tip. The use of a\n> phony \"far in the future\" date in the original code was an effective\n> way to ensure this expectation is held: a non-tag tip gets the same\n> \"far in the future\" timestamp, giving precedence to tags, and among\n> non-tag tips, names with shorter hops are preferred over longer\n> hops, without taking the \"taggerdate\" into account. As we are\n> taking over the \"taggerdate\" field to store the committer date for\n> tips with commits:\n> \n> (1) keep the original logic when comparing names based on two refs\n> both of which are from refs/tags/;\n> \n> (2) favoring a name based on a ref in refs/tags/ hierarchy over\n> a ref outside the hierarchy;\n> \n> (3) between two names based on a ref both outside refs/tags/, give\n> precedence to a name with shorter hops and use \"taggerdate\"\n> only to tie-break.\n> \n> A change to t4202 is a natural consequence. The test creates a\n> commit on a branch \"side\" and points at it with an unannotated tag\n> \"refs/tags/side-2\". The original code couldn't decide which one to\n> favor at all, and gave a name based on a branch (simply because\n> refs/heads/side sorts earlier than refs/tags/side-2). Because the\n> updated logic is taught to favor refs in refs/tags/ hierarchy, the\n> the test is updated to expect to see tags/side-2 instead.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n\nA quick bisect indicates that this patch might break \nt9807-git-p4-submit.sh 8 and 13. I haven't looked into\nit further, yet.\n\nhttps://travis-ci.org/git/git/jobs/211948406#L2152\nhttps://travis-ci.org/git/git/jobs/211948406#L2460\n\nNon-JS: https://s3.amazonaws.com/archive.travis-ci.org/jobs/211948406/log.txt\n\n- Lars\n"},{"id":"314310","messageId":"xmqqy3w48nif.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"9AE7ADCA-97F9-4857-AC55-76C4BD822C25@gmail.com","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T05:45:28Z","receivedAt":"2017-03-17T05:46:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> A quick bisect indicates that this patch might break \n> t9807-git-p4-submit.sh 8 and 13. I haven't looked into\n> it further, yet.\n\nAs I do not do P4, help in diagnosing why it breaks is appreciated.\nIf the test script expects a commit, that can be described in two\nmore more ways, is named one way (e.g. named after a branch) and the\nnew \"favor tags even if they are unannotated\" behaviour breaks its\nexpectation, and the only thing the test _should_ care about is what\ncommit the result is, then clearly the test script needs to be fixed.\nOn the other hand, if git-p4 command internally uses name-rev and it\nis not prepared to properly handle commits that can be named in more\nthan one way, the problem would be deeper, as it would mean it can\nmisbehave even without the change to name-rev when multiple branches\npoint at the same commit.\n\nThanks.\n\n"},{"id":"314312","messageId":"xmqqshmc8n09.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"xmqqy3w48nif.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T05:56:22Z","receivedAt":"2017-03-17T05:57:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Lars Schneider writes:\n>\n>> A quick bisect indicates that this patch might break \n>> t9807-git-p4-submit.sh 8 and 13. I haven't looked into\n>> it further, yet.\n>\n> As I do not do P4, help in diagnosing why it breaks is appreciated.\n> If the test script expects...\n> On the other hand, if git-p4 command internally uses name-rev and it\n> is not prepared to properly handle commits that can be named in more\n> than one way, the problem would be deeper, as it would mean it can\n> misbehave even without the change to name-rev when multiple branches\n> point at the same commit.\n\nYikes. Perhaps something along this line? \n\nThis function seems to want to learn which branch we are on, and\nrunning \"name-rev HEAD\" is *NEVER* the right way to do so. If you\nare on branch B which happens to point at the same commit as branch\nA, \"name-rev HEAD\" can say either A or B (and it is likely it would\nsay A simply because it sorts earlier, and the logic seems to favor\nthe one that was discovered earlier when all else being equal).\n\n git-p4.py | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex eab319d76e..351d1ab58e 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -582,7 +582,7 @@ def currentGitBranch():\n # on a detached head\n return None\n else:\n- return read_pipe([\"git\", \"name-rev\", \"HEAD\"]).split(\" \")[1].strip()\n+ return read_pipe([\"git\", \"symbolic-ref\", \"HEAD\"]).strip()[11:]\n \n def isValidGitDir(path):\n return git_dir(path) != None\n"},{"id":"314319","messageId":"8b74cbad-dca5-4dc8-4656-1568fb2f1868@drmicha.warpmail.net","threadId":"45385","inReplyTo":"xmqqvarae3zy.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/3] git-prompt: add a describe style for any tags","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2017-03-17T10:56:49Z","receivedAt":"2017-03-17T10:56:56Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 15.03.2017 20:25:\n> Michael J Gruber writes:\n> \n>> git-prompt has various describe styles, among them \"describe\" (by\n>> annotated tags) and \"default\" (by exact match with any tag).\n>>\n>> Add a mode \"tag\" that describes by any tag, annotated or not.\n>>\n>> Signed-off-by: Michael J Gruber \n>> ---\n>> contrib/completion/git-prompt.sh | 3 +++\n>> 1 file changed, 3 insertions(+)\n>>\n>> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\n>> index 97eacd7832..c6cbef38c2 100644\n>> --- a/contrib/completion/git-prompt.sh\n>> +++ b/contrib/completion/git-prompt.sh\n>> @@ -82,6 +82,7 @@\n>> # contains relative to newer annotated tag (v1.6.3.2~35)\n>> # branch relative to newer tag or branch (master~4)\n>> # describe relative to older annotated tag (v1.6.3.1-13-gdd42c2f)\n>> +# tag relative to any older tag (v1.6.3.1-13-gdd42c2f)\n> \n> I guess this feature makes sense. \n> \n> I just wish we had a well-known unannotated tag we can use for such\n> an example; using v1.6.3.1 that is annotated does not help to make\n> the distinctin between describe and tag stand out. We want to\n> convey \"both annotated one and unannotated one can be used\".\n> \n> I am wondering if it makes sense to do something like this instead:\n> \n> #\ttag \tsimilar to 'describe' but also allow unannotated tags\n\nor inventing a local lightweight tag such as \"lastgood-5-gbadbad\").\nEither way would be fine with me. (I guess it's in next as is now.)\n\n>> # default exactly matching tag\n>> #\n>> # If you would like a colored hint about the current dirty state, set\n>> @@ -443,6 +444,8 @@ __git_ps1 ()\n>> \t\t\t\t\tgit describe --contains HEAD ;;\n>> \t\t\t\t(branch)\n>> \t\t\t\t\tgit describe --contains --all HEAD ;;\n>> +\t\t\t\t(tag)\n>> +\t\t\t\t\tgit describe --tags HEAD ;;\n>> \t\t\t\t(describe)\n>> \t\t\t\t\tgit describe HEAD ;;\n>> \t\t\t\t(* | default)\n"},{"id":"314323","messageId":"a76bd1ae-74a8-a85c-852a-e36514cd05d5@drmicha.warpmail.net","threadId":"45385","inReplyTo":"xmqqzigme474.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] describe: debug is incompatible with contains","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2017-03-17T10:54:29Z","receivedAt":"2017-03-17T11:31:36Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 15.03.2017 20:21:\n> Michael J Gruber writes:\n> \n>> `git describe --contains` calls into `git name-rev` which does not have\n>> any searching to do and thus does not display any debug information.\n>>\n>> Say so in the documentation and catch the incompatible arguments.\n> \n> I am not sure if this is worth it. Those who are really doing the\n> debugging would be staring at the code while running it anyway, so\n> it is not like this new error condition would help anybody from\n> wasting time scratching her head before viewing the source and\n> realize that the underlying name-rev does not honor the option.\n\nThe story was: I tried to understand why git describe --contains did not\ndo what I expected. The documentation said that --debug would output\ncandidates, but it did not do anything.\n\nSo, instead of learning how git describe --contains works, I got\nside-tracked into understanding why git describe --contains does not do\nwhat the documentation says. That was a waste of time that we can avoid.\n\n\"viewing the source\" should not be necessary to understand what is going\non, should it?\n\n> If \"--debug\" is truly valuable, \"name-rev\" can gain \"--debug\" later\n> and then we can pass it down if we want.\n> \n> Also, it is not like \"--debug\" is incompatible. It is just the\n> \"--contains\" codepath is overly silent and does not give any useful\n> information when run in the debug mode. \"incompatible\" is more like\n> \"would not work correctly when both are given\", which is not the\n> case here.\n\nWell, thee notion of giving debug output is certainly not incompatible.\nBut if the \"--debug\" does not output anything with \"--contains\" then it\nis not working, which I would call incompatible (implementation, not\nconcept).\n\n>>\n>> Signed-off-by: Michael J Gruber \n>> ---\n>> Documentation/git-describe.txt | 2 +-\n>> builtin/describe.c | 2 ++\n>> 2 files changed, 3 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\n>> index 8755f3af7b..0f9adb6e9a 100644\n>> --- a/Documentation/git-describe.txt\n>> +++ b/Documentation/git-describe.txt\n>> @@ -69,7 +69,7 @@ OPTIONS\n>> --debug::\n>> \tVerbosely display information about the searching strategy\n>> \tbeing employed to standard error. The tag name will still\n>> -\tbe printed to standard out.\n>> +\tbe printed to standard out. This is incompatible with --contains.\n>> \n>> --long::\n>> \tAlways output the long format (the tag, the number of commits\n>> diff --git a/builtin/describe.c b/builtin/describe.c\n>> index 76c18059bf..01a6d159a0 100644\n>> --- a/builtin/describe.c\n>> +++ b/builtin/describe.c\n>> @@ -462,6 +462,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n>> \n>> \tif (longformat && abbrev == 0)\n>> \t\tdie(_(\"--long is incompatible with --abbrev=0\"));\n>> +\tif (contains && debug)\n>> +\t\tdie(_(\"--debug is incompatible with --contains\"));\n>> \n>> \tif (contains) {\n>> \t\tstruct string_list_item *item;\n"},{"id":"314324","messageId":"ce5252a5-b94a-a389-6b3e-0e4d7b243210@drmicha.warpmail.net","threadId":"45385","inReplyTo":"20170315225045.15788-3-gitster@pobox.com","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2017-03-17T11:25:01Z","receivedAt":"2017-03-17T11:31:37Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"> hops, without taking the \"taggerdate\" into account. As we are\n> taking over the \"taggerdate\" field to store the committer date for\n> tips with commits:\n> \n> (1) keep the original logic when comparing names based on two refs\n> both of which are from refs/tags/;\n> \n> (2) favoring a name based on a ref in refs/tags/ hierarchy over\n> a ref outside the hierarchy;\n> \n> (3) between two names based on a ref both outside refs/tags/, give\n> precedence to a name with shorter hops and use \"taggerdate\"\n> only to tie-break.\n> \n> A change to t4202 is a natural consequence. The test creates a\n> commit on a branch \"side\" and points at it with an unannotated tag\n> \"refs/tags/side-2\". The original code couldn't decide which one to\n> favor at all, and gave a name based on a branch (simply because\n> refs/heads/side sorts earlier than refs/tags/side-2). Because the\n> updated logic is taught to favor refs in refs/tags/ hierarchy, the\n> the test is updated to expect to see tags/side-2 instead.\n> \n> Signed-off-by: Junio C Hamano \n\nI think that strategy is fine and works as I expected in all tested\ncases. Thanks!\n\n> ---\n> \n> * I am sure others can add better heurisitics in is_better_name()\n> for comparing names based on non-tag refs, and I do not mind\n> people disagreeing with the logic that this patch happens to\n> implement. This is done primarily to illustrate the value of\n> using a separate helper function is_better_name() instead of\n> open-coding the selection logic in name_rev() function.\n> ---\n> builtin/name-rev.c | 57 +++++++++++++++++++++++++++++++++++++++++++++---------\n> t/t4202-log.sh | 2 +-\n> 2 files changed, 49 insertions(+), 10 deletions(-)\n> \n> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n> index f64c71d9bc..eac0180c62 100644\n> --- a/builtin/name-rev.c\n> +++ b/builtin/name-rev.c\n> @@ -13,6 +13,7 @@ typedef struct rev_name {\n> \tunsigned long taggerdate;\n> \tint generation;\n> \tint distance;\n> +\tint from_tag;\n> } rev_name;\n> \n> static long cutoff = LONG_MAX;\n> @@ -24,16 +25,47 @@ static int is_better_name(struct rev_name *name,\n> \t\t\t const char *tip_name,\n> \t\t\t unsigned long taggerdate,\n> \t\t\t int generation,\n> -\t\t\t int distance)\n> +\t\t\t int distance,\n> +\t\t\t int from_tag)\n> {\n> -\treturn (name->taggerdate > taggerdate ||\n> -\t\t(name->taggerdate == taggerdate &&\n> -\t\t name->distance > distance));\n> +\t/*\n> +\t * When comparing names based on tags, prefer names\n> +\t * based on the older tag, even if it is farther away.\n> +\t */\n> +\tif (from_tag && name->from_tag)\n> +\t\treturn (name->taggerdate > taggerdate ||\n> +\t\t\t(name->taggerdate == taggerdate &&\n> +\t\t\t name->distance > distance));\n> +\n> +#define COMPARE(attribute, smaller_is_better)\t \\\n> +\tif (name->attribute > attribute) \\\n> +\t\treturn smaller_is_better; \\\n> +\tif (name->attribute < attribute) \\\n> +\t\treturn !smaller_is_better\n\nI find that define pretty confusing. On first reading, the \"==\" case\nseems to be missing, but that is basically \"pass\" as one can see in the\nlater code.\n\nAlso, the comparison \">\" and \"<\" is used to switch between the cases\n\"tag vs. non-tag\" and \"non-tag vs. tag\" which happen to be encoded by\nthe from_tag attribute beeing \"1 vs. 0\" resp \"0 vs. 1\" in the following:\n\n> +\n> +\t/*\n> +\t * We know that at least one of them is a non-tag at this point.\n> +\t * favor a tag over a non-tag.\n> +\t */\n> +\tCOMPARE(from_tag, 0);\n> +\n\nBut in the next two instances you use it to do \"actual\" comparisons\nbetween distances and dates:\n\n> +\t/*\n> +\t * We are now looking at two non-tags. Tiebreak to favor\n> +\t * shorter hops.\n> +\t */\n> +\tCOMPARE(distance, 1);\n> +\n> +\t/* ... or tiebreak to favor older date */\n> +\tCOMPARE(taggerdate, 1);\n> +\n> +\t/* keep the current one if we cannot decide */\n> +\treturn 0;\n> +#undef COMPARE\n> }\n\nSo, while I do understand that now, I'm wondering whether I will next\ntime ;)\n\nHow about something like\n\n/* favor tag over non-tag */\nif (name->from_tag != from_tag)\n\treturn from_tag;\n\nat least for the first instance and possibly\n\n/* favor shorter hops for non-tags */\nif (name->distance != distance)\n\treturn name->distance > distance;\n\n/* tie-break by date */\nif (name->taggerdate != taggerdate)\n\treturn name->taggerdate > taggerdate;\n\n> \n> static void name_rev(struct commit *commit,\n> \t\tconst char *tip_name, unsigned long taggerdate,\n> -\t\tint generation, int distance,\n> +\t\tint generation, int distance, int from_tag,\n> \t\tint deref)\n> {\n> \tstruct rev_name *name = (struct rev_name *)commit->util;\n> @@ -57,12 +89,13 @@ static void name_rev(struct commit *commit,\n> \t\tcommit->util = name;\n> \t\tgoto copy_data;\n> \t} else if (is_better_name(name, tip_name, taggerdate,\n> -\t\t\t\t generation, distance)) {\n> +\t\t\t\t generation, distance, from_tag)) {\n> copy_data:\n> \t\tname->tip_name = tip_name;\n> \t\tname->taggerdate = taggerdate;\n> \t\tname->generation = generation;\n> \t\tname->distance = distance;\n> +\t\tname->from_tag = from_tag;\n> \t} else\n> \t\treturn;\n> \n> @@ -82,10 +115,12 @@ static void name_rev(struct commit *commit,\n> \t\t\t\t\t\t parent_number);\n> \n> \t\t\tname_rev(parents->item, new_name, taggerdate, 0,\n> -\t\t\t\tdistance + MERGE_TRAVERSAL_WEIGHT, 0);\n> +\t\t\t\t distance + MERGE_TRAVERSAL_WEIGHT,\n> +\t\t\t\t from_tag, 0);\n> \t\t} else {\n> \t\t\tname_rev(parents->item, tip_name, taggerdate,\n> -\t\t\t\tgeneration + 1, distance + 1, 0);\n> +\t\t\t\t generation + 1, distance + 1,\n> +\t\t\t\t from_tag, 0);\n> \t\t}\n> \t}\n> }\n> @@ -184,9 +219,13 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n> \t}\n> \tif (o && o->type == OBJ_COMMIT) {\n> \t\tstruct commit *commit = (struct commit *)o;\n> +\t\tint from_tag = starts_with(path, \"refs/tags/\");\n> \n> +\t\tif (taggerdate == ULONG_MAX)\n> +\t\t\ttaggerdate = ((struct commit *)o)->date;\n> \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n> -\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);\n> +\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0,\n> +\t\t\t from_tag, deref);\n> \t}\n> \treturn 0;\n> }\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 48b55bfd27..038911f230 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -398,7 +398,7 @@ cat > expect <<\\EOF\n> | |\n> | | Merge branch 'side'\n> | |\n> -| * commit side\n> +| * commit tags/side-2\n> | | Author: A U Thor \n> | |\n> | | side-2\n> \n"},{"id":"314344","messageId":"xmqqh92r99h7.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"ce5252a5-b94a-a389-6b3e-0e4d7b243210@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T16:03:16Z","receivedAt":"2017-03-17T16:18:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n>> +#define COMPARE(attribute, smaller_is_better)\t \\\n>> +\tif (name->attribute > attribute) \\\n>> +\t\treturn smaller_is_better; \\\n>> +\tif (name->attribute < attribute) \\\n>> +\t\treturn !smaller_is_better\n>\n> I find that define pretty confusing. On first reading, the \"==\" case\n> seems to be missing, but that is basically \"pass\" as one can see in the\n> later code.\n>\n> Also, the comparison \">\" and \"<\" is used to switch between the cases\n> \"tag vs. non-tag\" and \"non-tag vs. tag\" which happen to be encoded by\n> the from_tag attribute beeing \"1 vs. 0\" resp \"0 vs. 1\" in the following:\n> ...\n> But in the next two instances you use it to do \"actual\" comparisons\n> between distances and dates:\n\nThat is pretty much deliberate. The macro is designed to compare\nthe attribute and return if one side is strictly better than the\nother side, and fall through to tie-breaker that follow (i.e. lack\nof \"==\" is very deliberate). Also it is not \"return if one side is\nstrictly smaller\"---the second parameter decides if smaller means\nbetter or bigger means better (i.e. \"from_tag\" being 1 for a name\nbased on tag and 0 for a name based on non-tag can use the macro by\ntelling that the side that is strictly bigger is better).\n\n> How about something like\n> ...\n\nI did a sequence of COMPARE() macros that cascade to implement\ntie-breaking logic because I thought it was the easiest to read and\nunderstand, and I did \"tag vs non-tag first decides, and then\ntiebreak to favor shorter hops and then tiebreak on date, while\npaying attention to committer dates even when we are comparing two\ncommits\", because I thought that would be one of the easier logic to\nexplain to the users. \n\nBut this topic is not my itch, and quite honestly I'd be happier if\nyou took it over, improving both the implementation and the\nsemantics it implements, following your best judgment. You are\nequipped with better motivation than I am to make these decisions.\n\nThis patch has turned up a bug in git-p4 in that it seems to have\nmisunderstood that \"name-rev HEAD\" is a good way to learn which\nbranch we are currently on (it never is); as far as I am concerned,\nit has served its purpose ;-)\n\nThanks.\n"},{"id":"314352","messageId":"015A7026-960F-450C-9276-AAC3A0B11207@gmail.com","threadId":"45385","inReplyTo":"xmqqshmc8n09.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-03-17T17:09:50Z","receivedAt":"2017-03-17T17:11:34Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 17 Mar 2017, at 13:56, Junio C Hamano wrote:\n> \n> Junio C Hamano writes:\n> \n>> Lars Schneider writes:\n>> \n>>> A quick bisect indicates that this patch might break \n>>> t9807-git-p4-submit.sh 8 and 13. I haven't looked into\n>>> it further, yet.\n>> \n>> As I do not do P4, help in diagnosing why it breaks is appreciated.\n>> If the test script expects...\n>> On the other hand, if git-p4 command internally uses name-rev and it\n>> is not prepared to properly handle commits that can be named in more\n>> than one way, the problem would be deeper, as it would mean it can\n>> misbehave even without the change to name-rev when multiple branches\n>> point at the same commit.\n> \n> Yikes. Perhaps something along this line? \n> \n> This function seems to want to learn which branch we are on, and\n> running \"name-rev HEAD\" is *NEVER* the right way to do so. If you\n> are on branch B which happens to point at the same commit as branch\n> A, \"name-rev HEAD\" can say either A or B (and it is likely it would\n> say A simply because it sorts earlier, and the logic seems to favor\n> the one that was discovered earlier when all else being equal).\n> \n> git-p4.py | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-p4.py b/git-p4.py\n> index eab319d76e..351d1ab58e 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -582,7 +582,7 @@ def currentGitBranch():\n> # on a detached head\n> return None\n> else:\n> - return read_pipe([\"git\", \"name-rev\", \"HEAD\"]).split(\" \")[1].strip()\n> + return read_pipe([\"git\", \"symbolic-ref\", \"HEAD\"]).strip()[11:]\n> \n> def isValidGitDir(path):\n> return git_dir(path) != None\n\nFollowing your explanation this patch looks good to me and this fixes the\ntest failure. TBH I never thought about the difference of these commands\nbefore. \"rev\" and \"ref\" sound so similar although they denote completely \ndifferent things.\n\nThanks,\nLars"},{"id":"314353","messageId":"xmqqd1df961c.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"015A7026-960F-450C-9276-AAC3A0B11207@gmail.com","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T17:17:35Z","receivedAt":"2017-03-17T17:17:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> git-p4.py | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>> \n>> diff --git a/git-p4.py b/git-p4.py\n>> index eab319d76e..351d1ab58e 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -582,7 +582,7 @@ def currentGitBranch():\n>> # on a detached head\n>> return None\n>> else:\n>> - return read_pipe([\"git\", \"name-rev\", \"HEAD\"]).split(\" \")[1].strip()\n>> + return read_pipe([\"git\", \"symbolic-ref\", \"HEAD\"]).strip()[11:]\n>> \n>> def isValidGitDir(path):\n>> return git_dir(path) != None\n>\n> Following your explanation this patch looks good to me and this fixes the\n> test failure. TBH I never thought about the difference of these commands\n> before. \"rev\" and \"ref\" sound so similar although they denote completely \n> different things.\n\nThanks for testing.\n\nThe above was done merely to point out the problematic place and a\npossible solution. As I am not familiar with the code in git-p4.py,\nI didn't even try to check if the code already has a helper function\nthat strips \"refs/heads/\" from the beginning of the string (iow, I\nam not happy with the [11:]). I didn't and don't like the fact that\nthis function now runs \"symbolic-ref HEAD\" twice but I didn't try to\nsee if there are more suitable and idiomatic ways to do this with a\nsingle invocation.\n\nHence, I would really prefer not to commit mine myself. I'd rather\nsee somebody from git-p4 circle to come up with a version that is\nmore in line with the way things are done in the existing code and\nsend a tested version for me to apply.\n\nThanks.\n\n\n"},{"id":"314407","messageId":"xmqqa88j5xsx.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"xmqqd1df961c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T22:43:42Z","receivedAt":"2017-03-17T22:43:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Lars Schneider writes:\n> ...\n>> Following your explanation this patch looks good to me and this fixes the\n>> test failure. TBH I never thought about the difference of these commands\n>> before. \"rev\" and \"ref\" sound so similar although they denote completely \n>> different things.\n>\n> Thanks for testing.\n> ...\n> Hence, I would really prefer not to commit mine myself. I'd rather\n> see somebody from git-p4 circle to come up with a version that is\n> more in line with the way things are done in the existing code and\n> send a tested version for me to apply.\n\nTentatively I queued my hack together with the name-rev thing on\n'pu', and Travis seems to be OK with the result. \n\nIt would be nice if we can \"fix\" the use of \"name-rev HEAD\" in \"git\np4\" sooner rather than later. If the code truly uses it to figure\nout what branch we are currently on and acts on it, as the name of\nthat function suggests, it may be easy to construct a testcase where\nthe code without the fix does a wrong thing (e.g. have two branches\npointing at the same commit, and tell \"git p4\" to act on the one\nthat sorts later in the \"git branch --list\" output; the command\nwould act as if it were working on the other branch), and shows that\nthe patch fixes that problem. That would be a bug worth fixing\nquickly regardless of the \"name-rev\" change michael wanted to make.\n\nThanks.\n\n"},{"id":"315669","messageId":"516eccf0ff0df009d915c0ff2aa1a3d61ef63273.1490798142.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqa88j5xsx.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 2/3] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-29T14:39:16Z","receivedAt":"2017-03-29T14:39:39Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Junio C Hamano \n\n\"git name-rev\" assigned a phony \"far in the future\" date to tips of\nrefs that are not pointing at tag objects, and favored names based\non a ref with the oldest date. This made it almost impossible for\nan unannotated tags and branches to be counted as a viable base,\nwhich was especially problematic when the command is run with the\n\"--tags\" option. If an unannotated tag that points at an ancient\ncommit and an annotated tag that points at a much newer commit\nreaches the commit that is being named, the old unannotated tag was\nignored.\n\nUpdate the \"taggerdate\" field of the rev-name structure, which is\ninitialized from the tip of ref, to have the committer date if the\nobject at the tip of ref is a commit, not a tag, so that we can\noptionally take it into account when doing \"is this name better?\"\ncomparison logic.\n\nWhen \"name-rev\" is run without the \"--tags\" option, the general\nexpectation is still to name the commit based on a tag if possible,\nbut use non-tag refs as fallback, and tiebreak among these non-tag\nrefs by favoring names with shorter hops from the tip. The use of a\nphony \"far in the future\" date in the original code was an effective\nway to ensure this expectation is held: a non-tag tip gets the same\n\"far in the future\" timestamp, giving precedence to tags, and among\nnon-tag tips, names with shorter hops are preferred over longer\nhops, without taking the \"taggerdate\" into account. As we are\ntaking over the \"taggerdate\" field to store the committer date for\ntips with commits:\n\n (1) keep the original logic when comparing names based on two refs\n both of which are from refs/tags/;\n\n (2) favoring a name based on a ref in refs/tags/ hierarchy over\n a ref outside the hierarchy;\n\n (3) between two names based on a ref both outside refs/tags/, give\n precedence to a name with shorter hops and use \"taggerdate\"\n only to tie-break.\n\nA change to t4202 is a natural consequence. The test creates a\ncommit on a branch \"side\" and points at it with an unannotated tag\n\"refs/tags/side-2\". The original code couldn't decide which one to\nfavor at all, and gave a name based on a branch (simply because\nrefs/heads/side sorts earlier than refs/tags/side-2). Because the\nupdated logic is taught to favor refs in refs/tags/ hierarchy, the\nthe test is updated to expect to see tags/side-2 instead.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I am sure others can add better heurisitics in is_better_name()\n for comparing names based on non-tag refs, and I do not mind\n people disagreeing with the logic that this patch happens to\n implement. This is done primarily to illustrate the value of\n using a separate helper function is_better_name() instead of\n open-coding the selection logic in name_rev() function.\nSigned-off-by: Michael J Gruber \n---\n builtin/name-rev.c | 53 ++++++++++++++++++++++++++++++++++++++++++++---------\n t/t4202-log.sh | 2 +-\n 2 files changed, 45 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 7890f826ce..bf7ed015ae 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -13,6 +13,7 @@ typedef struct rev_name {\n \tunsigned long taggerdate;\n \tint generation;\n \tint distance;\n+\tint from_tag;\n } rev_name;\n \n static long cutoff = LONG_MAX;\n@@ -24,16 +25,43 @@ static int is_better_name(struct rev_name *name,\n \t\t\t const char *tip_name,\n \t\t\t unsigned long taggerdate,\n \t\t\t int generation,\n-\t\t\t int distance)\n+\t\t\t int distance,\n+\t\t\t int from_tag)\n {\n-\treturn (name->taggerdate > taggerdate ||\n-\t\t(name->taggerdate == taggerdate &&\n-\t\t name->distance > distance));\n+\t/*\n+\t * When comparing names based on tags, prefer names\n+\t * based on the older tag, even if it is farther away.\n+\t */\n+\tif (from_tag && name->from_tag)\n+\t\treturn (name->taggerdate > taggerdate ||\n+\t\t\t(name->taggerdate == taggerdate &&\n+\t\t\t name->distance > distance));\n+\n+\t/*\n+\t * We know that at least one of them is a non-tag at this point.\n+\t * favor a tag over a non-tag.\n+\t */\n+\tif (name->from_tag != from_tag)\n+\t\treturn from_tag;\n+\n+\t/*\n+\t * We are now looking at two non-tags. Tiebreak to favor\n+\t * shorter hops.\n+\t */\n+\tif (name->distance != distance)\n+\t\treturn name->distance > distance;\n+\n+\t/* ... or tiebreak to favor older date */\n+\tif (name->taggerdate != taggerdate)\n+\t\treturn name->taggerdate > taggerdate;\n+\n+\t/* keep the current one if we cannot decide */\n+\treturn 0;\n }\n \n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n-\t\tint generation, int distance,\n+\t\tint generation, int distance, int from_tag,\n \t\tint deref)\n {\n \tstruct rev_name *name = (struct rev_name *)commit->util;\n@@ -57,12 +85,13 @@ static void name_rev(struct commit *commit,\n \t\tcommit->util = name;\n \t\tgoto copy_data;\n \t} else if (is_better_name(name, tip_name, taggerdate,\n-\t\t\t\t generation, distance)) {\n+\t\t\t\t generation, distance, from_tag)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n \t\tname->generation = generation;\n \t\tname->distance = distance;\n+\t\tname->from_tag = from_tag;\n \t} else\n \t\treturn;\n \n@@ -82,10 +111,12 @@ static void name_rev(struct commit *commit,\n \t\t\t\t\t\t parent_number);\n \n \t\t\tname_rev(parents->item, new_name, taggerdate, 0,\n-\t\t\t\tdistance + MERGE_TRAVERSAL_WEIGHT, 0);\n+\t\t\t\t distance + MERGE_TRAVERSAL_WEIGHT,\n+\t\t\t\t from_tag, 0);\n \t\t} else {\n \t\t\tname_rev(parents->item, tip_name, taggerdate,\n-\t\t\t\tgeneration + 1, distance + 1, 0);\n+\t\t\t\t generation + 1, distance + 1,\n+\t\t\t\t from_tag, 0);\n \t\t}\n \t}\n }\n@@ -216,9 +247,13 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t}\n \tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n+\t\tint from_tag = starts_with(path, \"refs/tags/\");\n \n+\t\tif (taggerdate == ULONG_MAX)\n+\t\t\ttaggerdate = ((struct commit *)o)->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n-\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);\n+\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0,\n+\t\t\t from_tag, deref);\n \t}\n \treturn 0;\n }\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex f577990716..4afd8135cd 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -399,7 +399,7 @@ cat > expect <<\\EOF\n | |\n | | Merge branch 'side'\n | |\n-| * commit side\n+| * commit tags/side-2\n | | Author: A U Thor \n | |\n | | side-2\n-- \n2.12.2.712.gba4c48c431\n\n"},{"id":"315670","messageId":"cover.1490798142.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqa88j5xsx.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 0/3] name-rev sanity","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-29T14:39:14Z","receivedAt":"2017-03-29T14:39:40Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"So here is v2 of the name-rev series, the result of our discussions being:\n\nJunio C Hamano (2):\n name-rev: refactor logic to see if a new candidate is a better name\n name-rev: favor describing with tags and use committer date to\n tiebreak\n\nThat second patch is slighty changed as discussed, but still mostly Junio's\n\nMichael J Gruber (1):\n name-rev: provide debug output\n\nThis replaces the patch which documented that --debug does not work with --contains :)\n\n builtin/describe.c | 2 +\n builtin/name-rev.c | 117 +++++++++++++++++++++++++++++++++++++++++++++--------\n t/t4202-log.sh | 2 +-\n 3 files changed, 103 insertions(+), 18 deletions(-)\n\n-- \n2.12.2.712.gba4c48c431\n\n"},{"id":"315671","messageId":"a2687576f270d76fda1ecfa4c0df2e99bd641e5e.1490798142.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqa88j5xsx.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 1/3] name-rev: refactor logic to see if a new candidate is a better name","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-29T14:39:15Z","receivedAt":"2017-03-29T14:39:42Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Junio C Hamano \n\nWhen we encounter a new ref that could describe the commit we are\nlooking at, we compare the name that is formed using that ref and\nthe name we found so far and pick a better one.\n\nFactor the comparison logic out to a separate helper function, while\nkeeping the current logic the same (i.e. a name that is based on an\nolder tag is better, and if two tags of the same age can reach the\ncommit, the one with fewer number of hops to reach the commit is\nbetter).\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael J Gruber \n---\n builtin/name-rev.c | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 8bdc3eaa6f..7890f826ce 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -20,6 +20,17 @@ static long cutoff = LONG_MAX;\n /* How many generations are maximally preferred over _one_ merge traversal? */\n #define MERGE_TRAVERSAL_WEIGHT 65535\n \n+static int is_better_name(struct rev_name *name,\n+\t\t\t const char *tip_name,\n+\t\t\t unsigned long taggerdate,\n+\t\t\t int generation,\n+\t\t\t int distance)\n+{\n+\treturn (name->taggerdate > taggerdate ||\n+\t\t(name->taggerdate == taggerdate &&\n+\t\t name->distance > distance));\n+}\n+\n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n \t\tint generation, int distance,\n@@ -45,9 +56,8 @@ static void name_rev(struct commit *commit,\n \t\tname = xmalloc(sizeof(rev_name));\n \t\tcommit->util = name;\n \t\tgoto copy_data;\n-\t} else if (name->taggerdate > taggerdate ||\n-\t\t\t(name->taggerdate == taggerdate &&\n-\t\t\t name->distance > distance)) {\n+\t} else if (is_better_name(name, tip_name, taggerdate,\n+\t\t\t\t generation, distance)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n-- \n2.12.2.712.gba4c48c431\n\n"},{"id":"315672","messageId":"a9630cb3485bd2933f0addc65b90da57a09b783d.1490798142.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqa88j5xsx.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 3/3] name-rev: provide debug output","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-29T14:39:17Z","receivedAt":"2017-03-29T14:39:47Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, `git describe --contains --debug` does not create any debug\noutput because it does not pass the flag down to `git name-rev`, which\ndoes not know that flag.\n\nTeach the latter that flag and the former to pass it down.\n\nThe output is patterned after that of `git describe --debug`, with the\nfollowing differences:\n\ndescribe loops over all args to describe, then over all possible\ndescriptions; name-rev does it the other way round. Therefore, we need\nto amend each possible description by the arg that it is for (and we\nleave out the \"searching to describe\" header).\n\nThe date cut-off for name-rev kicks in way more often than the candidate\nnumber cut-off of describe, so we do not clutter the output with the\ncut-off.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/describe.c | 2 ++\n builtin/name-rev.c | 64 +++++++++++++++++++++++++++++++++++++++++++-----------\n 2 files changed, 53 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex a5cd8c513f..30196793f0 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -486,6 +486,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\t\t NULL);\n \t\tif (always)\n \t\t\targv_array_push(&args, \"--always\");\n+\t\tif (debug)\n+\t\t\targv_array_push(&args, \"--debug\");\n \t\tif (!all) {\n \t\t\targv_array_push(&args, \"--tags\");\n \t\t\tfor_each_string_list_item(item, &patterns)\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex bf7ed015ae..51302a79ba 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -18,6 +18,10 @@ typedef struct rev_name {\n \n static long cutoff = LONG_MAX;\n \n+static const char *prio_names[] = {\n+\tN_(\"head\"), N_(\"lightweight\"), N_(\"annotated\"),\n+};\n+\n /* How many generations are maximally preferred over _one_ merge traversal? */\n #define MERGE_TRAVERSAL_WEIGHT 65535\n \n@@ -59,10 +63,19 @@ static int is_better_name(struct rev_name *name,\n \treturn 0;\n }\n \n+struct name_ref_data {\n+\tint tags_only;\n+\tint name_only;\n+\tint debug;\n+\tstruct string_list ref_filters;\n+\tstruct string_list exclude_filters;\n+\tstruct object_array *revs;\n+};\n+\n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n \t\tint generation, int distance, int from_tag,\n-\t\tint deref)\n+\t\tint deref, struct name_ref_data *data)\n {\n \tstruct rev_name *name = (struct rev_name *)commit->util;\n \tstruct commit_list *parents;\n@@ -75,6 +88,7 @@ static void name_rev(struct commit *commit,\n \n \tif (deref) {\n \t\ttip_name = xstrfmt(\"%s^0\", tip_name);\n+\t\tfrom_tag += 1;\n \n \t\tif (generation)\n \t\t\tdie(\"generation: %d, but deref?\", generation);\n@@ -87,6 +101,36 @@ static void name_rev(struct commit *commit,\n \t} else if (is_better_name(name, tip_name, taggerdate,\n \t\t\t\t generation, distance, from_tag)) {\n copy_data:\n+\t\tif (data->debug) {\n+\t\t\tint i;\n+\t\t\tstatic int label_width = -1;\n+\t\t\tstatic int name_width = -1;\n+\t\t\tif (label_width < 0) {\n+\t\t\t\tint w;\n+\t\t\t\tfor (i = 0; i < ARRAY_SIZE(prio_names); i++) {\n+\t\t\t\t\tw = strlen(_(prio_names[i]));\n+\t\t\t\t\tif (label_width < w)\n+\t\t\t\t\t\tlabel_width = w;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (name_width < 0) {\n+\t\t\t\tint w;\n+\t\t\t\tfor (i = 0; i < data->revs->nr; i++) {\n+\t\t\t\t\tw = strlen(data->revs->objects[i].name);\n+\t\t\t\t\tif (name_width < w)\n+\t\t\t\t\t\tname_width = w;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tfor (i = 0; i < data->revs->nr; i++)\n+\t\t\t\tif (!oidcmp(&commit->object.oid,\n+\t\t\t\t\t &data->revs->objects[i].item->oid))\n+\t\t\t\t\tfprintf(stderr, \" %-*s %8d %-*s %s~%d\\n\",\n+\t\t\t\t\t\tlabel_width,\n+\t\t\t\t\t\t_(prio_names[from_tag]),\n+\t\t\t\t\t\tdistance, name_width,\n+\t\t\t\t\t\tdata->revs->objects[i].name,\n+\t\t\t\t\t\ttip_name, generation);\n+\t\t}\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n \t\tname->generation = generation;\n@@ -112,11 +156,11 @@ static void name_rev(struct commit *commit,\n \n \t\t\tname_rev(parents->item, new_name, taggerdate, 0,\n \t\t\t\t distance + MERGE_TRAVERSAL_WEIGHT,\n-\t\t\t\t from_tag, 0);\n+\t\t\t\t from_tag, 0, data);\n \t\t} else {\n \t\t\tname_rev(parents->item, tip_name, taggerdate,\n \t\t\t\t generation + 1, distance + 1,\n-\t\t\t\t from_tag, 0);\n+\t\t\t\t from_tag, 0, data);\n \t\t}\n \t}\n }\n@@ -146,13 +190,6 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)\n \treturn refname;\n }\n \n-struct name_ref_data {\n-\tint tags_only;\n-\tint name_only;\n-\tstruct string_list ref_filters;\n-\tstruct string_list exclude_filters;\n-};\n-\n static struct tip_table {\n \tstruct tip_table_entry {\n \t\tunsigned char sha1[20];\n@@ -236,7 +273,6 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t}\n \n \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n-\n \twhile (o && o->type == OBJ_TAG) {\n \t\tstruct tag *t = (struct tag *) o;\n \t\tif (!t->tagged)\n@@ -253,7 +289,7 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t\t\ttaggerdate = ((struct commit *)o)->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n \t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0,\n-\t\t\t from_tag, deref);\n+\t\t\t from_tag, deref, data);\n \t}\n \treturn 0;\n }\n@@ -383,7 +419,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_array revs = OBJECT_ARRAY_INIT;\n \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n-\tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n+\tstruct name_ref_data data = { 0, 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n \tstruct option opts[] = {\n \t\tOPT_BOOL(0, \"name-only\", &data.name_only, N_(\"print only names (no SHA-1)\")),\n \t\tOPT_BOOL(0, \"tags\", &data.tags_only, N_(\"only use tags to name the commits\")),\n@@ -393,6 +429,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t\t\t\t N_(\"ignore refs matching \")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\n+\t\tOPT_BOOL(0, \"debug\", &data.debug, N_(\"debug search strategy on stderr\")),\n \t\tOPT_BOOL(0, \"stdin\", &transform_stdin, N_(\"read from stdin\")),\n \t\tOPT_BOOL(0, \"undefined\", &allow_undefined, N_(\"allow to print `undefined` names (default)\")),\n \t\tOPT_BOOL(0, \"always\", &always,\n@@ -458,6 +495,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \n \tif (cutoff)\n \t\tcutoff = cutoff - CUTOFF_DATE_SLOP;\n+\tdata.revs = &revs;\n \tfor_each_ref(name_ref, &data);\n \n \tif (transform_stdin) {\n-- \n2.12.2.712.gba4c48c431\n\n"},{"id":"315689","messageId":"xmqq4lycatsu.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"cover.1490798142.git.git@grubix.eu","subject":"Re: [PATCH v2 0/3] name-rev sanity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-29T17:15:13Z","receivedAt":"2017-03-29T17:15:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> So here is v2 of the name-rev series, the result of our discussions being:\n>\n> Junio C Hamano (2):\n> name-rev: refactor logic to see if a new candidate is a better name\n> name-rev: favor describing with tags and use committer date to\n> tiebreak\n>\n> That second patch is slighty changed as discussed, but still mostly Junio's\n>\n> Michael J Gruber (1):\n> name-rev: provide debug output\n>\n> This replaces the patch which documented that --debug does not work with --contains :)\n>\n> builtin/describe.c | 2 +\n> builtin/name-rev.c | 117 +++++++++++++++++++++++++++++++++++++++++++++--------\n> t/t4202-log.sh | 2 +-\n> 3 files changed, 103 insertions(+), 18 deletions(-)\n\nThe first two applies cleanly to the same base as jc/name-rev that\nthe first two of these patches are meant to replace, but the third\none doesn't apply on top. Are you depending on something newer?\n"},{"id":"315691","messageId":"xmqqvaqs9dws.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"xmqq4lycatsu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/3] name-rev sanity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-29T17:43:47Z","receivedAt":"2017-03-29T17:43:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> The first two applies cleanly to the same base as jc/name-rev that\n> the first two of these patches are meant to replace, but the third\n> one doesn't apply on top. Are you depending on something newer?\n\nAh, of course, you are depending on your other topic ;-)\nI'll wiggle these in.\n\nThanks.\n"},{"id":"315772","messageId":"12937baf-f36d-836c-612d-72578a5929b4@grubix.eu","threadId":"45385","inReplyTo":"xmqqvaqs9dws.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/3] name-rev sanity","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-30T13:48:12Z","receivedAt":"2017-03-30T13:48:36Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 29.03.2017 19:43:\n> Junio C Hamano writes:\n> \n>> The first two applies cleanly to the same base as jc/name-rev that\n>> the first two of these patches are meant to replace, but the third\n>> one doesn't apply on top. Are you depending on something newer?\n> \n> Ah, of course, you are depending on your other topic ;-)\n> I'll wiggle these in.\n> \n> Thanks.\n> \n\nYes, sorry, isn't that in next already? I should have meantioned it anyways.\n\nAlso, I should split patch 3 into name-rev and describe related parts\nand update the name-rev documentation. We don't have tests for describe\n--debug, that should be ok.\n\nMihael\n"},{"id":"315796","messageId":"xmqqinmq65at.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"12937baf-f36d-836c-612d-72578a5929b4@grubix.eu","subject":"Re: [PATCH v2 0/3] name-rev sanity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-30T17:30:18Z","receivedAt":"2017-03-30T17:30:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Junio C Hamano venit, vidit, dixit 29.03.2017 19:43:\n> ...\n>> Ah, of course, you are depending on your other topic ;-)\n>> I'll wiggle these in.\n>> \n>> Thanks.\n>\n> Yes, sorry, isn't that in next already? I should have meantioned it anyways.\n\nNo worries. jc/name-rev was meant to be replaced.\n\n> Also, I should split patch 3 into name-rev and describe related parts\n> and update the name-rev documentation. We don't have tests for describe\n> --debug, that should be ok.\n\nOK. I do not think these three patches are in 'next' yet, so feel\nfree to further replace the last one (or for that matter the first\ntwo as necessary). I do not think we want to cast the debugging\ninformation in stone, but it would not hurt if you made sure that\n\"describe --debug [--contains]\" does not dump core ;-)\n\nThanks.\n"},{"id":"315888","messageId":"21cf9d6f55d17463ab6eccdd78d57cf4a1b8e9e1.1490967948.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqinmq65at.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3 3/4] name-rev: provide debug output","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-31T13:51:23Z","receivedAt":"2017-03-31T13:51:50Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, `git describe --contains --debug` does not create any debug\noutput because it does not pass the flag down to `git name-rev`, which\ndoes not know that flag.\n\nTeach the latter that flag, so that the former can pass it down (in\nthe following commit).\n\nThe output is patterned after that of `git describe --debug`, with the\nfollowing differences:\n\ndescribe loops over all args to describe, then over all possible\ndescriptions; name-rev does it the other way round. Therefore, we need\nto amend each possible description by the arg that it is for (and we\nleave out the \"searching to describe\" header).\n\nThe date cut-off for name-rev kicks in way more often than the candidate\nnumber cut-off of describe, so we do not clutter the output with the\ncut-off.\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/git-name-rev.txt | 5 ++++\n builtin/name-rev.c | 64 +++++++++++++++++++++++++++++++++---------\n 2 files changed, 56 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt\nindex e8e68f528c..fd78ee86e8 100644\n--- a/Documentation/git-name-rev.txt\n+++ b/Documentation/git-name-rev.txt\n@@ -42,6 +42,11 @@ OPTIONS\n --all::\n \tList all commits reachable from all refs\n \n+--debug::\n+\tVerbosely display information about the searching strategy\n+\tbeing employed to standard error. The symbolic name will still\n+\tbe printed to standard out.\n+\n --stdin::\n \tTransform stdin by substituting all the 40-character SHA-1\n \thexes (say $hex) with \"$hex ($rev_name)\". When used with\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex bf7ed015ae..51302a79ba 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -18,6 +18,10 @@ typedef struct rev_name {\n \n static long cutoff = LONG_MAX;\n \n+static const char *prio_names[] = {\n+\tN_(\"head\"), N_(\"lightweight\"), N_(\"annotated\"),\n+};\n+\n /* How many generations are maximally preferred over _one_ merge traversal? */\n #define MERGE_TRAVERSAL_WEIGHT 65535\n \n@@ -59,10 +63,19 @@ static int is_better_name(struct rev_name *name,\n \treturn 0;\n }\n \n+struct name_ref_data {\n+\tint tags_only;\n+\tint name_only;\n+\tint debug;\n+\tstruct string_list ref_filters;\n+\tstruct string_list exclude_filters;\n+\tstruct object_array *revs;\n+};\n+\n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n \t\tint generation, int distance, int from_tag,\n-\t\tint deref)\n+\t\tint deref, struct name_ref_data *data)\n {\n \tstruct rev_name *name = (struct rev_name *)commit->util;\n \tstruct commit_list *parents;\n@@ -75,6 +88,7 @@ static void name_rev(struct commit *commit,\n \n \tif (deref) {\n \t\ttip_name = xstrfmt(\"%s^0\", tip_name);\n+\t\tfrom_tag += 1;\n \n \t\tif (generation)\n \t\t\tdie(\"generation: %d, but deref?\", generation);\n@@ -87,6 +101,36 @@ static void name_rev(struct commit *commit,\n \t} else if (is_better_name(name, tip_name, taggerdate,\n \t\t\t\t generation, distance, from_tag)) {\n copy_data:\n+\t\tif (data->debug) {\n+\t\t\tint i;\n+\t\t\tstatic int label_width = -1;\n+\t\t\tstatic int name_width = -1;\n+\t\t\tif (label_width < 0) {\n+\t\t\t\tint w;\n+\t\t\t\tfor (i = 0; i < ARRAY_SIZE(prio_names); i++) {\n+\t\t\t\t\tw = strlen(_(prio_names[i]));\n+\t\t\t\t\tif (label_width < w)\n+\t\t\t\t\t\tlabel_width = w;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (name_width < 0) {\n+\t\t\t\tint w;\n+\t\t\t\tfor (i = 0; i < data->revs->nr; i++) {\n+\t\t\t\t\tw = strlen(data->revs->objects[i].name);\n+\t\t\t\t\tif (name_width < w)\n+\t\t\t\t\t\tname_width = w;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tfor (i = 0; i < data->revs->nr; i++)\n+\t\t\t\tif (!oidcmp(&commit->object.oid,\n+\t\t\t\t\t &data->revs->objects[i].item->oid))\n+\t\t\t\t\tfprintf(stderr, \" %-*s %8d %-*s %s~%d\\n\",\n+\t\t\t\t\t\tlabel_width,\n+\t\t\t\t\t\t_(prio_names[from_tag]),\n+\t\t\t\t\t\tdistance, name_width,\n+\t\t\t\t\t\tdata->revs->objects[i].name,\n+\t\t\t\t\t\ttip_name, generation);\n+\t\t}\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n \t\tname->generation = generation;\n@@ -112,11 +156,11 @@ static void name_rev(struct commit *commit,\n \n \t\t\tname_rev(parents->item, new_name, taggerdate, 0,\n \t\t\t\t distance + MERGE_TRAVERSAL_WEIGHT,\n-\t\t\t\t from_tag, 0);\n+\t\t\t\t from_tag, 0, data);\n \t\t} else {\n \t\t\tname_rev(parents->item, tip_name, taggerdate,\n \t\t\t\t generation + 1, distance + 1,\n-\t\t\t\t from_tag, 0);\n+\t\t\t\t from_tag, 0, data);\n \t\t}\n \t}\n }\n@@ -146,13 +190,6 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)\n \treturn refname;\n }\n \n-struct name_ref_data {\n-\tint tags_only;\n-\tint name_only;\n-\tstruct string_list ref_filters;\n-\tstruct string_list exclude_filters;\n-};\n-\n static struct tip_table {\n \tstruct tip_table_entry {\n \t\tunsigned char sha1[20];\n@@ -236,7 +273,6 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t}\n \n \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n-\n \twhile (o && o->type == OBJ_TAG) {\n \t\tstruct tag *t = (struct tag *) o;\n \t\tif (!t->tagged)\n@@ -253,7 +289,7 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t\t\ttaggerdate = ((struct commit *)o)->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n \t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0,\n-\t\t\t from_tag, deref);\n+\t\t\t from_tag, deref, data);\n \t}\n \treturn 0;\n }\n@@ -383,7 +419,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_array revs = OBJECT_ARRAY_INIT;\n \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n-\tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n+\tstruct name_ref_data data = { 0, 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n \tstruct option opts[] = {\n \t\tOPT_BOOL(0, \"name-only\", &data.name_only, N_(\"print only names (no SHA-1)\")),\n \t\tOPT_BOOL(0, \"tags\", &data.tags_only, N_(\"only use tags to name the commits\")),\n@@ -393,6 +429,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t\t\t\t N_(\"ignore refs matching \")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\n+\t\tOPT_BOOL(0, \"debug\", &data.debug, N_(\"debug search strategy on stderr\")),\n \t\tOPT_BOOL(0, \"stdin\", &transform_stdin, N_(\"read from stdin\")),\n \t\tOPT_BOOL(0, \"undefined\", &allow_undefined, N_(\"allow to print `undefined` names (default)\")),\n \t\tOPT_BOOL(0, \"always\", &always,\n@@ -458,6 +495,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \n \tif (cutoff)\n \t\tcutoff = cutoff - CUTOFF_DATE_SLOP;\n+\tdata.revs = &revs;\n \tfor_each_ref(name_ref, &data);\n \n \tif (transform_stdin) {\n-- \n2.12.2.739.gfc3eb97820\n\n"},{"id":"315889","messageId":"1f59ad094670665b9db44bdc0addf7de922e0246.1490967948.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqinmq65at.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3 4/4] describe: pass --debug down to name-rev","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-31T13:51:24Z","receivedAt":"2017-03-31T13:51:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Now that name-rev knows --debug, pass that flag down to name-rev when we\ncall it for doing describe --contains.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/describe.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex a5cd8c513f..30196793f0 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -486,6 +486,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\t\t NULL);\n \t\tif (always)\n \t\t\targv_array_push(&args, \"--always\");\n+\t\tif (debug)\n+\t\t\targv_array_push(&args, \"--debug\");\n \t\tif (!all) {\n \t\t\targv_array_push(&args, \"--tags\");\n \t\t\tfor_each_string_list_item(item, &patterns)\n-- \n2.12.2.739.gfc3eb97820\n\n"},{"id":"315890","messageId":"cover.1490967948.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqinmq65at.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3 0/4] name-rev sanity","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-31T13:51:20Z","receivedAt":"2017-03-31T13:51:54Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"v3 splits the old 3/3 into name-rev and describe related parts and\nadds documentation.\n\nNo core dumps encountered so far ;)\n\nDid I mention this is on top of mg/describe-debug-l10n (next)?\n\nJunio C Hamano (2):\n name-rev: refactor logic to see if a new candidate is a better name\n name-rev: favor describing with tags and use committer date to\n tiebreak\n\nMichael J Gruber (2):\n name-rev: provide debug output\n describe: pass --debug down to name-rev\n\n Documentation/git-name-rev.txt | 5 ++\n builtin/describe.c | 2 +\n builtin/name-rev.c | 117 +++++++++++++++++++++++++++++++++++------\n t/t4202-log.sh | 2 +-\n 4 files changed, 108 insertions(+), 18 deletions(-)\n\n-- \n2.12.2.739.gfc3eb97820\n\n"},{"id":"315891","messageId":"b3d7263ef7b435c6fd64f1fc820e7f6fb668adba.1490967948.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqinmq65at.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3 1/4] name-rev: refactor logic to see if a new candidate is a better name","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-31T13:51:21Z","receivedAt":"2017-03-31T13:51:59Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Junio C Hamano \n\nWhen we encounter a new ref that could describe the commit we are\nlooking at, we compare the name that is formed using that ref and\nthe name we found so far and pick a better one.\n\nFactor the comparison logic out to a separate helper function, while\nkeeping the current logic the same (i.e. a name that is based on an\nolder tag is better, and if two tags of the same age can reach the\ncommit, the one with fewer number of hops to reach the commit is\nbetter).\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael J Gruber \n---\n builtin/name-rev.c | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 8bdc3eaa6f..7890f826ce 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -20,6 +20,17 @@ static long cutoff = LONG_MAX;\n /* How many generations are maximally preferred over _one_ merge traversal? */\n #define MERGE_TRAVERSAL_WEIGHT 65535\n \n+static int is_better_name(struct rev_name *name,\n+\t\t\t const char *tip_name,\n+\t\t\t unsigned long taggerdate,\n+\t\t\t int generation,\n+\t\t\t int distance)\n+{\n+\treturn (name->taggerdate > taggerdate ||\n+\t\t(name->taggerdate == taggerdate &&\n+\t\t name->distance > distance));\n+}\n+\n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n \t\tint generation, int distance,\n@@ -45,9 +56,8 @@ static void name_rev(struct commit *commit,\n \t\tname = xmalloc(sizeof(rev_name));\n \t\tcommit->util = name;\n \t\tgoto copy_data;\n-\t} else if (name->taggerdate > taggerdate ||\n-\t\t\t(name->taggerdate == taggerdate &&\n-\t\t\t name->distance > distance)) {\n+\t} else if (is_better_name(name, tip_name, taggerdate,\n+\t\t\t\t generation, distance)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n-- \n2.12.2.739.gfc3eb97820\n\n"},{"id":"315892","messageId":"aa0bac0a16647c74283fb392046409da094d2e41.1490967948.git.git@grubix.eu","threadId":"45385","inReplyTo":"xmqqinmq65at.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3 2/4] name-rev: favor describing with tags and use committer date to tiebreak","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-31T13:51:22Z","receivedAt":"2017-03-31T13:52:05Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Junio C Hamano \n\n\"git name-rev\" assigned a phony \"far in the future\" date to tips of\nrefs that are not pointing at tag objects, and favored names based\non a ref with the oldest date. This made it almost impossible for\nan unannotated tags and branches to be counted as a viable base,\nwhich was especially problematic when the command is run with the\n\"--tags\" option. If an unannotated tag that points at an ancient\ncommit and an annotated tag that points at a much newer commit\nreaches the commit that is being named, the old unannotated tag was\nignored.\n\nUpdate the \"taggerdate\" field of the rev-name structure, which is\ninitialized from the tip of ref, to have the committer date if the\nobject at the tip of ref is a commit, not a tag, so that we can\noptionally take it into account when doing \"is this name better?\"\ncomparison logic.\n\nWhen \"name-rev\" is run without the \"--tags\" option, the general\nexpectation is still to name the commit based on a tag if possible,\nbut use non-tag refs as fallback, and tiebreak among these non-tag\nrefs by favoring names with shorter hops from the tip. The use of a\nphony \"far in the future\" date in the original code was an effective\nway to ensure this expectation is held: a non-tag tip gets the same\n\"far in the future\" timestamp, giving precedence to tags, and among\nnon-tag tips, names with shorter hops are preferred over longer\nhops, without taking the \"taggerdate\" into account. As we are\ntaking over the \"taggerdate\" field to store the committer date for\ntips with commits:\n\n (1) keep the original logic when comparing names based on two refs\n both of which are from refs/tags/;\n\n (2) favoring a name based on a ref in refs/tags/ hierarchy over\n a ref outside the hierarchy;\n\n (3) between two names based on a ref both outside refs/tags/, give\n precedence to a name with shorter hops and use \"taggerdate\"\n only to tie-break.\n\nA change to t4202 is a natural consequence. The test creates a\ncommit on a branch \"side\" and points at it with an unannotated tag\n\"refs/tags/side-2\". The original code couldn't decide which one to\nfavor at all, and gave a name based on a branch (simply because\nrefs/heads/side sorts earlier than refs/tags/side-2). Because the\nupdated logic is taught to favor refs in refs/tags/ hierarchy, the\nthe test is updated to expect to see tags/side-2 instead.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I am sure others can add better heurisitics in is_better_name()\n for comparing names based on non-tag refs, and I do not mind\n people disagreeing with the logic that this patch happens to\n implement. This is done primarily to illustrate the value of\n using a separate helper function is_better_name() instead of\n open-coding the selection logic in name_rev() function.\nSigned-off-by: Michael J Gruber \n---\n builtin/name-rev.c | 53 ++++++++++++++++++++++++++++++++++++++++++++---------\n t/t4202-log.sh | 2 +-\n 2 files changed, 45 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 7890f826ce..bf7ed015ae 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -13,6 +13,7 @@ typedef struct rev_name {\n \tunsigned long taggerdate;\n \tint generation;\n \tint distance;\n+\tint from_tag;\n } rev_name;\n \n static long cutoff = LONG_MAX;\n@@ -24,16 +25,43 @@ static int is_better_name(struct rev_name *name,\n \t\t\t const char *tip_name,\n \t\t\t unsigned long taggerdate,\n \t\t\t int generation,\n-\t\t\t int distance)\n+\t\t\t int distance,\n+\t\t\t int from_tag)\n {\n-\treturn (name->taggerdate > taggerdate ||\n-\t\t(name->taggerdate == taggerdate &&\n-\t\t name->distance > distance));\n+\t/*\n+\t * When comparing names based on tags, prefer names\n+\t * based on the older tag, even if it is farther away.\n+\t */\n+\tif (from_tag && name->from_tag)\n+\t\treturn (name->taggerdate > taggerdate ||\n+\t\t\t(name->taggerdate == taggerdate &&\n+\t\t\t name->distance > distance));\n+\n+\t/*\n+\t * We know that at least one of them is a non-tag at this point.\n+\t * favor a tag over a non-tag.\n+\t */\n+\tif (name->from_tag != from_tag)\n+\t\treturn from_tag;\n+\n+\t/*\n+\t * We are now looking at two non-tags. Tiebreak to favor\n+\t * shorter hops.\n+\t */\n+\tif (name->distance != distance)\n+\t\treturn name->distance > distance;\n+\n+\t/* ... or tiebreak to favor older date */\n+\tif (name->taggerdate != taggerdate)\n+\t\treturn name->taggerdate > taggerdate;\n+\n+\t/* keep the current one if we cannot decide */\n+\treturn 0;\n }\n \n static void name_rev(struct commit *commit,\n \t\tconst char *tip_name, unsigned long taggerdate,\n-\t\tint generation, int distance,\n+\t\tint generation, int distance, int from_tag,\n \t\tint deref)\n {\n \tstruct rev_name *name = (struct rev_name *)commit->util;\n@@ -57,12 +85,13 @@ static void name_rev(struct commit *commit,\n \t\tcommit->util = name;\n \t\tgoto copy_data;\n \t} else if (is_better_name(name, tip_name, taggerdate,\n-\t\t\t\t generation, distance)) {\n+\t\t\t\t generation, distance, from_tag)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n \t\tname->taggerdate = taggerdate;\n \t\tname->generation = generation;\n \t\tname->distance = distance;\n+\t\tname->from_tag = from_tag;\n \t} else\n \t\treturn;\n \n@@ -82,10 +111,12 @@ static void name_rev(struct commit *commit,\n \t\t\t\t\t\t parent_number);\n \n \t\t\tname_rev(parents->item, new_name, taggerdate, 0,\n-\t\t\t\tdistance + MERGE_TRAVERSAL_WEIGHT, 0);\n+\t\t\t\t distance + MERGE_TRAVERSAL_WEIGHT,\n+\t\t\t\t from_tag, 0);\n \t\t} else {\n \t\t\tname_rev(parents->item, tip_name, taggerdate,\n-\t\t\t\tgeneration + 1, distance + 1, 0);\n+\t\t\t\t generation + 1, distance + 1,\n+\t\t\t\t from_tag, 0);\n \t\t}\n \t}\n }\n@@ -216,9 +247,13 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t}\n \tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n+\t\tint from_tag = starts_with(path, \"refs/tags/\");\n \n+\t\tif (taggerdate == ULONG_MAX)\n+\t\t\ttaggerdate = ((struct commit *)o)->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n-\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);\n+\t\tname_rev(commit, xstrdup(path), taggerdate, 0, 0,\n+\t\t\t from_tag, deref);\n \t}\n \treturn 0;\n }\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex f577990716..4afd8135cd 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -399,7 +399,7 @@ cat > expect <<\\EOF\n | |\n | | Merge branch 'side'\n | |\n-| * commit side\n+| * commit tags/side-2\n | | Author: A U Thor \n | |\n | | side-2\n-- \n2.12.2.739.gfc3eb97820\n\n"},{"id":"315924","messageId":"xmqqtw69z8vz.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"21cf9d6f55d17463ab6eccdd78d57cf4a1b8e9e1.1490967948.git.git@grubix.eu","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-31T16:52:16Z","receivedAt":"2017-03-31T16:52:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, `git describe --contains --debug` does not create any debug\n> output because it does not pass the flag down to `git name-rev`, which\n> does not know that flag.\n>\n> Teach the latter that flag, so that the former can pass it down (in\n> the following commit).\n>\n> The output is patterned after that of `git describe --debug`, with the\n> following differences:\n>\n> describe loops over all args to describe, then over all possible\n> descriptions; name-rev does it the other way round. Therefore, we need\n> to amend each possible description by the arg that it is for (and we\n> leave out the \"searching to describe\" header).\n>\n> The date cut-off for name-rev kicks in way more often than the candidate\n> number cut-off of describe, so we do not clutter the output with the\n> cut-off.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n\n> static void name_rev(struct commit *commit,\n> \t\tconst char *tip_name, unsigned long taggerdate,\n> \t\tint generation, int distance, int from_tag,\n> -\t\tint deref)\n> +\t\tint deref, struct name_ref_data *data)\n> {\n> \tstruct rev_name *name = (struct rev_name *)commit->util;\n> \tstruct commit_list *parents;\n> @@ -75,6 +88,7 @@ static void name_rev(struct commit *commit,\n> \n> \tif (deref) {\n> \t\ttip_name = xstrfmt(\"%s^0\", tip_name);\n> +\t\tfrom_tag += 1;\n\nWhy this change? I didn't see it explained in the proposed log\nmessage. \"deref\" is true only when our immediate caller is the one\nthat inspected the object at the tip and found it to be a tag object\n(i.e. not a lightweight tag or a branch). from_tag is about \"is the\ntip within refs/tags/ hierarchy? Yes/No?\" and such a caller will\nset it appropriately when calling us. This function just passes it\ndown when it recursively calls itself. \n\nWe shouldn't be mucking with that value ourselves here, should we?\n\nThe only case that this change may make a difference I can think of\nis when you have a tag object pointed at from outside refs/tags\n(e.g. refs/heads/foo is a tag object); if you are trying to change\nthe definition of \"from_tag\" from the current \"Is the tip inside\nrefs/tags/?\" to \"Is the tip either inside refs/tags/ or is it a tag\nobject anywhere?\", that may be a good change (I didn't think things\nthrough, though), but that shouldn't be hidden inside a commit that\nclaims to only add support for debugging.\n\nWhat problem are you solving? \n\n> @@ -236,7 +273,6 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n> \t}\n> \n> \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n> -\n> \twhile (o && o->type == OBJ_TAG) {\n> \t\tstruct tag *t = (struct tag *) o;\n> \t\tif (!t->tagged)\n\nThis is a patch noise we can do without.\n\nThanks.\n\n"},{"id":"315925","messageId":"xmqqpogxz8pv.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"xmqqtw69z8vz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-31T16:55:56Z","receivedAt":"2017-03-31T16:56:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> The only case that this change may make a difference I can think of\n> is when you have a tag object pointed at from outside refs/tags\n> (e.g. refs/heads/foo is a tag object); if you are trying to change\n> the definition of \"from_tag\" from the current \"Is the tip inside\n> refs/tags/?\" to \"Is the tip either inside refs/tags/ or is it a tag\n> object anywhere?\", that may be a good change (I didn't think things\n> through, though), but that shouldn't be hidden inside a commit that\n> claims to only add support for debugging.\n\nAnd if that \"a tag object outside refs/tags/\" is what you are\nsolving, I think a better place to do it is in name_ref(). \n\nInstead of saying \"from_tag is true iff it starts with refs/tags/\",\nyou'd say \"... or deref is set to true, because we know that the\noriginal was a tag object in that case\".\n\n\n> What problem are you solving? \n>\n>> @@ -236,7 +273,6 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n>> \t}\n>> \n>> \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n>> -\n>> \twhile (o && o->type == OBJ_TAG) {\n>> \t\tstruct tag *t = (struct tag *) o;\n>> \t\tif (!t->tagged)\n>\n> This is a patch noise we can do without.\n>\n> Thanks.\n"},{"id":"315939","messageId":"14D0E7F4-0DC7-42ED-8DEE-B0338ECBA80D@grubix.eu","threadId":"45385","inReplyTo":"xmqqtw69z8vz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-03-31T18:02:57Z","receivedAt":"2017-03-31T18:03:20Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Am 31. März 2017 18:52:16 MESZ schrieb Junio C Hamano :\n>Michael J Gruber writes:\n>\n>> Currently, `git describe --contains --debug` does not create any\n>debug\n>> output because it does not pass the flag down to `git name-rev`,\n>which\n>> does not know that flag.\n>>\n>> Teach the latter that flag, so that the former can pass it down (in\n>> the following commit).\n>>\n>> The output is patterned after that of `git describe --debug`, with\n>the\n>> following differences:\n>>\n>> describe loops over all args to describe, then over all possible\n>> descriptions; name-rev does it the other way round. Therefore, we\n>need\n>> to amend each possible description by the arg that it is for (and we\n>> leave out the \"searching to describe\" header).\n>>\n>> The date cut-off for name-rev kicks in way more often than the\n>candidate\n>> number cut-off of describe, so we do not clutter the output with the\n>> cut-off.\n>>\n>> Signed-off-by: Michael J Gruber \n>> ---\n>\n>> static void name_rev(struct commit *commit,\n>> \t\tconst char *tip_name, unsigned long taggerdate,\n>> \t\tint generation, int distance, int from_tag,\n>> -\t\tint deref)\n>> +\t\tint deref, struct name_ref_data *data)\n>> {\n>> \tstruct rev_name *name = (struct rev_name *)commit->util;\n>> \tstruct commit_list *parents;\n>> @@ -75,6 +88,7 @@ static void name_rev(struct commit *commit,\n>> \n>> \tif (deref) {\n>> \t\ttip_name = xstrfmt(\"%s^0\", tip_name);\n>> +\t\tfrom_tag += 1;\n>\n>Why this change? I didn't see it explained in the proposed log\n>message. \"deref\" is true only when our immediate caller is the one\n>that inspected the object at the tip and found it to be a tag object\n>(i.e. not a lightweight tag or a branch). from_tag is about \"is the\n>tip within refs/tags/ hierarchy? Yes/No?\" and such a caller will\n>set it appropriately when calling us. This function just passes it\n>down when it recursively calls itself. \n>\n>We shouldn't be mucking with that value ourselves here, should we?\n>\n>The only case that this change may make a difference I can think of\n>is when you have a tag object pointed at from outside refs/tags\n>(e.g. refs/heads/foo is a tag object); if you are trying to change\n>the definition of \"from_tag\" from the current \"Is the tip inside\n>refs/tags/?\" to \"Is the tip either inside refs/tags/ or is it a tag\n>object anywhere?\", that may be a good change (I didn't think things\n>through, though), but that shouldn't be hidden inside a commit that\n>claims to only add support for debugging.\n>\n>What problem are you solving? \n\nSorry, I forgot about that change and failed to mention it.\n\nIt 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.\n\n>> @@ -236,7 +273,6 @@ static int name_ref(const char *path, const\n>struct object_id *oid, int flags, vo\n>> \t}\n>> \n>> \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n>> -\n>> \twhile (o && o->type == OBJ_TAG) {\n>> \t\tstruct tag *t = (struct tag *) o;\n>> \t\tif (!t->tagged)\n>\n>This is a patch noise we can do without.\n>\n>Thanks.\n\n"},{"id":"315940","messageId":"xmqqd1cxz5gb.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"14D0E7F4-0DC7-42ED-8DEE-B0338ECBA80D@grubix.eu","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-31T18:06:28Z","receivedAt":"2017-03-31T18:06:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n>>The only case that this change may make a difference I can think of\n>>is when you have a tag object pointed at from outside refs/tags\n>>(e.g. refs/heads/foo is a tag object); if you are trying to change\n>>the definition of \"from_tag\" from the current \"Is the tip inside\n>>refs/tags/?\" to \"Is the tip either inside refs/tags/ or is it a tag\n>>object anywhere?\", that may be a good change (I didn't think things\n>>through, though), but that shouldn't be hidden inside a commit that\n>>claims to only add support for debugging.\n>>\n>>What problem are you solving? \n>\n> Sorry, I forgot about that change and failed to mention it.\n>\n> It makes no difference in the non-debug case which cares about the\n> Boolean only. In the debug case, I want to distinguish between\n> annotated and lightweight tags, just like describe --debug does. By\n> adding 1 via deref and passing this down, I know that an annotated tag\n> gets the value 2, a lightweight tag 1 and everything else 0, just like\n> describe --tags.\n\nSo it sounds like you meant to do something else, and the\nimplementation is wrong for that something else (i.e. it wouldn't do\nthe right thing for a tag object outside refs/tags/, with or without\nthe \"--debug\" option passed).\n\n>>> @@ -236,7 +273,6 @@ static int name_ref(const char *path, const\n>>struct object_id *oid, int flags, vo\n>>> \t}\n>>> \n>>> \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n>>> -\n>>> \twhile (o && o->type == OBJ_TAG) {\n>>> \t\tstruct tag *t = (struct tag *) o;\n>>> \t\tif (!t->tagged)\n>>\n>>This is a patch noise we can do without.\n>>\n>>Thanks.\n"},{"id":"315943","messageId":"xmqqzig1xpm6.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"xmqqd1cxz5gb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-31T18:33:53Z","receivedAt":"2017-03-31T18:34:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Michael J Gruber writes:\n>\n>>>The only case that this change may make a difference I can think of\n>>>is when you have a tag object pointed at from outside refs/tags\n>>>(e.g. refs/heads/foo is a tag object); if you are trying to change\n>>>the definition of \"from_tag\" from the current \"Is the tip inside\n>>>refs/tags/?\" to \"Is the tip either inside refs/tags/ or is it a tag\n>>>object anywhere?\", that may be a good change (I didn't think things\n>>>through, though), but that shouldn't be hidden inside a commit that\n>>>claims to only add support for debugging.\n>>>\n>>>What problem are you solving? \n>>\n>> Sorry, I forgot about that change and failed to mention it.\n>>\n>> It makes no difference in the non-debug case which cares about the\n>> Boolean only. In the debug case, I want to distinguish between\n>> annotated and lightweight tags, just like describe --debug does. By\n>> adding 1 via deref and passing this down, I know that an annotated tag\n>> gets the value 2, a lightweight tag 1 and everything else 0, just like\n>> describe --tags.\n>\n> So it sounds like you meant to do something else, and the\n> implementation is wrong for that something else (i.e. it wouldn't do\n> the right thing for a tag object outside refs/tags/, with or without\n> the \"--debug\" option passed).\n\nThe damage seems worse, but I may be misreading the code.\n\nis_better_name() compares name->from_tag and from_tag numerically,\nbecause it was designed to take a boolean view of that variable.\nNow, an artificially bumped 2 gets compared with name->from_tag that\nmay be 1 and gets different priority. That artificially inflated\nvalue may be propagated to name->from_tag when the current tip is\njudged as a better basis for naming the object.\n\nIf this change is only for debugging, perhaps inside if(data->debug)\nyou added, instead of looking at from_tag, you can look at both\nfrom_tag and deref to choose which prio-nmes to show, without\nbutchering the value in from_tag variable to affect the existing\ncode that is exercised with or without --debug?\n"},{"id":"315947","messageId":"xmqqshltxnwt.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"14D0E7F4-0DC7-42ED-8DEE-B0338ECBA80D@grubix.eu","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-31T19:10:42Z","receivedAt":"2017-03-31T19:10:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n>>What problem are you solving? \n>\n> Sorry, I forgot about that change and failed to mention it.\n>\n> It makes no difference in the non-debug case which cares about the\n> Boolean only. In the debug case, I want to distinguish between\n> annotated and lightweight tags, just like describe --debug\n> does. By adding 1 via deref and passing this down, I know that an\n> annotated tag gets the value 2, a lightweight tag 1 and everything\n> else 0, just like describe --tags.\n\nIf you want to only affect debug display, perhaps you should start\nwith a patch like the attached, before adding any debug code as a\npreparatory step. Then add your debugging thing, _WITHOUT_ the\nincrement in name_rev(), that uses from_tag to choose between\nlightweight and annotated as a separate step.\n\nWhen we decide that it would make sense to give precedence to\nannotated ones over lightweight ones in is_better_name(), the\ncomparison can be further tweaked to actually compare values of the\nfrom_tag thing in *name and the current candidate. That would have\nto be a separate step, as it changes the semantics (I suspect it\nwould be a better change but it may not be).\n\nHow does that sound?\n\n-- >8 --\nSubject: name-rev: allow to tell annotated and lightweight tags apart\n\nWe do not use this feature yet, but from_tag that is passed around\nand kept in the rev_name structure now takes three values, instead\nof a boolean \"did this come from refs/tags/ hierarchy?\". A new\nvalue '2' is \"this is an annotated tag that came from refs/tags/\nhierarchy\".\n\n---\n builtin/name-rev.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex bf7ed015ae..fe2d306e7c 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -41,7 +41,7 @@ static int is_better_name(struct rev_name *name,\n \t * We know that at least one of them is a non-tag at this point.\n \t * favor a tag over a non-tag.\n \t */\n-\tif (name->from_tag != from_tag)\n+\tif (!!name->from_tag != !!from_tag)\n \t\treturn from_tag;\n \n \t/*\n@@ -247,8 +247,11 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \t}\n \tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n-\t\tint from_tag = starts_with(path, \"refs/tags/\");\n-\n+\t\tint from_tag;\n+\t\tif (starts_with(path, \"refs/tags/\"))\n+\t\t\tfrom_tag = 1 + deref;\n+\t\telse\n+\t\t\tfrom_tag = 0;\n \t\tif (taggerdate == ULONG_MAX)\n \t\t\ttaggerdate = ((struct commit *)o)->date;\n \t\tpath = name_ref_abbrev(path, can_abbreviate_output);\n"},{"id":"316088","messageId":"c85bc2a1-56d8-8a02-6089-2b8cb3d39e99@grubix.eu","threadId":"45385","inReplyTo":"xmqqzig1xpm6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-04-03T14:46:01Z","receivedAt":"2017-04-03T14:46:15Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 31.03.2017 20:33:\n> Junio C Hamano writes:\n> \n>> Michael J Gruber writes:\n>>\n>>>> The only case that this change may make a difference I can think of\n>>>> is when you have a tag object pointed at from outside refs/tags\n>>>> (e.g. refs/heads/foo is a tag object); if you are trying to change\n>>>> the definition of \"from_tag\" from the current \"Is the tip inside\n>>>> refs/tags/?\" to \"Is the tip either inside refs/tags/ or is it a tag\n>>>> object anywhere?\", that may be a good change (I didn't think things\n>>>> through, though), but that shouldn't be hidden inside a commit that\n>>>> claims to only add support for debugging.\n>>>>\n>>>> What problem are you solving? \n>>>\n>>> Sorry, I forgot about that change and failed to mention it.\n>>>\n>>> It makes no difference in the non-debug case which cares about the\n>>> Boolean only. In the debug case, I want to distinguish between\n>>> annotated and lightweight tags, just like describe --debug does. By\n>>> adding 1 via deref and passing this down, I know that an annotated tag\n>>> gets the value 2, a lightweight tag 1 and everything else 0, just like\n>>> describe --tags.\n>>\n>> So it sounds like you meant to do something else, and the\n>> implementation is wrong for that something else (i.e. it wouldn't do\n>> the right thing for a tag object outside refs/tags/, with or without\n>> the \"--debug\" option passed).\n> \n> The damage seems worse, but I may be misreading the code.\n> \n> is_better_name() compares name->from_tag and from_tag numerically,\n> because it was designed to take a boolean view of that variable.\n> Now, an artificially bumped 2 gets compared with name->from_tag that\n> may be 1 and gets different priority. That artificially inflated\n> value may be propagated to name->from_tag when the current tip is\n> judged as a better basis for naming the object.\n\nNo, I checked not to change the existing behaviour.\n\nIf you look at the comment above that then you see that one of the sides\nof the comparison is a non-tag, so we compare 0 to 1 or 2, the boolean\noutcome being the same.\n\n> If this change is only for debugging, perhaps inside if(data->debug)\n> you added, instead of looking at from_tag, you can look at both\n> from_tag and deref to choose which prio-nmes to show, without\n> butchering the value in from_tag variable to affect the existing\n> code that is exercised with or without --debug?\n\nWhat I did overlook, though, was that name-rev uses the notion \"under\nrefs/tags\" for \"being a tag\".\n\nIn fact, it's puzzling how different describe and name-rev proceed and\nweigh the alternatives. I didn't mean to change that.\n\nIn retrospect, displaying the \"same\" debug information for the two\ncommands doesn't make too much sense as long as they use different\ninformation. name-rev does-not distinguish between tag types, so why\neven display it?\n\nI think I should change 3/4 to display exactly those bits that name-rev\nactually uses for weighing different possible descriptions; they are\ndifferents from the \"describe-bits\". So please withhold 3/4 and 4/4.\n\nMichael\n"},{"id":"316098","messageId":"xmqqa87xs9mn.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"c85bc2a1-56d8-8a02-6089-2b8cb3d39e99@grubix.eu","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-03T17:07:12Z","receivedAt":"2017-04-03T17:07:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> No, I checked not to change the existing behaviour.\n>\n> If you look at the comment above that then you see that one of the sides\n> of the comparison is a non-tag, so we compare 0 to 1 or 2, the boolean\n> outcome being the same.\n\nThat one I understand, but if you compare 1 and 2 (one side being a\nlightweight, the other being an annotated tag), they no longer\ncompare equal, no?\n"},{"id":"320294","messageId":"xmqqfug0t8aa.fsf@gitster.mtv.corp.google.com","threadId":"45385","inReplyTo":"c85bc2a1-56d8-8a02-6089-2b8cb3d39e99@grubix.eu","subject":"Re: [PATCH v3 3/4] name-rev: provide debug output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-20T05:19:09Z","receivedAt":"2017-05-20T05:19:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> I think I should change 3/4 to display exactly those bits that name-rev\n> actually uses for weighing different possible descriptions; they are\n> differents from the \"describe-bits\". So please withhold 3/4 and 4/4.\n\nOK, I think 1&2/4 from this series can progress before that as it is\nan end-user visible improvement. While looking at it, I also found\na variable that recent \"timestamp_t\" series didn't notice and\nupdate, so perhaps 1&2/4 needs to be rebased on a fix for that\nanyway ;-)\n\nThanks. Will hold 3&4/4 for now.\n"}]}