On 5 February 2015 at 16:05, Julian Foad <[email protected]> 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 reference. -- Ivan Zhakov
