Troy Curtis Jr wrote:
> On Thu, Mar 6, 2008 at 5:12 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
>>I feel this "option 2" is the better route, but right now I'm open to either
>>way, and I acknowledge that moving code to a different architectural layer is a
>>bigger task that you might be less comfortable with. Ideally I or someone else
>>would prepare this new layer first, and then you would be able to use it.
> I think it is certainly the best route, but one that I definetly was not
> confortable submitting out of the gate: "Hey you don't know me bu
> here's my patch
> that completely changes the way svn parsing command-line arguments, oh and I'm
> removing a piece of the public API to boot!". Though now that it's being
> discussed I don't mind taking a stab at doing some of the "grunt" work needed.
My latest thought is that this would fit logically in libsvn_client. It might
not be generic to ALL clients - it's very much aimed at command-line clients -
but I don't think that means it would be wrong to put it in libsvn_client.
That would mean the present svn, svnlook, svnadmin, etc. could share it if they
want to, as well as third-party clients.
>>>+ if ((error = svn_client_root_url_from_path(&tmp_root_url, target,
>>>+ ctx, pool)))
>>>+ if ( (error->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
>>>+ || (error->apr_err == SVN_ERR_WC_NOT_DIRECTORY))
>>I would like to see the practical effect of these exceptions: what sort of
>>commands will unexpectedly choose the current working directory's repository
>>because of this?
>>Here's a made-up example of what I mean. This example isn't showing a real
>> $ cd ~/tmp/sandbox
>> $ svn copy ^/branches/1.4.x ~/src/subversion/tags/1.5.x
>> svn: 'branches/1.4.x' not found in the repository of 'sandbox'
>>In these cases, where the target is a not-yet-existing path or URL (perhaps a
>>target of "svn copy"?), could we nevertheless determine the repository in which
>>it will exist so that we don't get these surprises? Or is that not the
> This is exactly the case. One of the cases is for a non-existant url and the
> other is for a non-existant filesystem path. Discovered by trial-and-error
> as I am still a Subversion code novice (but maybe not so much after this all
> gets implemented!). It certainly seems like a good idea to try a bit harder
> to find out what repository a non-existant target is meaning. We could keep
> pulling components off the end until we find a match, and if we don't then
> fall through.
> Would it be enough to just try removing one component? Is it even valid to
> have a path specifying non-existant parents?
I'm not sure how to do this. Pulling off components and re-trying would work in
some cases but not in general.
> I definetly think option 2 is really the right way to do things. Also coupled
> with Pilato's and Wright's request it could be useful for things other than
> relative url parsing.
> So are you advocating putting all these things inside the command line client
> code, or perhaps stick them in libsvn_client. It seems to me if we
> effectively deprecate the svn_opt_args_to_target_array3() function, then we
> ought to provide a public equivalent for the "Next Generation". Or it may be
> that no other projects use that API and it is unneeded.
I think we can just do the usual API-revision thing: the existing
svn_opt_args_to_target_array3() will remain available but deprecated, and the
new function will be svn_client_args_to_target_array() (name reflecting its new
location) will be the preferred replacement.
This is likely to involve some code duplication, but it need not be too much
because (as in the patch I attached to the last email ) we can factor out
some of the common parts and make them available from the lower-level library
as private inter-library API calls ("svn_opt__arg...").
I was previously concerned that we'd have to create a new library to do this
properly, which would be a bit of a hassle, but unless someone finds an
objection to it going in libsvn_client I think we have a relatively easy route.
So, let's say you create
- public, in new file libsvn_client/cmdline.c, like the version I called
svn_cl__args_to_target_array4() in , which calls
- instead of duplicating the WC's knowledge of admin. dir. names;
- these two being private but inter-library APIs as suggested in , with
declarations in svn_opt.h, to avoid too much code duplication because the
deprecated svn_opt_args_to_target_array3() will still have to perform these two
kinds of processing.
Then it sounds like everybody wants the results returned in a structure with
separate fields for the "target kind", the path, and the peg revision. (I'd
leave aside the talk about an "operative revision" field for now; that's not
currently in the scope of these functions and would be an extra complication.)
You can decide whether to do the character-encoding and canonicalisation
before or after resolving to a relative URL. To do it before, you'd need to
ensure all of the conversion and checking funcs that are involved can accept a
relative URL, which at least one of them presently can't. To do it afterwards,
you can have the "inner" function (a file-static function that I called
svn_cl__args_to_target_array4()) return targets among which the relative-URLs
haven't been processed, and then the "outer" function that resolves them
against a repository root can process them afterwards. Both ways seem equally
good to me at the moment.
If you're up for this I'm willing to help.
 My unfinished patch for suggestive purposes,
attached to email <http://svn.haxx.se/dev/archive-2008-03/0159.shtml>.
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-07 11:22:50 CET