Josh Pieper <jjp@pobox.com> writes:
> Philip Martin wrote:
>> > @@ -2083,6 +2153,19 @@
>> > /* If a conflict happens, then the entry will be
>> > marked "Conflicted" and will track either 2 or 3 new
>> > temporary fulltext files that resulted. */
>>
>> Is the above comment now redundant?
>
> Well, my new comment doesn't really state it, but I suppose it can go.
Your comment doesn't need to state it, it's information that applies
to the old code.
>> > +
>> > + /* Run a dry-run of the merge to see if a conflict will
>> > + occur. This is needed so we can report back to the
>> > + client as the changes come in. */
>> > + base = svn_wc_adm_access_path (adm_access);
>> > +
>> > + SVN_ERR (svn_wc_merge (svn_path_join (base, txtb, pool),
>> > + svn_path_join (base, tmp_txtb, pool),
>> > + svn_path_join (base, base_name, pool),
>> > + adm_access,
>> > + oldrev_str, newrev_str, ".mine",
>> > + TRUE, &merge_outcome, diff3_cmd,
>> > + pool));
>>
>> This is a bit unfortunate. I think I see what you are doing, the old
>> code could examine the wc state to get this information but in the new
>> code the notification callback is called before the log file is run.
>> A dry-run merge can be time consuming when dealing with large files,
>> although since only files with local mods are affected I guess it may
>> not be too bad in practice. I don't know if there are any
>> alternatives, I guess it's difficult to postpone the notification
>> callback.
>
> Yeah, I thought about it some too and decided that a constant factor
> extra merge per file with local mods was probably worth it to drop the
> O(n^2) behavior. I imagine more people will run into the checkout of
> a large directory than would run into having many large files with
> local mods. And at worst the latter half would be only twice as slow.
> The former could easily be many orders of magnitude slower.
I tend to agreee, but I still think it's unfortunate that we have to
do a potentially expensive operation twice.
>> Have you tested over ra_dav? I recall we had problems in the past
>> with wcprops during an interrupted update, although I can't remember
>> the exact details. It's possible we made the wcprops code robust
>> enough for this change to work.
>
> No I haven't. Is the editor somehow driven differently via ra_dav
> that would cause different problems? Was there an issue, and can you
> remember it?
The problem is that ra_dav is the only layer that uses wcprops, if you
only test over ra_local you won't see wcprops related failures.
The original problem generated some dev list discussion between me and
Karl, and some code from Karl. As I recall the problem was that an
interrupted update caused the wcprops and the text-base/revision to be
out-of-sync (I can't remember which one was wrong). I don't know
whether the wcprops code is still fragile, or whether Karl's change
fixed it.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun May 23 19:54:02 2004