On Feb 4, 2008 9:41 AM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Feb 1, 2008 5:37 PM, David Glasser <glasser_at_davidglasser.net> wrote:
>
> Hi David, appreciate you taking a look at this, comments below.
>
>
> > On Jan 30, 2008 11:34 AM, <pburba_at_tigris.org> wrote:
> > > Author: pburba
> > > Date: Wed Jan 30 11:34:41 2008
> > > New Revision: 29085
> > Whoa. We're doing a full RA roundtrip for every range in every source
> > in every mergeinfo prop? That seems terribly suboptimal. We should
> > go the other way round: use svn_client__get_history_as_mergeinfo to
> > find the history of the path just once, and then use something like
> > svn_mergeinfo_remove. You should be able to replace the whole
> > function with that sort of thing.
>
> The problem is that svn_client__get_history_as_mergeinfo() can only
> look backwards (I know, hence the name!). This isn't a problem if our
> merge targets working revision is greater than any of the added
> mergeinfo's revisions. Just peg the target to the working rev and do
> as you suggest (I haven't tried this but it certainly seems like it
> should work).
>
> But what if our WC merge target is at working revision *less* than any
> of the added ranges? What can we use for our peg revision to
> svn_client__get_history_as_mergeinfo()? Consider this simple example:
[clear example snipped]
Ugh. You're right. (and this is why letting people merge into
non-simple wcs is hard...)
You might be able to do something where you start with
svn_client__get_history_as_mergeinfo, and then if you see newer ranges
and find out from the server that it's the same line-of-history, then
you can augment the history-as-mergeinfo. Or something. But you're
right, it's definitely more complicated than I thought.
Remind me, how important is this change? Is it largely cosmetic, or
does it have serious effects? Have we tested its performance impact?
Again, if I'm not misinterpreting how it works, it adds a huge number
of roundtrips to every merge request.
> > I must be missing something> > because I can't see how this can possibly work at all.
>
> ...and at first I thought you were right, this must be broken, but I
> tested it out with multi-line mergeinfo and it works. Why, because
> filter_self_referential_mergeinfo() returns an array of svn_prop_t's
> to merge_props_changed() which calls svn_wc_merge_props2() to merge
> the prop adds to the original props. Thing is, svn_wc_merge_props2()
> works perfectly well even if multiple elements in the incoming prop
> changes array refer to the same propname, e.g.
>
> If we have this single elelment propchanges array:
>
> Name Value
> svn:mergeinfo '/A:6\n/A_COPY_2:9\n/A_COPY_3:10'
>
> Or this semantically equivalent 3 element array:
>
> Name Value
> svn:mergeinfo '/A:6'
> svn:mergeinfo '/A_COPY_2:9'
> svn:mergeinfo '/A_COPY_3:10'
>
> Then svn_wc_merge_props2() happily treats these the same way.
Ohhhh... because mergeinfo adds are very special-cased or something?
> > (Also,
> > shouldn't there be a space after the ":" in the property value?)
>
> No, our created mergeinfo has no whitespace. Although it appears our
> parser allows whitespace when using svn propset, but then it doesn't
> get interpreted correctly. I'll look into this some more and start a
> separate thread if necessary.
Huh, wonder why I thought that.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-07 22:23:48 CET