[PATCH 1/2] t5704: Fix the test that checks for excluded tags

[PATCH 1/2] t5704: Fix the test that checks for excluded tags

[Lukas Fleischer]

In c9a42c4 (bundle: allow rev-list options to exclude annotated tags,
2009-01-02), we added a test to check whether annotated tags, which fall
outside the specified date range, are excluded from bundles. However,
when initializing the repository, a command to create a lightweight tag
was used. Fix this by replacing `git tag` by `git tag -a`. Furthermore,
explicitly mention in the test message that an annotated tag is created
and also test whether tags within the specified date range are included
properly.

Note that this fix reveals that the annotated tag exclusion actually
does not work. Therefore, the test is marked expect-failure for now.

Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
---
 t/t5704-bundle.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index a45c316..2f063ea 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -6,7 +6,7 @@ test_description='some bundle related tests'
 test_expect_success 'setup' '
 	test_commit initial &&
 	test_tick &&
-	git tag -m tag tag &&
+	git tag -am tag tag &&
 	test_commit second &&
 	test_commit third &&
 	git tag -d initial &&
@@ -14,7 +14,10 @@ test_expect_success 'setup' '
 	git tag -d third
 '
 
-test_expect_success 'tags can be excluded by rev-list options' '
+test_expect_failure 'annotated tags can be excluded by rev-list options' '
+	git bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&
+	git ls-remote bundle > output &&
+	grep tag output &&
 	git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
 	git ls-remote bundle > output &&
 	! grep tag output
-- 
2.0.3

[PATCH 2/2] bundle: Fix exclusion of annotated tags

[Lukas Fleischer]

In commit c9a42c4 (bundle: allow rev-list options to exclude annotated
tags, 2009-01-02), support for excluding annotated tags outside the
specified date range was added. However, the wrong order of parameters
was chosen when calling memchr(). Fix this by swapping the character to
search for with the maximum length parameter.

Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
---
 bundle.c          | 4 ++--
 t/t5704-bundle.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bundle.c b/bundle.c
index 71a21a6..b708906 100644
--- a/bundle.c
+++ b/bundle.c
@@ -221,8 +221,8 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
 	line = memmem(buf, size, "\ntagger ", 8);
 	if (!line++)
 		return 1;
-	lineend = memchr(line, buf + size - line, '\n');
-	line = memchr(line, lineend ? lineend - line : buf + size - line, '>');
+	lineend = memchr(line, '\n', buf + size - line);
+	line = memchr(line, '>', lineend ? lineend - line : buf + size - line);
 	if (!line++)
 		return 1;
 	date = strtoul(line, NULL, 10);
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 2f063ea..8a4d299 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -14,7 +14,7 @@ test_expect_success 'setup' '
 	git tag -d third
 '
 
-test_expect_failure 'annotated tags can be excluded by rev-list options' '
+test_expect_success 'annotated tags can be excluded by rev-list options' '
 	git bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&
 	git ls-remote bundle > output &&
 	grep tag output &&
-- 
2.0.3

Re: [PATCH 1/2] t5704: Fix the test that checks for excluded tags

[Junio C Hamano]

Lukas Fleischer <git@cryptocrack.de> writes:

> In c9a42c4 (bundle: allow rev-list options to exclude annotated tags,
> 2009-01-02), we added a test to check whether annotated tags, which fall
> outside the specified date range, are excluded from bundles. However,
> when initializing the repository, a command to create a lightweight tag
> was used. Fix this by replacing `git tag` by `git tag -a`. Furthermore,
> explicitly mention in the test message that an annotated tag is created
> and also test whether tags within the specified date range are included
> properly.
>
> Note that this fix reveals that the annotated tag exclusion actually
> does not work. Therefore, the test is marked expect-failure for now.
>
> Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
> ---
>  t/t5704-bundle.sh | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
> index a45c316..2f063ea 100755
> --- a/t/t5704-bundle.sh
> +++ b/t/t5704-bundle.sh
> @@ -6,7 +6,7 @@ test_description='some bundle related tests'
>  test_expect_success 'setup' '
>  	test_commit initial &&
>  	test_tick &&
> -	git tag -m tag tag &&
> +	git tag -am tag tag &&

I'd prefer to see this spelled as "-a -m tag", but anyway,
this suggests to me that a request to create a light-weight tag
should be made to error out when -m is given, or automatically
promote itself to create an annotated tag, perhaps?  That is in line
with what happens when you do "git tag -F <file> tagname".

Oh, wait.

	$ git tag -d foo
        $ git rev-parse refs/tags/foo --
        fatal: bad revision 'refs/tags/foo'
        $ git tag -m msg foo
        $ git cat-file -t refs/tags/foo
        tag
        $ git cat-file tag refs/tags/foo
        object d84843c...
        type commit
        tag foo
        tagger Junio ....

	msg
	$ git version
        git version 2.1.0-rc0-247-g66c8a75

The output from "git blame -L'/^int cmd_tag/,/^}/' builtin/tag.c"
seems to indicate that we automatically turned annotate on when a
message is given via -m or -F since the very first version of "git
tag" that was re-implemented in C, i.e. 62e09ce9 (Make git tag a
builtin., 2007-07-20).

Your analysis starts to sound fishy.  What version of Git are you
talking about?

>  	test_commit second &&
>  	test_commit third &&
>  	git tag -d initial &&
> @@ -14,7 +14,10 @@ test_expect_success 'setup' '
>  	git tag -d third
>  '
>  
> -test_expect_success 'tags can be excluded by rev-list options' '
> +test_expect_failure 'annotated tags can be excluded by rev-list options' '
> +	git bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&
> +	git ls-remote bundle > output &&
> +	grep tag output &&
>  	git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
>  	git ls-remote bundle > output &&
>  	! grep tag output

