Re: svnignore?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svnignore?

Daniel Shahaf-2
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.

config-test-line-continuation.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svnignore?

Branko Čibej
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
Reply | Threaded
Open this post in threaded view
|

Re: svnignore?

Branko Čibej
In reply to this post by Daniel Shahaf-2
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

Reply | Threaded
Open this post in threaded view
|

Re: svnignore?

Branko Čibej
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

Reply | Threaded
Open this post in threaded view
|

Re: svnignore?

Branko Čibej
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

Reply | Threaded
Open this post in threaded view
|

Re: svnignore?

Branko Čibej
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