{"thread":{"id":"37276","subject":"[PATCH 2/2] bundle: Fix exclusion of annotated tags","startedAt":"2014-08-02T08:39:06Z","lastAt":"2014-08-04T20:57:03Z","messageCount":7,"participants":["Lukas Fleischer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"247165","messageId":"1406968747-16100-1-git-send-email-git@cryptocrack.de","threadId":"37276","inReplyTo":null,"subject":"[PATCH 1/2] t5704: Fix the test that checks for excluded tags","fromName":"Lukas Fleischer","fromEmail":"git@cryptocrack.de","sentAt":"2014-08-02T08:39:06Z","receivedAt":"2014-08-02T08:39:06Z","isPatch":true,"sender":{"key":"git@cryptocrack.de","avatar":null},"body":"In c9a42c4 (bundle: allow rev-list options to exclude annotated tags,\n2009-01-02), we added a test to check whether annotated tags, which fall\noutside the specified date range, are excluded from bundles. However,\nwhen initializing the repository, a command to create a lightweight tag\nwas used. Fix this by replacing `git tag` by `git tag -a`. Furthermore,\nexplicitly mention in the test message that an annotated tag is created\nand also test whether tags within the specified date range are included\nproperly.\n\nNote that this fix reveals that the annotated tag exclusion actually\ndoes not work. Therefore, the test is marked expect-failure for now.\n\nSigned-off-by: Lukas Fleischer <git@cryptocrack.de>\n---\n t/t5704-bundle.sh | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex a45c316..2f063ea 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -6,7 +6,7 @@ test_description='some bundle related tests'\n test_expect_success 'setup' '\n \ttest_commit initial &&\n \ttest_tick &&\n-\tgit tag -m tag tag &&\n+\tgit tag -am tag tag &&\n \ttest_commit second &&\n \ttest_commit third &&\n \tgit tag -d initial &&\n@@ -14,7 +14,10 @@ test_expect_success 'setup' '\n \tgit tag -d third\n '\n \n-test_expect_success 'tags can be excluded by rev-list options' '\n+test_expect_failure 'annotated tags can be excluded by rev-list options' '\n+\tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n+\tgit ls-remote bundle > output &&\n+\tgrep tag output &&\n \tgit bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&\n \tgit ls-remote bundle > output &&\n \t! grep tag output\n-- \n2.0.3\n"},{"id":"247164","messageId":"1406968747-16100-2-git-send-email-git@cryptocrack.de","threadId":"37276","inReplyTo":"1406968747-16100-1-git-send-email-git@cryptocrack.de","subject":"[PATCH 2/2] bundle: Fix exclusion of annotated tags","fromName":"Lukas Fleischer","fromEmail":"git@cryptocrack.de","sentAt":"2014-08-02T08:39:07Z","receivedAt":"2014-08-02T08:39:07Z","isPatch":true,"sender":{"key":"git@cryptocrack.de","avatar":null},"body":"In commit c9a42c4 (bundle: allow rev-list options to exclude annotated\ntags, 2009-01-02), support for excluding annotated tags outside the\nspecified date range was added. However, the wrong order of parameters\nwas chosen when calling memchr(). Fix this by swapping the character to\nsearch for with the maximum length parameter.\n\nSigned-off-by: Lukas Fleischer <git@cryptocrack.de>\n---\n bundle.c          | 4 ++--\n t/t5704-bundle.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 71a21a6..b708906 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -221,8 +221,8 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n \tline = memmem(buf, size, \"\\ntagger \", 8);\n \tif (!line++)\n \t\treturn 1;\n-\tlineend = memchr(line, buf + size - line, '\\n');\n-\tline = memchr(line, lineend ? lineend - line : buf + size - line, '>');\n+\tlineend = memchr(line, '\\n', buf + size - line);\n+\tline = memchr(line, '>', lineend ? lineend - line : buf + size - line);\n \tif (!line++)\n \t\treturn 1;\n \tdate = strtoul(line, NULL, 10);\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex 2f063ea..8a4d299 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -14,7 +14,7 @@ test_expect_success 'setup' '\n \tgit tag -d third\n '\n \n-test_expect_failure 'annotated tags can be excluded by rev-list options' '\n+test_expect_success 'annotated tags can be excluded by rev-list options' '\n \tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n \tgit ls-remote bundle > output &&\n \tgrep tag output &&\n-- \n2.0.3\n"},{"id":"247243","messageId":"xmqqzjfkqap5.fsf@gitster.dls.corp.google.com","threadId":"37276","inReplyTo":"1406968747-16100-1-git-send-email-git@cryptocrack.de","subject":"Re: [PATCH 1/2] t5704: Fix the test that checks for excluded tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-04T20:08:54Z","receivedAt":"2014-08-04T20:08:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <git@cryptocrack.de> writes:\n\n> In c9a42c4 (bundle: allow rev-list options to exclude annotated tags,\n> 2009-01-02), we added a test to check whether annotated tags, which fall\n> outside the specified date range, are excluded from bundles. However,\n> when initializing the repository, a command to create a lightweight tag\n> was used. Fix this by replacing `git tag` by `git tag -a`. Furthermore,\n> explicitly mention in the test message that an annotated tag is created\n> and also test whether tags within the specified date range are included\n> properly.\n>\n> Note that this fix reveals that the annotated tag exclusion actually\n> does not work. Therefore, the test is marked expect-failure for now.\n>\n> Signed-off-by: Lukas Fleischer <git@cryptocrack.de>\n> ---\n>  t/t5704-bundle.sh | 7 +++++--\n>  1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\n> index a45c316..2f063ea 100755\n> --- a/t/t5704-bundle.sh\n> +++ b/t/t5704-bundle.sh\n> @@ -6,7 +6,7 @@ test_description='some bundle related tests'\n>  test_expect_success 'setup' '\n>  \ttest_commit initial &&\n>  \ttest_tick &&\n> -\tgit tag -m tag tag &&\n> +\tgit tag -am tag tag &&\n\nI'd prefer to see this spelled as \"-a -m tag\", but anyway,\nthis suggests to me that a request to create a light-weight tag\nshould be made to error out when -m is given, or automatically\npromote itself to create an annotated tag, perhaps?  That is in line\nwith what happens when you do \"git tag -F <file> tagname\".\n\nOh, wait.\n\n\t$ git tag -d foo\n        $ git rev-parse refs/tags/foo --\n        fatal: bad revision 'refs/tags/foo'\n        $ git tag -m msg foo\n        $ git cat-file -t refs/tags/foo\n        tag\n        $ git cat-file tag refs/tags/foo\n        object d84843c...\n        type commit\n        tag foo\n        tagger Junio ....\n\n\tmsg\n\t$ git version\n        git version 2.1.0-rc0-247-g66c8a75\n\nThe output from \"git blame -L'/^int cmd_tag/,/^}/' builtin/tag.c\"\nseems to indicate that we automatically turned annotate on when a\nmessage is given via -m or -F since the very first version of \"git\ntag\" that was re-implemented in C, i.e. 62e09ce9 (Make git tag a\nbuiltin., 2007-07-20).\n\nYour analysis starts to sound fishy.  What version of Git are you\ntalking about?\n\n>  \ttest_commit second &&\n>  \ttest_commit third &&\n>  \tgit tag -d initial &&\n> @@ -14,7 +14,10 @@ test_expect_success 'setup' '\n>  \tgit tag -d third\n>  '\n>  \n> -test_expect_success 'tags can be excluded by rev-list options' '\n> +test_expect_failure 'annotated tags can be excluded by rev-list options' '\n> +\tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n> +\tgit ls-remote bundle > output &&\n> +\tgrep tag output &&\n>  \tgit bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&\n>  \tgit ls-remote bundle > output &&\n>  \t! grep tag output\n"},{"id":"247244","messageId":"xmqqvbq8qama.fsf@gitster.dls.corp.google.com","threadId":"37276","inReplyTo":"1406968747-16100-2-git-send-email-git@cryptocrack.de","subject":"Re: [PATCH 2/2] bundle: Fix exclusion of annotated tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-04T20:10:37Z","receivedAt":"2014-08-04T20:10:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <git@cryptocrack.de> writes:\n\n> In commit c9a42c4 (bundle: allow rev-list options to exclude annotated\n> tags, 2009-01-02), support for excluding annotated tags outside the\n> specified date range was added. However, the wrong order of parameters\n> was chosen when calling memchr(). Fix this by swapping the character to\n> search for with the maximum length parameter.\n>\n> Signed-off-by: Lukas Fleischer <git@cryptocrack.de>\n> ---\n>  bundle.c          | 4 ++--\n>  t/t5704-bundle.sh | 2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/bundle.c b/bundle.c\n> index 71a21a6..b708906 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -221,8 +221,8 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n>  \tline = memmem(buf, size, \"\\ntagger \", 8);\n>  \tif (!line++)\n>  \t\treturn 1;\n> -\tlineend = memchr(line, buf + size - line, '\\n');\n> -\tline = memchr(line, lineend ? lineend - line : buf + size - line, '>');\n> +\tlineend = memchr(line, '\\n', buf + size - line);\n> +\tline = memchr(line, '>', lineend ? lineend - line : buf + size - line);\n\nGood spotting; thanks.\n\n>  \tif (!line++)\n>  \t\treturn 1;\n>  \tdate = strtoul(line, NULL, 10);\n> diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\n> index 2f063ea..8a4d299 100755\n> --- a/t/t5704-bundle.sh\n> +++ b/t/t5704-bundle.sh\n> @@ -14,7 +14,7 @@ test_expect_success 'setup' '\n>  \tgit tag -d third\n>  '\n>  \n> -test_expect_failure 'annotated tags can be excluded by rev-list options' '\n> +test_expect_success 'annotated tags can be excluded by rev-list options' '\n>  \tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n>  \tgit ls-remote bundle > output &&\n>  \tgrep tag output &&\n"},{"id":"247246","messageId":"xmqqmwbkq9sp.fsf@gitster.dls.corp.google.com","threadId":"37276","inReplyTo":"xmqqzjfkqap5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] t5704: Fix the test that checks for excluded tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-04T20:28:22Z","receivedAt":"2014-08-04T20:28:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\n>> index a45c316..2f063ea 100755\n>> --- a/t/t5704-bundle.sh\n>> +++ b/t/t5704-bundle.sh\n>> @@ -6,7 +6,7 @@ test_description='some bundle related tests'\n>>  test_expect_success 'setup' '\n>>  \ttest_commit initial &&\n>>  \ttest_tick &&\n>> -\tgit tag -m tag tag &&\n>> +\tgit tag -am tag tag &&\n> ...\n> Oh, wait.\n\nIn any case, the fix in 2/2 is real, and applying both and then\nreverting the above hunk passes the test.  Also, applying both,\nreverting the above hunk *and* reverting the fix to bundle.c of\ncourse makes the rest fail.\n\nSo I would be tempted to squash these two patches into one using the\nlog message from the latter one, while excluding the change in the\nabove hunk.\n\nThanks.\n\n>> @@ -14,7 +14,10 @@ test_expect_success 'setup' '\n>>  \tgit tag -d third\n>>  '\n>>  \n>> -test_expect_success 'tags can be excluded by rev-list options' '\n>> +test_expect_failure 'annotated tags can be excluded by rev-list options' '\n>> +\tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n>> +\tgit ls-remote bundle > output &&\n>> +\tgrep tag output &&\n>>  \tgit bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&\n>>  \tgit ls-remote bundle > output &&\n>>  \t! grep tag output\n"},{"id":"247253","messageId":"1407185823-9950-1-git-send-email-git@cryptocrack.de","threadId":"37276","inReplyTo":"1406968747-16100-1-git-send-email-git@cryptocrack.de","subject":"[PATCH 1/2] t5704: Complement annotated tag exclusion test","fromName":"Lukas Fleischer","fromEmail":"git@cryptocrack.de","sentAt":"2014-08-04T20:57:02Z","receivedAt":"2014-08-04T20:57:02Z","isPatch":true,"sender":{"key":"git@cryptocrack.de","avatar":null},"body":"In c9a42c4 (bundle: allow rev-list options to exclude annotated tags,\n2009-01-02), we added a test to check whether annotated tags, which fall\noutside the specified date range, are excluded from bundles. Complement\nthis test by also checking whether tags inside the date range are\nincluded. Since this addition reveals that the annotated tag exclusion\nis flawed, mark the test expect-failure for now.\n\nSigned-off-by: Lukas Fleischer <git@cryptocrack.de>\n---\nI decided that it is still worthwhile to have this in a separate patch.\nFeel free to squash 1/2 and 2/2 or let me know that they should be\nmerged if you prefer that.\n\n t/t5704-bundle.sh | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex a45c316..2d53388 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -14,7 +14,10 @@ test_expect_success 'setup' '\n \tgit tag -d third\n '\n \n-test_expect_success 'tags can be excluded by rev-list options' '\n+test_expect_failure 'tags can be excluded by rev-list options' '\n+\tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n+\tgit ls-remote bundle > output &&\n+\tgrep tag output &&\n \tgit bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&\n \tgit ls-remote bundle > output &&\n \t! grep tag output\n-- \n2.0.4\n"},{"id":"247252","messageId":"1407185823-9950-2-git-send-email-git@cryptocrack.de","threadId":"37276","inReplyTo":"1407185823-9950-1-git-send-email-git@cryptocrack.de","subject":"[PATCH 2/2] bundle: Fix exclusion of annotated tags","fromName":"Lukas Fleischer","fromEmail":"git@cryptocrack.de","sentAt":"2014-08-04T20:57:03Z","receivedAt":"2014-08-04T20:57:03Z","isPatch":true,"sender":{"key":"git@cryptocrack.de","avatar":null},"body":"In commit c9a42c4 (bundle: allow rev-list options to exclude annotated\ntags, 2009-01-02), support for excluding annotated tags outside the\nspecified date range was added. However, the wrong order of parameters\nwas chosen when calling memchr(). Fix this by swapping the character to\nsearch for with the maximum length parameter.\n\nSigned-off-by: Lukas Fleischer <git@cryptocrack.de>\n---\n bundle.c          | 4 ++--\n t/t5704-bundle.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 71a21a6..b708906 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -221,8 +221,8 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n \tline = memmem(buf, size, \"\\ntagger \", 8);\n \tif (!line++)\n \t\treturn 1;\n-\tlineend = memchr(line, buf + size - line, '\\n');\n-\tline = memchr(line, lineend ? lineend - line : buf + size - line, '>');\n+\tlineend = memchr(line, '\\n', buf + size - line);\n+\tline = memchr(line, '>', lineend ? lineend - line : buf + size - line);\n \tif (!line++)\n \t\treturn 1;\n \tdate = strtoul(line, NULL, 10);\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex 2d53388..a828c71 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -14,7 +14,7 @@ test_expect_success 'setup' '\n \tgit tag -d third\n '\n \n-test_expect_failure 'tags can be excluded by rev-list options' '\n+test_expect_success 'tags can be excluded by rev-list options' '\n \tgit bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&\n \tgit ls-remote bundle > output &&\n \tgrep tag output &&\n-- \n2.0.4\n"}]}