{"thread":{"id":"46234","subject":"[PATCH] name-rev: Fix tag lookup on repository with mixed types of tags","startedAt":"2017-06-22T19:52:55Z","lastAt":"2017-06-22T20:38:41Z","messageCount":2,"participants":["orgads@gmail.com","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"322977","messageId":"20170622195245.11252-1-orgads@gmail.com","threadId":"46234","inReplyTo":null,"subject":"[PATCH] name-rev: Fix tag lookup on repository with mixed types of tags","fromName":"","fromEmail":"orgads@gmail.com","sentAt":"2017-06-22T19:52:45Z","receivedAt":"2017-06-22T19:52:55Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"From: Orgad Shaneh <orgads@gmail.com>\n\nCommit 7550424804 (name-rev: include taggerdate in considering the best\nname) introduced a bug in name-rev.\n\nIf a repository has both annotated and non-annotated tags, annotated\ntag will always win, even if it was created decades after the commit.\n\nConsider a repository that always used non-annotated tags, and at some\npoint started using annotated tags - name-rev --tags will return the\nfirst annotated tags for all the old commits (in our repository it is\nfollowed by ~5067 for one commit, or by ~120^2~21^2~88^2~87 for\nanother...). This is obviously not what the user expects.\n\nThe taggerdate should only be matched if *both tags* have it.\n---\n builtin/name-rev.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 92a5d8a5d2..8f77023482 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -46,11 +46,13 @@ static void name_rev(struct commit *commit,\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((taggerdate == ULONG_MAX || name->taggerdate == taggerdate) &&\n \t\t\t name->distance > distance)) {\n copy_data:\n \t\tname->tip_name = tip_name;\n-\t\tname->taggerdate = taggerdate;\n+\t\tif (taggerdate != ULONG_MAX) {\n+\t\t\tname->taggerdate = taggerdate;\n+\t\t}\n \t\tname->generation = generation;\n \t\tname->distance = distance;\n \t} else\n-- \n2.13.1.windows.1.1.ga36e14b3aa\n\n"},{"id":"322991","messageId":"CAGZ79kYFcZ_wMQ5grH-keFpnf=V7Y=3KkAJEH3AuoZYMDvqTTQ@mail.gmail.com","threadId":"46234","inReplyTo":"20170622195245.11252-1-orgads@gmail.com","subject":"Re: [PATCH] name-rev: Fix tag lookup on repository with mixed types of tags","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-06-22T20:38:30Z","receivedAt":"2017-06-22T20:38:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jun 22, 2017 at 12:52 PM,  <orgads@gmail.com> wrote:\n> From: Orgad Shaneh <orgads@gmail.com>\n>\n> Commit 7550424804 (name-rev: include taggerdate in considering the best\n> name) introduced a bug in name-rev.\n>\n> If a repository has both annotated and non-annotated tags, annotated\n> tag will always win, even if it was created decades after the commit.\n>\n> Consider a repository that always used non-annotated tags, and at some\n> point started using annotated tags - name-rev --tags will return the\n> first annotated tags for all the old commits (in our repository it is\n> followed by ~5067 for one commit, or by ~120^2~21^2~88^2~87 for\n> another...). This is obviously not what the user expects.\n>\n> The taggerdate should only be matched if *both tags* have it.\n\nPlease sign off the patch. See https://developercertificate.org/\nor Documentation/SubmittingPatches\n\n> ---\n>  builtin/name-rev.c | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n> index 92a5d8a5d2..8f77023482 100644\n> --- a/builtin/name-rev.c\n> +++ b/builtin/name-rev.c\n> @@ -46,11 +46,13 @@ static void name_rev(struct commit *commit,\n>                 commit->util = name;\n>                 goto copy_data;\n>         } else if (name->taggerdate > taggerdate ||\n> -                       (name->taggerdate == taggerdate &&\n> +                       ((taggerdate == ULONG_MAX || name->taggerdate == taggerdate) &&\n>                          name->distance > distance)) {\n>  copy_data:\n>                 name->tip_name = tip_name;\n> -               name->taggerdate = taggerdate;\n> +               if (taggerdate != ULONG_MAX) {\n> +                       name->taggerdate = taggerdate;\n> +               }\n\nstyle: you may omit the braces for a single statement\nafter if.\n\n>                 name->generation = generation;\n>                 name->distance = distance;\n>         } else\n> --\n> 2.13.1.windows.1.1.ga36e14b3aa\n>\n"}]}