On Tue, Apr 01, 2003 at 10:57:57PM -0600, kfogel@collab.net wrote:
i'll look into all these in more depth tomorrow, but off the top of my
head... [hope this is coherent, i'm tired.]
> The new doc string isn't really clear on how condensation behaves when
> there is a mixture of URL and local path targets. Does *pbasename get
> set to the local path prefix, or to the URL prefix? Whichever one it
> is, does condensation (i.e., removal of redundancies) only happen to
> that kind of path, and the other targets are left unchanged?
this is not an excuse for my not documenting it, but one of the added test
cases answers at least one of your questions:
[target-test.py]
+ ('mixed paths and URLs',
+ 'z/A/B http://host/A/C/D',
+ ': ' + cwd + '/z/A/B, http://host/A/C/D, \n')]
in other words, if there is a mixture of paths and URLs, then they have
no common prefix, period. *PBASENAME will be empty.
i *think* condensation will apply to all of them.
Example:
targets: foo/bar/baz http://a/b/c http://a/b foo/bar/baz/tux
=> pbasename: ""
condensed_targets: foo/bar/baz http://a/b
yeah, i should document this, and make the test case check that
the condensing actually happens. thanks for pointing it out.
(originally i was going to refuse to accept a mixture of paths and URLs,
but i figured, just cuz they have no prefix in common, that doesn't mean
you can't still condense them.)
> I can maybe work around the problem, by simply having the caller,
> svn_client_commit(), guarantee that no paths are URLs,
what does the caller do if you pass it two paths that have no common
prefix? whether the targets are homogenous or mixed, the caller is going
to have to handle this case. in the case of commit, you're probably
just dealing with paths, but if some other calling code wants to be able
to handle a mix URLs and paths -- well, in general with URLs you are going
to be calling repos access functions, and with PATHs you'll need WC access
functions... so, you're going to have to sort them apart at some point
anyway. if you want to find the common prefix of all the URLs, and the
common prefix of all the paths, you may as well just sort them first and
pass each set to svn_path_condense_targets separately.
(since svn_path_condense_targets didn't really work on URLs at all until
this change, my assumption is that all the *existing* callers already pass
it a list of all paths.)
> > + * New strings are allocated in POOL.
> > + */
> > +static char *
> > +get_longest_path_ancestor (const char *path1,
> > + const char *path2,
> > + apr_pool_t *pool)
>
> Shouldn't this return 'const char *'?
i am sure you are probably right, but could you fill me in on the element
of C-fu that i am missing that would have enabled me to make that judgment?
(the original library function returned a 'char *' (no const)
so i kept it that way for the replacement library function and its
helper.)
> > + for (i = 0; path1[i] != ':'; i++)
>
> Why not "(path1[i] != ':' || path2[i] != ':')" for the condition?
> That gives you an early out, thought that's obviously negligible in
in the case where it would be an 'early out', you hit:
> > + if (path1[i] != path2[i])
> > + return NULL;
and all's good.
but, now that i look closely, there's a bug here, because the latter check
doesn't happen if we hit the semicolon first, and it should.
i think the odds of anybody hitting this bug are tiny to none (the
subsequent handling probably ensures that NULL will be returned anyway),
but i'll commit a fix tomorrow.
> Why does protocol difference imply "real" difference?
>
> Is http://foo/bar/baz != svn://foo/bar/baz? Once you get inside the
> repository, they are the same. The fact that they use different
> access methods might be irrelevant.
1. on a string basis, they have no common prefix, which at best calls
for arbitrarily picking one.
2. do you really want to encode that kind of knowledge in a path library?
3. those URLs don't necessarily point to the same repository (think
Apache config and svnserve -r options)
> Minor nits, of course. Thanks for resolving this issue! And the
> regression tests were most welcome.
sure thing. better comments and a bugfix will be forthcoming.
:) brian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Apr 2 11:32:13 2003