On Tue, Jan 22, 2002 at 07:54:53PM -0600, philip@tigris.org wrote:
>...
> * subversion/libsvn_client/ra.c
> (svn_client__open_ra_session): Add new arbitrary_wc parameter, which is
> set when the working copy revision does not necessarily relate to the
> revision coming from the server.
Gah... why a new parameter? If you are not referring to the revision in the
working copy, then you must ignore everything in the wc. Stuff coming from
the server (deltas, diffs, prop deltas, etc) are all going to be relative to
the false revision and cannot be applied to the wc. Thus, the wc is
irrelevant.
And we already have a way to express that: base_dir == NULL. I suspect the
"arbitrary_wc" flag is wrong. Use NULL for base_dir instead.
-1 on this change pending discussion/explanation here.
>...
> +++ NEW/trunk/subversion/libsvn_client/ra.c Tue Jan 22 19:54:31 2002
>...
> @@ -88,7 +89,16 @@
> ccb.prefix_path = cb->base_dir;
> ccb.pool = cb->pool;
>
> - return svn_wc_get_wc_prop(&ccb, relpath, name, value);
> + err = svn_wc_get_wc_prop(&ccb, relpath, name, value);
> + if (err && err->apr_err == SVN_ERR_WC_OBSTRUCTED_UPDATE && cb->arbitrary_wc)
> + {
> + /* If we have an arbitrary working copy revision, then the requested
> + entry may not exist. This is acceptable, we simply have no
> + properties. */
> + *value = NULL;
> + return SVN_NO_ERROR;
> + }
> + return err;
If the revision does not correspond to the WC, then you CANNOT return ANY
props from the WC. This should have bailed way sooner... not just when an
error happens to occur.
>...
> + /* Set the arbitrary wc revision flag, to indicate that the wc revision
> + may bear no relation to the revision of the requested data. */
> SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL, anchor,
> - TRUE, use_admin, auth_baton, pool));
> + use_admin, use_admin, TRUE, auth_baton,
> + pool));
Set the anchor to null if you're going to ignore the wc.
In truth, I think you want to construct an update from REV1:REV2 ignoring
the WC. When a delta for a file is described, do a get_file over another
sessions to get the REV1 file. That session should be based on the WC so
that the REV1 fetch can be a delta on the wire (which you won't notice cuz
you should get a fulltext). You can then apply the REV1:REV2 delta for that
file to get the REV2 fulltext.
Now, the problem is that the REV1:REV2 delta isn't going to fetch deltas for
the REV1:REV2 stuff. ra_dav actually fetches a fulltext for REV2. To get it
to fetch a delta, it needs the wc_props (for REV1). (technically, it could
do a couple extra requests rather than require the wc_prop)
But... get_file *should* be returning all the props for REV1. But you have a
chicken and egg. The delta said "I changed FOO in REV1->REV2" and then it
gives you a "delta" (really the REV2 fulltext). But you really want to get
the fulltext and props for REV1 via get_file first, so that REV1->REV2 can
be a true delta. But you can't know to get REV1:FOO until you were told...
The answer is that apply_textdelta(FOO) is called first. You can then fetch
the fulltext and props for FOO. Then ra_dav will fetch the REV1->REV2 delta
and if you're sneaky, you can respond to the get_wc_prop callback with the
props that you just fetched.
>...
> + /* ### TODO: Forcing the anchor to null prevents the server sending
> + diffs over the second session. The repository diff editor doesn't
> + handle them yet. */
> + SVN_ERR (svn_client__open_ra_session (&session2, ra_lib, URL,
> + NULL, /* ### TODO: anchor, */
> + FALSE, FALSE, TRUE,
> auth_baton, pool));
>
> + if (anchor)
> + {
> + /* Closing and reopening the first session, forcing the anchor to
> + null, is required. Otherwise the ra_dav server will send
> + differences between the working copy and end_revision instead
> + of between start_revision and end_revision. This in itself is
> + not a problem, the editor could be modified to handle it, but
> + ra_local doesn't do this and there is no way to distinguish
> + which we are using. */
> + SVN_ERR (ra_lib->close (session));
> + SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL, NULL,
> + FALSE, FALSE, TRUE,
> + auth_baton, pool));
I'm suspicious of the above code. It sounds like too much knowledge on the
client's part.
Please... as I said earlier today, this is a bug in ra_dav's get_file.
Working around a bug is fine, but it should be noted as such. I don't see
anything like "### oops. work around an ra_dav bug. <this> happens, so we
set <that> to disable <glarb>"
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:58 2006