[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1656893 - in /subversion/trunk/subversion: svnsync/sync.c tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.dump tests/cmdline/svnsync_tests_data/mergeinfo-contains-r0.expected.dump

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Thu, 5 Feb 2015 16:26:57 +0300

On 5 February 2015 at 16:05, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> Ivan Zhakov wrote:
>> On 4 February 2015 at 20:45, Julian Foad wrote:
>>> So we're on the third version of the code and third version of the test, for a
>>> tiny edge-case feature. Clearly it's fragile. I had a bad feeling about writing it
>>> this way in the first place.
>> I had the same feelings when were reviewing backport nomination in 1.8.x.
>> I wonder why you duplicated code that parses svn:mergeinfo instead of
>> using svn_mergeinfo_parse() ? I mean:
>> 1. Parse svn:mergeinfo using svn_mergeinfo_parse()
> The parser contains checks including a check that the start rev is not zero.
> I would have to split the low-level parsing from the validation feature of the current parser.
> I started doing this... but didn't know when to stop. (There are lots of things the current
> parser currently checks for, and canonicalizes, and so lots of possibilities for separating
> low-level parsing from higher-level processing.)
Now I understand the situation. Check for zero revision introduced in
r925290 and released in 1.7.0. But didn't not this change break
backward compatibility?

Making svnsync rewrite svn:mergeinfo is also questionable approach
taking into account write-through proxy configurations etc.

I'm not familiar with merge-tracking code, but I'm not sure that
svn_mergeinfo_parse() is proper layer for validating for zero revision

Ivan Zhakov
Received on 2015-02-05 14:28:33 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.