Greg Stein <gstein@lyra.org> writes:
> On Tue, Apr 01, 2003 at 03:21:55PM -0600, sussman@tigris.org wrote:
> >...
> > Issue #1090: svn_client_copy(URL, wc) now recognizes different UUIDs.
> > If the repository UUIDs are different, then schedule a normal add,
> > rather than an add-with-history.
> >
> > Note that this is the first change which actually *depends* on the
> > repository having a fetchable UUID. But this should be in accord with
> > our compatibility policy, since we've had repository UUIDs since svn 0.19.
>
> So what happens when you run it against an older repository? For example,
> our own svn.collab.net? Graceful failure? Maybe an assumption that the
> repositories are the same or different?
Heh, you have good precognition. I should have inspected this more
carefully. :-)
svn.collab.net doesn't yet have a repository UUID, so I tried running
$ svn cp http://svn.collab.net/repos/svn/trunk/subversion/include/svn_wc.h .
In the ethereal trace, we do a PROPFIND for the repository-uuid
property of svn_wc.h, and get back a 207 response containing a 404 for
the specific property.
But it seems that in this situation, svn_ra_dav__get_one_prop returns
an svn_string_t of "", rather than NULL, which is what would normally
throw a graceful error ("Repository has no UUID.")
So later on, when repos_to_wc_copy() compares UUIDs of src and dst,
it's comparing two empty strings, so it thinks they're the same. Doh.
I'll fix this bug -- svn_client_uuid_from_url() should do some sanity
checking on the value it returns: the value shouldn't be NULL, nor
should it have a strlen of 0.
> While our compat policy defines the *outermost* bounds of compatibility, it
> doesn't say "feel free to break things" either. Is there a way to get this
> to work acceptably against older repositories without much effort?
If one or both of the repository UUIDs is non-existent, I guess all we
can do is *assume* the UUIDs are different, and schedule an addition
without history. Is that preferable to just throwing an error?
> > * main.py (copy_repos): new optional 'ignore_uuid' argument, so we can
> > pass --ignore-uuid to 'svnadmin load'.
>
> Could you maybe include partial paths to the filenames? It took me "a while"
> to figure out which file was changed here. If it had said:
>
> * tests/clients/cmdline/svntest/main.py
>
> Then it would have been a no-brainer.
Yeah, sorry bout that.
> > +++ trunk/subversion/libsvn_client/copy.c Tue Apr 1 15:21:53 2003
> >...
> > + /* Get the repository uuid for the *parent* of the destination,
> > + since dst_path doesn't actually exist yet. */
>
> How do you know the parent exists? Maybe somebody is copying into a new
> directory? It would seem that you'd have to "walk up" the working copy until
> you found a directory that had a URL in its entries file.
If you create a new wc directory and schedule it for addition, it
still has a URL in its entry, even if the URL doesn't exist yet.
But yes, I'm thinking here that opening an RA session to a fictional
URL is still bound to cause problems: it will probably cause ra_local
and ra_svn to fail immediately, and ra_dav will choke when doing a
PROPFIND on that fictional url.
So I'll do what you say -- walk up the wc until we don't get an error
fetching the UUID.
>
> >...
> > +++ trunk/subversion/libsvn_client/ra.c Tue Apr 1 15:21:53 2003
> >...
> > +svn_error_t *
> > +svn_client_uuid_from_url (const char **uuid,
> > + const char *url,
> > + svn_client_ctx_t *ctx,
> > + apr_pool_t *pool)
> > +{
> > + svn_ra_plugin_t *ra_lib;
> > + void *ra_baton, *session;
> > + apr_pool_t *subpool = svn_pool_create (pool);
>
> Creating a local subpool like this doesn't really mesh with our pool
> management policy. There is no iteration occurring here. Normally, we'd
> defer this out to the caller to manage.
This is one of those annoying times when the caller wants to pass in
*two* pools: one to hold the UUID return value, and one for doing the
temporary RA session in. Should this function take two pools? One
for return data, one for scratchwork?
>
> >...
> > +++ trunk/subversion/libsvn_ra_dav/session.c Tue Apr 1 15:21:53 2003
> > @@ -598,15 +598,23 @@
> > apr_pool_t *pool)
> > {
> > svn_ra_session_t *ras = session_baton;
> > + svn_error_t *err;
> >
> > if (! ras->uuid)
> > {
> > - apr_hash_t *props;
> > const svn_string_t *value;
>
> Why is the error out at the higher level? It can sit "inside" this block.
Surely.
>
> > - SVN_ERR(svn_ra_dav__get_dir(ras, "", 0, NULL, NULL, &props, pool));
> > - value = apr_hash_get(props, SVN_PROP_ENTRY_UUID, APR_HASH_KEY_STRING);
> > + ne_propname uuid_propname = { SVN_DAV_PROP_NS_DAV, "repository-uuid" };
>
> static const! No need to keep rebuilding this on every entry. (not like
> that is a big problem, given you're about to hit the network, but above is a
> departure from our typical pattern)
Will fix.
> > +
> > + if (ignore_uuid):
> > + load_args = load_args + " --ignore-uuid"
>
> Parentheses in an "if" statement? :-)
*Blush*
Fixes coming up soon, thanks.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Apr 2 19:09:58 2003