You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2017/12/30 10:00:03 UTC

Re: svnignore?

Branko Čibej wrote on Sat, 30 Dec 2017 10:09 +0100:
> Don't know what you were trying

[[[
% cat ~/.subversion/config
[miscellany]
global-ignores = bar
 foo
% touch foo bar
]]]

> but see parse_option() and
> parse_value_continuation_lines() in subversion/libsvn_subr/config_file.c.

(lldb) p ctx->value->data
(char *) $2 = 0x00007ffff7e1d310 "bar oo"

And if I «touch ./oo» then ./oo gets ignored and ./foo doesn't.

We've got an off-by-one somewhere.

Cheers,

Daniel

Re: svnignore?

Posted by Branko Čibej <br...@apache.org>.
On 30.12.2017 14:52, Branko Čibej wrote:
> I'll commit later today, with Daniel's text.

s/text/test/.

r1819603 and proposed for 1.10.0.

-- Brane


Re: svnignore?

Posted by Branko Čibej <br...@apache.org>.
On 30.12.2017 14:41, Branko Čibej wrote:
> On 30.12.2017 11:29, Branko Čibej wrote:
>> On 30.12.2017 11:07, Daniel Shahaf wrote:
>>> Daniel Shahaf wrote on Sat, 30 Dec 2017 10:00 +0000:
>>>> We've got an off-by-one somewhere.
>>> And here's a regression test.  It passes if the expectation is "foo ar
>>> az" but fails if the expectation is "foo bar baz".
>> This bug seems to be new on trunk (and probably 1.10.x). I suspect the
>> authzperf branch merge, but haven't actually checked.
> Confirmed, this bug appeared in r1776832, which was the merge from the
> authzperf branch.

And fixed ...

[[[
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c	(revision 1819597)
+++ subversion/libsvn_subr/config_file.c	(working copy)
@@ -466,6 +466,7 @@
               else
                 {
                   /* This is a continuation line. Read it. */
+                  SVN_ERR(parser_ungetc(ctx, ch));
                   SVN_ERR(parser_get_line(ctx, ctx->line_read, &ch));
 
                   /* Trailing whitespace is ignored. */
]]]


I'll commit later today, with Daniel's text.

-- Brane


Re: svnignore?

Posted by Branko Čibej <br...@apache.org>.
On 30.12.2017 11:29, Branko Čibej wrote:
> On 30.12.2017 11:07, Daniel Shahaf wrote:
>> Daniel Shahaf wrote on Sat, 30 Dec 2017 10:00 +0000:
>>> We've got an off-by-one somewhere.
>> And here's a regression test.  It passes if the expectation is "foo ar
>> az" but fails if the expectation is "foo bar baz".
> This bug seems to be new on trunk (and probably 1.10.x). I suspect the
> authzperf branch merge, but haven't actually checked.

Confirmed, this bug appeared in r1776832, which was the merge from the
authzperf branch.

-- Brane


Re: svnignore?

Posted by Branko Čibej <br...@apache.org>.
On 30.12.2017 11:07, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Sat, 30 Dec 2017 10:00 +0000:
>> We've got an off-by-one somewhere.
> And here's a regression test.  It passes if the expectation is "foo ar
> az" but fails if the expectation is "foo bar baz".

This bug seems to be new on trunk (and probably 1.10.x). I suspect the
authzperf branch merge, but haven't actually checked.

-- Brane


Re: svnignore?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Sat, 30 Dec 2017 10:00 +0000:
> We've got an off-by-one somewhere.

And here's a regression test.  It passes if the expectation is "foo ar
az" but fails if the expectation is "foo bar baz".

The patch is attached (rather than inline) because it adds a literal tab
to config-test.cfg.

Re: svnignore?

Posted by Branko Čibej <br...@apache.org>.
On 30.12.2017 11:00, Daniel Shahaf wrote:
> Branko Čibej wrote on Sat, 30 Dec 2017 10:09 +0100:
>> Don't know what you were trying
> [[[
> % cat ~/.subversion/config
> [miscellany]
> global-ignores = bar
>  foo
> % touch foo bar
> ]]]
>
>> but see parse_option() and
>> parse_value_continuation_lines() in subversion/libsvn_subr/config_file.c.
> (lldb) p ctx->value->data
> (char *) $2 = 0x00007ffff7e1d310 "bar oo"
>
> And if I «touch ./oo» then ./oo gets ignored and ./foo doesn't.
>
> We've got an off-by-one somewhere.

Indeed ... and here I thought we had tests for this. I'll look into it
later today, but I suspect one of the recent merges may have introduced
this bug.

-- Brane