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.
>
> # why provide an option at all?
>
> eol-style normalization *should* be a no-op when synchronizing SOURCE
> repositories that don't require it. I'm just a little squeamish about
> silently fixing up what I'm syncing without having been asked by the
> user to do so.
>
> # why store the option as a property of the TARGET repository?
>
> Some SOURCE repositories require eol-normalization in order to be
> successfully synced to a TARGET using svnsync 1.6. Other source
> repositories do not.
>
> Largely, this is a question of whether or not the source repository
> has accepted commits from Subclipse on Windows in the past.
>
> A source repository that requires eol-normalization *now* will
> probably require it forever. We can't rely on the owner of the
> source repository taking the time to do a dump->normalize->load.
>
> A source repository that does not require eol-normalization *now*
> probably won't require it in the future, as the defect in Subclipse
> on Windows has been fixed.
>
> From this I conclude:
>
> The answer to the question "does this SOURCE repository require
> eol-normalization in order to be successfully synced with svnsync
> 1.6?" is a property of the SOURCE and is *unlikely* to change.
>
> However:
>
> We have no way to associate this flag with the SOURCE, since it's
> outside of our control. The next logical place to store it is with
> the TARGET.
>
> Therefore, I chose to provide this as an option to the init subcommand
> so that the TARGET would remember this setting. This allows you, as
> the creator of the TARGET repository to "set and forget" this.
:)
You first explain how it makes sense to store in the source, and then opt
out to storing in the target? In effect, if you have only a single source
that needs normalizing, all sources will have to be normalized.
This adds to the argument I make further below...
>
> # why store the option as a revprop in r0?
>
> This seems to be where svnsync stores its metadata. Who am I to
> question that?
>
> # Problems with this design
>
> In order to pass --fix-svn-prop-eol-style to init, you need to
> *already* know that SOURCE requires it. The usual way to learn this
> is by attempting first without --fix-svn-prop-eol-style and then
> failing at some point during the initial sync. Possibly after hours
> and thousands of revisions. Clearly, this is a bit of a
> chicken-and-the-egg problem.
>
> A user in this situation has three options:
>
> 1. start over by creating a new TARGET, passing
> --fix-svn-prop-eol-style this time.
>
> 2. manually set svn:sync-fix-svn-prop-eol-style on r0
>
> 3. remember to always pass --fix-svn-prop-eol-style to future calls to
> sync and copy-revprops.
>
> I don't like the first option because it means repeating all the work
> already done up to the point of the first failure.
>
> I don't like option two because it feels like a hack.
>
> I don't like option three because it requires the owner of the TARGET
> repository to remember which of his clones require
> fix-svn-prop-eol-style and which do not. This because a configuration
> headache when one want to, e.g. sync a directory full of TARGET
> repositories.
>
> # What I'd really like
>
> A subcommand to manipulate this flag, so that it can be set as
> required after the first failure shows up. Alternatively,
> --fix-svn-prop-eol-style on sync and copy-revprops could be made
> "sticky", causing all future calls (even without the flag) to do
> eol-style normalization.)
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 :)
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.
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)
I'll look into this some further and will probably commit a patch pretty
soon, implementing "the simplest design". If the need arises, we can still
add your configuration code on top of that.
Let me say again, I hope you've found hacking Subversion being fun, and that
you might come back and fix more things :)
Thanks,
~Neels
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2352713
Received on 2009-05-22 02:44:14 CEST