Karl Fogel <kfogel@newton.ch.collab.net> writes:
> [First question from the patch review...]
>
> Philip, did you ever get any response to your mail below?
Wow, an old patch! No, I don't remember any comments.
>
> I haven't reviewed it yet; first want to find out if maybe you already
> gotten responses and decided not to apply it.
I didn't apply it because of the way the patch handled properties in
ra_dav/fetch.c, I didn't understand the implications of the change.
There is also the question of the extra coupling between the ra code
and the editor.
The patch is out of date now, I assume the file retrieval bit will
need to handle properties as well, but it should be possible to do
that in the same way the text is handled.
I did subsequently try to come up with a more general solution
involving a "custom" ra session instead of a pair of cooperating
sessions. My plan was to have the editor override the "custom" ra
session's callbacks, so that I could move all the patch's special ra
behaviour into the editor file. However it turned out to be a bit
tricky, it was difficult to get session and editor initialisation to
setup the correct callbacks, and it all got a bit clumsy. In
comparison, the approach used in the patch is quite straightforward,
assuming that we can accept the special processing in the ra files.
> Philip Martin <philip@codematters.co.uk> writes:
> > This is a patch for 'svn diff -rREV1:REV2' that replaces the two
> > independent RA sessions with a cooperating pair. This allows the REV1
> > fulltext to be used as the basis for the REV1 to REV2 diff allowing
> > the server to send a diff.
> >
> > I don't know if this is the right way to implement it, but it appears
> > to do what I expect and it passes the tests. There is one possible
> > problem, I had to add the SVN_RA_DAV__LP_VSN_URL property to the
> > properties hash returned by svn_ra_dav__get_file.
This was the problem bit, it's probably out-of-date now.
> > Index: ../svn/subversion/libsvn_ra_dav/fetch.c
> > ===================================================================
> > --- ../svn/subversion/libsvn_ra_dav/fetch.c
> > +++ ../svn/subversion/libsvn_ra_dav/fetch.c Thu Jan 24 14:59:32 2002
> > @@ -856,6 +856,12 @@
> > svn_string_create(val, ras->pool));
> > #undef NSLEN
> > }
> > + /* ### TODO: Need this to make the double ra session override file
> > + work. Can I just add properties like this? */
> > + apr_hash_set(*props,
> > + SVN_RA_DAV__LP_VSN_URL,
> > + APR_HASH_KEY_STRING,
> > + svn_string_create(get_vsn_url(rsrc), ras->pool));
> > }
> >
> > return SVN_NO_ERROR;
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 27 19:32:15 2002