{"thread":{"id":"29706","subject":"[PATCH v2 1/4] strbuf: improve strbuf_get*line documentation","startedAt":"2012-02-23T09:42:20Z","lastAt":"2012-02-23T23:36:34Z","messageCount":8,"participants":["Thomas Rast","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"185252","messageId":"cover.1329988335.git.trast@student.ethz.ch","threadId":"29706","inReplyTo":"7vty2i5ie2.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2 0/4] Making an elephant out of a getline() bug","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-02-23T09:42:20Z","receivedAt":"2012-02-23T09:42:20Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"So you all made very good points, and I don't want to repeat them.\nJunio's analysis\n\n> Perhaps we would want to squash something like this to the test to avoid\n> \"seq\", using J6t's idea.  The issue is that we do not write the end of\n> line for the long boundary (because it is hidden from us), keep reading\n> and rejecting bogus letters, and then the tip is written without\n> terminating the boundary line.  So checking boundary line is not a very\n> useful test, but \"fetch\" and \"list-heads\" are.\n\nis of course also correct for the case where the overlong line is in a\nboundary commit.  (The analysis about the accidentally-leading '-' is\nalso correct and can create a boundary line where there was none, like\nin the perl case.)\n\nThe patches in this round are:\n\n[1] new; following up on Peff's complaint that I should document\n    strbuf_getwholeline_fd, I found that the strbuf_get*line\n    documentation was also lacking.\n\n[2] previously (1/2), now with documentation added\n\n[3] new; Junio corrected me on the ': > file' spelling, which I added\n    because the start of the test used it too.  So let's just fix it\n    outright to match the Git style all over.\n\n[4] previously (2/2), now incorporating a strbuf_release and the\n    suggestions for the tests as sent by Junio.  I upped the %0982 a\n    bit as it felt *too* tailored for the old bug, and this way\n    everyone can see immediately that it will exceed the 1024 char\n    limit (without thinking about the length of a sha and accounting\n    for \\0 and \\n).\n\nThomas Rast (4):\n  strbuf: improve strbuf_get*line documentation\n  bundle: put strbuf_readline_fd in strbuf.c with adjustments\n  t5704: match tests to modern style\n  bundle: use a strbuf to scan the log for boundary commits\n\n Documentation/technical/api-strbuf.txt |   19 +++++++++++--\n bundle.c                               |   36 +++++++-----------------\n strbuf.c                               |   16 +++++++++++\n strbuf.h                               |    1 +\n t/t5704-bundle.sh                      |   47 ++++++++++++++++----------------\n 5 files changed, 67 insertions(+), 52 deletions(-)\n\n-- \n1.7.9.1.430.g4998543\n"},{"id":"185249","messageId":"d5522f8d1154321804cb60fb4f53d538242ead88.1329988335.git.trast@student.ethz.ch","threadId":"29706","inReplyTo":"cover.1329988335.git.trast@student.ethz.ch","subject":"[PATCH v2 1/4] strbuf: improve strbuf_get*line documentation","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-02-23T09:42:21Z","receivedAt":"2012-02-23T09:42:21Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"strbuf_getline() was not documented very clearly, though a reader\nfamiliar with getline() would not have had any questions about it.\nstrbuf_getwholeline() was not documented at all.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n Documentation/technical/api-strbuf.txt |   12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\nindex afe2759..cc6db5e 100644\n--- a/Documentation/technical/api-strbuf.txt\n+++ b/Documentation/technical/api-strbuf.txt\n@@ -255,8 +255,16 @@ same behaviour as well.\n \n `strbuf_getline`::\n \n-\tRead a line from a FILE* pointer. The second argument specifies the line\n-\tterminator character, typically `'\\n'`.\n+\tRead a line from a FILE*. The second argument specifies the\n+\tline terminator character, typically `'\\n'`.  Reading stops\n+\tafter the terminator or at EOF.  The terminator is removed\n+\tfrom the buffer before returning.  Returns 0 unless there was\n+\tnothing left before EOF, in which case it returns `EOF`.\n+\n+`strbuf_getwholeline`::\n+\t\n+\tLike `strbuf_getline`, but keeps the trailing terminator (if\n+\tany) in the buffer.\n \n `stripspace`::\n \n-- \n1.7.9.1.430.g4998543\n"},{"id":"185250","messageId":"b1d225393ad182bf5e77e9ff43e0ad468944c0cf.1329988335.git.trast@student.ethz.ch","threadId":"29706","inReplyTo":"cover.1329988335.git.trast@student.ethz.ch","subject":"[PATCH v2 2/4] bundle: put strbuf_readline_fd in strbuf.c with adjustments","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-02-23T09:42:22Z","receivedAt":"2012-02-23T09:42:22Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The comment even said that it should eventually go there.  While at\nit, match the calling convention and name of the function to the\nstrbuf_get*line family.  So it now is strbuf_getwholeline_fd.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n Documentation/technical/api-strbuf.txt |    7 +++++++\n bundle.c                               |   21 ++-------------------\n strbuf.c                               |   16 ++++++++++++++++\n strbuf.h                               |    1 +\n 4 files changed, 26 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\nindex cc6db5e..827c4bd 100644\n--- a/Documentation/technical/api-strbuf.txt\n+++ b/Documentation/technical/api-strbuf.txt\n@@ -266,6 +266,13 @@ same behaviour as well.\n \tLike `strbuf_getline`, but keeps the trailing terminator (if\n \tany) in the buffer.\n \n+`strbuf_getwholeline_fd`::\n+\n+\tLike `strbuf_getwholeline`, but operates on a file descriptor.\n+\tIt reads one character at a time, so it is very slow.  Do not\n+\tuse it unless you need the correct position in the file\n+\tdescriptor!\n+\n `stripspace`::\n \n \tStrip whitespace from a buffer. The second parameter controls if\ndiff --git a/bundle.c b/bundle.c\nindex b8acf3c..313de42 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -23,23 +23,6 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,\n \tlist->nr++;\n }\n \n-/* Eventually this should go to strbuf.[ch] */\n-static int strbuf_readline_fd(struct strbuf *sb, int fd)\n-{\n-\tstrbuf_reset(sb);\n-\n-\twhile (1) {\n-\t\tchar ch;\n-\t\tssize_t len = xread(fd, &ch, 1);\n-\t\tif (len <= 0)\n-\t\t\treturn len;\n-\t\tstrbuf_addch(sb, ch);\n-\t\tif (ch == '\\n')\n-\t\t\tbreak;\n-\t}\n-\treturn 0;\n-}\n-\n static int parse_bundle_header(int fd, struct bundle_header *header,\n \t\t\t       const char *report_path)\n {\n@@ -47,7 +30,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n \tint status = 0;\n \n \t/* The bundle header begins with the signature */\n-\tif (strbuf_readline_fd(&buf, fd) ||\n+\tif (strbuf_getwholeline_fd(&buf, fd, '\\n') ||\n \t    strcmp(buf.buf, bundle_signature)) {\n \t\tif (report_path)\n \t\t\terror(\"'%s' does not look like a v2 bundle file\",\n@@ -57,7 +40,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n \t}\n \n \t/* The bundle header ends with an empty line */\n-\twhile (!strbuf_readline_fd(&buf, fd) &&\n+\twhile (!strbuf_getwholeline_fd(&buf, fd, '\\n') &&\n \t       buf.len && buf.buf[0] != '\\n') {\n \t\tunsigned char sha1[20];\n \t\tint is_prereq = 0;\ndiff --git a/strbuf.c b/strbuf.c\nindex ff0b96b..5135d59 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -383,6 +383,22 @@ int strbuf_getline(struct strbuf *sb, FILE *fp, int term)\n \treturn 0;\n }\n \n+int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)\n+{\n+\tstrbuf_reset(sb);\n+\n+\twhile (1) {\n+\t\tchar ch;\n+\t\tssize_t len = xread(fd, &ch, 1);\n+\t\tif (len <= 0)\n+\t\t\treturn EOF;\n+\t\tstrbuf_addch(sb, ch);\n+\t\tif (ch == term)\n+\t\t\tbreak;\n+\t}\n+\treturn 0;\n+}\n+\n int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n {\n \tint fd, len;\ndiff --git a/strbuf.h b/strbuf.h\nindex fbf059f..3effaa8 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -116,6 +116,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)\n \n extern int strbuf_getwholeline(struct strbuf *, FILE *, int);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n+extern int strbuf_getwholeline_fd(struct strbuf *, int, int);\n \n extern void stripspace(struct strbuf *buf, int skip_comments);\n extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n-- \n1.7.9.1.430.g4998543\n"},{"id":"185253","messageId":"c6579adc648119fbd1c54e2a1a9a0de86e0e8b57.1329988335.git.trast@student.ethz.ch","threadId":"29706","inReplyTo":"cover.1329988335.git.trast@student.ethz.ch","subject":"[PATCH v2 3/4] t5704: match tests to modern style","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-02-23T09:42:23Z","receivedAt":"2012-02-23T09:42:23Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The test did not adhere to the current style on several counts:\n\n- empty lines around the test blocks, but within the test string\n\n- ': > file' or even just '> file' with an extra space\n\n- inconsistent indentation\n\n- hand-rolled commits instead of using test_commit\n\nFix all of them.  There's a catch to the last point: test_commit\ncreates a tag.  We still change it to test_commit, and explicitly\ndelete the tags, so as to highlight that the test relies on not having\nthem.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n t/t5704-bundle.sh |   33 ++++++++-------------------------\n 1 file changed, 8 insertions(+), 25 deletions(-)\n\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex 4ae127d..f35f559 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -4,59 +4,42 @@ test_description='some bundle related tests'\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-\n-\t: > file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m initial &&\n+\ttest_commit initial &&\n \ttest_tick &&\n \tgit tag -m tag tag &&\n-\t: > file2 &&\n-\tgit add file2 &&\n-\t: > file3 &&\n-\ttest_tick &&\n-\tgit commit -m second &&\n-\tgit add file3 &&\n-\ttest_tick &&\n-\tgit commit -m third\n-\n+\ttest_commit second &&\n+\ttest_commit third &&\n+\tgit tag -d initial &&\n+\tgit tag -d second &&\n+\tgit tag -d third\n '\n \n test_expect_success 'tags can be excluded by rev-list options' '\n-\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-\n '\n \n test_expect_success 'die if bundle file cannot be created' '\n-\n \tmkdir adir &&\n \ttest_must_fail git bundle create adir --all\n-\n '\n \n test_expect_failure 'bundle --stdin' '\n-\n \techo master | git bundle create stdin-bundle.bdl --stdin &&\n \tgit ls-remote stdin-bundle.bdl >output &&\n \tgrep master output\n-\n '\n \n test_expect_failure 'bundle --stdin <rev-list options>' '\n-\n \techo master | git bundle create hybrid-bundle.bdl --stdin tag &&\n \tgit ls-remote hybrid-bundle.bdl >output &&\n \tgrep master output\n-\n '\n \n test_expect_success 'empty bundle file is rejected' '\n-\n-    >empty-bundle && test_must_fail git fetch empty-bundle\n-\n+\t: >empty-bundle &&\n+\ttest_must_fail git fetch empty-bundle\n '\n \n test_done\n-- \n1.7.9.1.430.g4998543\n"},{"id":"185251","messageId":"97490978be559ba10466c90ed499fd8fc64a9d8b.1329988335.git.trast@student.ethz.ch","threadId":"29706","inReplyTo":"cover.1329988335.git.trast@student.ethz.ch","subject":"[PATCH v2 4/4] bundle: use a strbuf to scan the log for boundary commits","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-02-23T09:42:24Z","receivedAt":"2012-02-23T09:42:24Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The first part of the bundle header contains the boundary commits, and\ncould be approximated by\n\n  # v2 git bundle\n  $(git rev-list --pretty=oneline --boundary <ARGS> | grep ^-)\n\ngit-bundle actually spawns exactly this rev-list invocation, and does\nthe grepping internally.\n\nThere was a subtle bug in the latter step: it used fgets() with a\n1024-byte buffer.  If the user has sufficiently long subjects (e.g.,\nby not adhering to the git oneline-subject convention in the first\nplace), the 'oneline' format can easily overflow the buffer.  fgets()\nthen returns only the first part of the line _without a \\n_, and it\nwould be printed as such, turning the next line into trailing garbage.\nOn top of that, fgets() returns the rest of the line in the next\ncall(s).  If one of these remaining parts starts with '-', git-bundle\nwould mistakenly insert it into the boundary list.\n\nFix it by using strbuf_getwholeline() instead, which handles arbitrary\nline lengths correctly.\n\nNote that on the receiving side in parse_bundle_header() we were\nalready using strbuf_getwholeline_fd(), so that part is safe.\n\nThanks to Peff, Johannes Sixt and Junio for suggestions.\n\nReported-by: Jannis Pohlmann <jannis.pohlmann@codethink.co.uk>\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n bundle.c          |   15 ++++++++-------\n t/t5704-bundle.sh |   16 ++++++++++++++++\n 2 files changed, 24 insertions(+), 7 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 313de42..7a760db 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -234,7 +234,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n \tconst char **argv_pack = xmalloc(6 * sizeof(const char *));\n \tint i, ref_count = 0;\n-\tchar buffer[1024];\n+\tstruct strbuf buf = STRBUF_INIT;\n \tstruct rev_info revs;\n \tstruct child_process rls;\n \tFILE *rls_fout;\n@@ -266,20 +266,21 @@ int create_bundle(struct bundle_header *header, const char *path,\n \tif (start_command(&rls))\n \t\treturn -1;\n \trls_fout = xfdopen(rls.out, \"r\");\n-\twhile (fgets(buffer, sizeof(buffer), rls_fout)) {\n+\twhile (strbuf_getwholeline(&buf, rls_fout, '\\n') != EOF) {\n \t\tunsigned char sha1[20];\n-\t\tif (buffer[0] == '-') {\n-\t\t\twrite_or_die(bundle_fd, buffer, strlen(buffer));\n-\t\t\tif (!get_sha1_hex(buffer + 1, sha1)) {\n+\t\tif (buf.len > 0 && buf.buf[0] == '-') {\n+\t\t\twrite_or_die(bundle_fd, buf.buf, buf.len);\n+\t\t\tif (!get_sha1_hex(buf.buf + 1, sha1)) {\n \t\t\t\tstruct object *object = parse_object(sha1);\n \t\t\t\tobject->flags |= UNINTERESTING;\n-\t\t\t\tadd_pending_object(&revs, object, buffer);\n+\t\t\t\tadd_pending_object(&revs, object, buf.buf);\n \t\t\t}\n-\t\t} else if (!get_sha1_hex(buffer, sha1)) {\n+\t\t} else if (!get_sha1_hex(buf.buf, sha1)) {\n \t\t\tstruct object *object = parse_object(sha1);\n \t\t\tobject->flags |= SHOWN;\n \t\t}\n \t}\n+\tstrbuf_release(&buf);\n \tfclose(rls_fout);\n \tif (finish_command(&rls))\n \t\treturn error(\"rev-list died\");\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex f35f559..a51c8b0 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -42,4 +42,20 @@ test_expect_success 'empty bundle file is rejected' '\n \ttest_must_fail git fetch empty-bundle\n '\n \n+# This triggers a bug in older versions where the resulting line (with\n+# --pretty=oneline) was longer than a 1024-char buffer.\n+test_expect_success 'ridiculously long subject in boundary' '\n+\t: >file4 &&\n+\ttest_tick &&\n+\tgit add file4 &&\n+\tprintf \"%01200d\\n\" 0 | git commit -F - &&\n+\ttest_commit fifth &&\n+\tgit bundle create long-subject-bundle.bdl HEAD^..HEAD &&\n+\tgit bundle list-heads long-subject-bundle.bdl >heads &&\n+\ttest -s heads &&\n+\tgit fetch long-subject-bundle.bdl &&\n+\tsed -n \"/^-/{p;q}\" long-subject-bundle.bdl >boundary &&\n+\tgrep \"^-$_x40 \" boundary\n+'\n+\n test_done\n-- \n1.7.9.1.430.g4998543\n"},{"id":"185257","messageId":"20120223100800.GA2912@sigill.intra.peff.net","threadId":"29706","inReplyTo":"d5522f8d1154321804cb60fb4f53d538242ead88.1329988335.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 1/4] strbuf: improve strbuf_get*line documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-23T10:08:00Z","receivedAt":"2012-02-23T10:08:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 23, 2012 at 10:42:21AM +0100, Thomas Rast wrote:\n\n> strbuf_getline() was not documented very clearly, though a reader\n> familiar with getline() would not have had any questions about it.\n> strbuf_getwholeline() was not documented at all.\n\nThanks for improving the existing docs. One suggestion:\n\n>  `strbuf_getline`::\n>  \n> -\tRead a line from a FILE* pointer. The second argument specifies the line\n> -\tterminator character, typically `'\\n'`.\n> +\tRead a line from a FILE*. The second argument specifies the\n> +\tline terminator character, typically `'\\n'`.  Reading stops\n> +\tafter the terminator or at EOF.  The terminator is removed\n> +\tfrom the buffer before returning.  Returns 0 unless there was\n> +\tnothing left before EOF, in which case it returns `EOF`.\n\nThe get*line functions are unlike the rest of the strbuf API in that\nthey overwrite, rather than append to, the strbuf argument. Maybe:\n\n  s/from a FILE\\*/&, overwriting the existing contents of the strbuf/\n\n?\n\n-Peff\n"},{"id":"185288","messageId":"7vobsp4bie.fsf@alter.siamese.dyndns.org","threadId":"29706","inReplyTo":"cover.1329988335.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 0/4] Making an elephant out of a getline() bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-23T19:04:25Z","receivedAt":"2012-02-23T19:04:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@student.ethz.ch> writes:\n\n> So you all made very good points, and I don't want to repeat them.\n\nI really wish you did not make an elephant out of this, or if you must,\nkept the minimum bugfix part that must apply to older codebase and making\nan elephant part clearly separable.\n\nAnyway, I've reordered so that the bugfix part would apply to an older\nmaintenance release, and then the remainder applied on top would merge\ncleanly to the bleeding edge, so there is no need to resend.\n\nThanks for working on this, everybody.\n"},{"id":"185307","messageId":"7vzkc915rx.fsf@alter.siamese.dyndns.org","threadId":"29706","inReplyTo":"c6579adc648119fbd1c54e2a1a9a0de86e0e8b57.1329988335.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 3/4] t5704: match tests to modern style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-23T23:36:34Z","receivedAt":"2012-02-23T23:36:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@student.ethz.ch> writes:\n\n> Fix all of them.  There's a catch to the last point: test_commit\n> creates a tag.  We still change it to test_commit, and explicitly\n> delete the tags, so as to highlight that the test relies on not having\n> them.\n\nI do not have an objection to the use of these three test_commits, and I\ndo not have an objection to delete the extra three tags they create so\nthat the set of refs in the repository matches what the later test\nexpects, either.\n\nBut I found the explanation a bit iffy.\n\n>  test_expect_success 'setup' '\n> +\ttest_commit initial &&\n>  \ttest_tick &&\n>  \tgit tag -m tag tag &&\n> +\ttest_commit second &&\n> +\ttest_commit third &&\n> +\tgit tag -d initial &&\n> +\tgit tag -d second &&\n> +\tgit tag -d third\n>  '\n>  \n>  test_expect_success 'tags can be excluded by rev-list options' '\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>  '\n\nIf you do not delete 'third' tag, which matches the tip of the current\nbranch, the resulting bundle created with \"create --all\" would end up\ncontaining that tag, and you will see it in the ls-remote output.\n\nBut the funny thing is that you can leave initial and second in the\nrepository and the resulting bundle still passes the test.  'initial',\n'second' and 'tag' are excluded.  Exclusion of 'tag' tag (sheesh, it makes\nthis conversation more confusing than necessary) is what this test checks,\nand the date range given to the rev-list ensures that initial and second\nare not included.\n\nBut the tip of the current branch and the lightweight 'third' tag both\npoint at the same commit object, and that, together with the fact that we\nask for '--all', is the reason why it is included in the result, making\nthe test fail.\n\nSo perhaps a better fix may be to do something like the attached on top,\nand rewrite everything after \"Fix all of them\".\n\n\t... Fix all of them.\n\n        Also rename the manually created tag 'tag' that points at a commit\n        that is older than the --since threshold a later test uses, to a\n        more descriptive 'ancienttag', and update the check that reads\n        from the resulting bundle with ls-remote to look for 'ancienttag'.\n        The purpose of this test is to make sure that the tag that\n        predates the date range is not in the resulting bundle, but\n        because these test_commit also will create tags, one of which\n        (namely, 'third') points at a commit that is newer than that\n        threshold, ls-remote will list it in its output. Looking for 'tag'\n        will match refs/tags/third, making the test incorrectly fail.\n\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex a51c8b0..3c436e7 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -6,18 +6,15 @@ 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 -m tag antienttag &&\n \ttest_commit second &&\n-\ttest_commit third &&\n-\tgit tag -d initial &&\n-\tgit tag -d second &&\n-\tgit tag -d third\n+\ttest_commit third\n '\n \n test_expect_success 'tags can be excluded by rev-list options' '\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+\t! grep antienttag output\n '\n \n test_expect_success 'die if bundle file cannot be created' '\n"}]}