Committed in -r37795. Nominated for backport.
B. Smith-Mannschott wrote:
> On Fri, May 22, 2009 at 02:43, Neels J Hofmeyr <neels_at_elego.de> wrote:
>> B. Smith-Mannschott wrote:
>>> Let me try to explain the thinking that lead to the current design of
>>> my 3404 patch, and see if that helps answer your questions. In this
>>> description I'll be distinguishing between SOURCE repositories and
>>> TARGET repositories.
>>>
> [ ...snip... ]
>> Your reasoning makes a lot of sense, but is based on the assumption that it
>> is required to be able to switch this on and off.
>>
>> Consider this:
>>
>>
>> a) If you svnsync (with svn >= 1.6), and you ever encounter an inconsistent
>> eol, your *only* option to successfully sync is to convert the line ending.
>> If you choose not to convert, you might as well delete the target repos.
>>
>> My conclusion here is that we should always convert, but say that we did, if
>> we did.
>>
>> Can you deliver a use case where it's absolutely required not to carry out
>> an svnsync in case the props' eols are inconsistent?
>>
>>
>> b) Searching for inconsistent line endings is cheap. Running the check on
>> already consistent properties is a matter of scanning the contents once.
>> This is neglectable in the face of transmitting the data over the network.
>>
>> Adding configuration options, documentation thereof and creating a new -r0
>> property is, IMHO, quite disproportionate. So...
>>
>>> # The simplest design
>>>
>>> The simplest design would just always do eol-style normalization, but
>>> I'm hesitant about this.
>> I'm not :)
>
> In that case (and in light of your argument above), we should go the
> simplest route and always normalize on sync. This is nice because it
> does afford a much simpler solution.
>
>> Thanks a lot for your effort, I can see there is a lot of thought in it and
>> your patch makes a lot of sense. I'm saying this 'cause I don't want to put
>> you off by rejecting a large chunk of your distinguished work.
>>
>> At the end of the day, though, I hope you agree that it's too much code for
>> too cheap and straightforward a thing.
>
> I'd rather the solution be simple than complex.
>
>> Btw, about using svn_subst_translate_cstring2() -- from what brane says it
>> seems that this function is too expensive to be used "all the time", like in
>> my solution. I'll investigate a litte more. We'd also need some indicator
>> whether anything was changed. I'm thinking either write a new string
>> function to do just that, or try to re-use the existing function that only
>> *checks* for consistency and then do the more expensive conversion if the
>> check says so. (just thinking aloud)
>
> Earlier I'd intended to use my own eol-normalization function [1]. I
> decided to leave it out of the submission since it was pointed out
> that Subversion already contains similar functionality. While I
> suspected the expense of svn_subst_translate_cstring2 [2], I figured
> that with the logic being optional and network IO involved it probably
> wouldn't matter.
>
> [1]: http://svn.haxx.se/dev/archive-2009-04/0682.shtml
> [2]: http://svn.haxx.se/dev/archive-2009-04/0709.shtml
>
>> I'll look into this some further and will probably commit a patch pretty
>> soon, implementing "the simplest design".
>
> Great!
>
>> If the need arises, we can still
>> add your configuration code on top of that.
>
> I'll keep it around, should it be needed, but I'd be just as happy if
> it's not needed.
>
>> Let me say again, I hope you've found hacking Subversion being fun, and that
>> you might come back and fix more things :)
>
> Yes, it was fun. The first time I'd used C in anger in... oh... 10
> years. An interesting challenge.
>
> // Ben
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2352746
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2353088
Received on 2009-05-23 05:38:31 CEST