Re: [PATCH 2/2] bundle: Fix exclusion of annotated tags

[Junio C Hamano]

Lukas Fleischer <git@cryptocrack.de> writes:

> In commit c9a42c4 (bundle: allow rev-list options to exclude annotated
> tags, 2009-01-02), support for excluding annotated tags outside the
> specified date range was added. However, the wrong order of parameters
> was chosen when calling memchr(). Fix this by swapping the character to
> search for with the maximum length parameter.
>
> Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
> ---
>  bundle.c          | 4 ++--
>  t/t5704-bundle.sh | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bundle.c b/bundle.c
> index 71a21a6..b708906 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -221,8 +221,8 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
>  	line = memmem(buf, size, "\ntagger ", 8);
>  	if (!line++)
>  		return 1;
> -	lineend = memchr(line, buf + size - line, '\n');
> -	line = memchr(line, lineend ? lineend - line : buf + size - line, '>');
> +	lineend = memchr(line, '\n', buf + size - line);
> +	line = memchr(line, '>', lineend ? lineend - line : buf + size - line);

Good spotting; thanks.

>  	if (!line++)
>  		return 1;
>  	date = strtoul(line, NULL, 10);
> diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
> index 2f063ea..8a4d299 100755
> --- a/t/t5704-bundle.sh
> +++ b/t/t5704-bundle.sh
> @@ -14,7 +14,7 @@ test_expect_success 'setup' '
>  	git tag -d third
>  '
>  
> -test_expect_failure 'annotated tags can be excluded by rev-list options' '
> +test_expect_success 'annotated tags can be excluded by rev-list options' '
>  	git bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&
>  	git ls-remote bundle > output &&
>  	grep tag output &&

Re: [PATCH 1/2] t5704: Fix the test that checks for excluded tags

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
>> index a45c316..2f063ea 100755
>> --- a/t/t5704-bundle.sh
>> +++ b/t/t5704-bundle.sh
>> @@ -6,7 +6,7 @@ test_description='some bundle related tests'
>>  test_expect_success 'setup' '
>>  	test_commit initial &&
>>  	test_tick &&
>> -	git tag -m tag tag &&
>> +	git tag -am tag tag &&
> ...
> Oh, wait.

In any case, the fix in 2/2 is real, and applying both and then
reverting the above hunk passes the test.  Also, applying both,
reverting the above hunk *and* reverting the fix to bundle.c of
course makes the rest fail.

So I would be tempted to squash these two patches into one using the
log message from the latter one, while excluding the change in the
above hunk.

Thanks.

>> @@ -14,7 +14,10 @@ test_expect_success 'setup' '
>>  	git tag -d third
>>  '
>>  
>> -test_expect_success 'tags can be excluded by rev-list options' '
>> +test_expect_failure 'annotated tags can be excluded by rev-list options' '
>> +	git bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&
>> +	git ls-remote bundle > output &&
>> +	grep tag output &&
>>  	git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
>>  	git ls-remote bundle > output &&
>>  	! grep tag output

[PATCH 1/2] t5704: Complement annotated tag exclusion test

[Lukas Fleischer]

In c9a42c4 (bundle: allow rev-list options to exclude annotated tags,
2009-01-02), we added a test to check whether annotated tags, which fall
outside the specified date range, are excluded from bundles. Complement
this test by also checking whether tags inside the date range are
included. Since this addition reveals that the annotated tag exclusion
is flawed, mark the test expect-failure for now.

Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
---
I decided that it is still worthwhile to have this in a separate patch.
Feel free to squash 1/2 and 2/2 or let me know that they should be
merged if you prefer that.

 t/t5704-bundle.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index a45c316..2d53388 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -14,7 +14,10 @@ test_expect_success 'setup' '
 	git tag -d third
 '
 
-test_expect_success 'tags can be excluded by rev-list options' '
+test_expect_failure 'tags can be excluded by rev-list options' '
+	git bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&
+	git ls-remote bundle > output &&
+	grep tag output &&
 	git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
 	git ls-remote bundle > output &&
 	! grep tag output
-- 
2.0.4

[PATCH 2/2] bundle: Fix exclusion of annotated tags

[Lukas Fleischer]

In commit c9a42c4 (bundle: allow rev-list options to exclude annotated
tags, 2009-01-02), support for excluding annotated tags outside the
specified date range was added. However, the wrong order of parameters
was chosen when calling memchr(). Fix this by swapping the character to
search for with the maximum length parameter.

Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
---
 bundle.c          | 4 ++--
 t/t5704-bundle.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bundle.c b/bundle.c
index 71a21a6..b708906 100644
--- a/bundle.c
+++ b/bundle.c
@@ -221,8 +221,8 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)
 	line = memmem(buf, size, "\ntagger ", 8);
 	if (!line++)
 		return 1;
-	lineend = memchr(line, buf + size - line, '\n');
-	line = memchr(line, lineend ? lineend - line : buf + size - line, '>');
+	lineend = memchr(line, '\n', buf + size - line);
+	line = memchr(line, '>', lineend ? lineend - line : buf + size - line);
 	if (!line++)
 		return 1;
 	date = strtoul(line, NULL, 10);
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 2d53388..a828c71 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -14,7 +14,7 @@ test_expect_success 'setup' '
 	git tag -d third
 '
 
-test_expect_failure 'tags can be excluded by rev-list options' '
+test_expect_success 'tags can be excluded by rev-list options' '
 	git bundle create bundle --all --since=7.Apr.2005.15:14:00.-0700 &&
 	git ls-remote bundle > output &&
 	grep tag output &&
-- 
2.0.4